[Libreoffice-commits] core.git: sw/source

[Noel Grandin (via logerrit)]

 sw/source/core/unocore/unoparagraph.cxx |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

New commits:
commit 1e5e65715cf053deb7d52e62dd598ac51cea5ee6
Author:     Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Thu Nov 10 13:16:11 2022 +0200
Commit:     Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Thu Nov 10 20:14:13 2022 +0100

    Revert "tdf#126788 remove weak m_wThis reference"
    
    Causes race condition
    
    And add comment so I don't have think through this again.
    
    This reverts commit 2ae53eb5dcbe0a3c5b4dc5323639a11878a38101.
    
    Change-Id: Id18a7313e363a4ce4930b01fc760a3746b35a711
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/142548
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>

diff --git a/sw/source/core/unocore/unoparagraph.cxx 
b/sw/source/core/unocore/unoparagraph.cxx
index d2f3db46680e..7861f831f9a7 100644
--- a/sw/source/core/unocore/unoparagraph.cxx
+++ b/sw/source/core/unocore/unoparagraph.cxx
@@ -114,6 +114,7 @@ class SwXParagraph::Impl
 {
 public:
     SwXParagraph& m_rThis;
+    unotools::WeakReference<SwXParagraph> m_wThis;
     std::mutex m_Mutex; // just for OInterfaceContainerHelper4
     ::comphelper::OInterfaceContainerHelper4<css::lang::XEventListener> 
m_EventListeners;
     SfxItemPropertySet const& m_rPropSet;
@@ -191,8 +192,19 @@ void SwXParagraph::Impl::Notify(const SfxHint& rHint)
         std::unique_lock aGuard(m_Mutex);
         if (m_EventListeners.getLength(aGuard) != 0)
         {
-            uno::Reference<uno::XInterface> const xThis(m_rThis);
-            lang::EventObject const ev(m_rThis);
+            // fdo#72695: if UNO object is already dead, don't revive it with 
event
+            // The specific pattern we are guarding against is this:
+            // [1] Thread1 takes the SolarMutex
+            // [2] Thread2 decrements the SwXParagraph reference count, and 
calls the
+            //     SwXParagraph destructor, which tries to take the 
SolarMutex, and blocks
+            // [3] Thread1 destroys a SwTextNode, which calls this Notify 
event, which results
+            //     in a double-free if we construct the xThis object.
+            uno::Reference<uno::XInterface> const xThis(m_wThis);
+            if (!xThis.is())
+            {   // fdo#72695: if UNO object is already dead, don't revive it 
with event
+                return;
+            }
+            lang::EventObject const ev(xThis);
             m_EventListeners.disposeAndClear(aGuard, ev);
         }
     }
@@ -214,10 +226,6 @@ SwXParagraph::SwXParagraph(
 
 SwXParagraph::~SwXParagraph()
 {
-    SolarMutexGuard aGuard;
-    // need to stop listening before destruction so we don't get DYING events
-    // that might then revive the SwXParagraph when constructing an EventObject
-    m_pImpl->EndListeningAll();
 }
 
 const SwTextNode * SwXParagraph::GetTextNode() const
@@ -264,6 +272,8 @@ SwXParagraph::CreateXParagraph(SwDoc & rDoc, SwTextNode 
*const pTextNode,
     {
         pTextNode->SetXParagraph(xParagraph);
     }
+    // need a permanent Reference to initialize m_wThis
+    pXPara->m_pImpl->m_wThis = xParagraph.get();
     return xParagraph;
 }