basctl/source/basicide/doceventnotifier.cxx |   28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

New commits:
commit ab38a8d13de0b14b3c389ef27152aac958033401
Author:     Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Mon Aug 21 09:49:02 2023 +0200
Commit:     Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Mon Aug 21 14:04:43 2023 +0200

    tdf#156721 Crash if cursor in Basic IDE is in a macro from a document..
    
    .. and that document is closed
    
    regression from
        commit f3e127217d8daa443b8eda52ac4810e375cc7d63
        Author: Noel Grandin <noel.gran...@collabora.co.uk>
        Date:   Tue May 9 15:21:20 2023 +0200
    use comphelper::WeakComponentImplHelper in
    DocumentEventNotifier::Impl
    
    Change-Id: I5c8e68cd222ee1d66dc832700c4a39fd74223643
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155889
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>

diff --git a/basctl/source/basicide/doceventnotifier.cxx 
b/basctl/source/basicide/doceventnotifier.cxx
index 81bb50be3919..254c719bb5cc 100644
--- a/basctl/source/basicide/doceventnotifier.cxx
+++ b/basctl/source/basicide/doceventnotifier.cxx
@@ -148,20 +148,26 @@ namespace basctl
             if ( !_rEvent.EventName.equalsAscii( aEvent.pEventName ) )
                 continue;
 
+            // Listener implementations require that we hold the mutex, but to 
avoid lock ordering issues,
+            // we need to take the solar mutex before we take our own mutex.
+            aGuard.unlock();
+
+            // Listener implements require that we hold the solar mutex.
+            SolarMutexGuard aSolarGuard;
+
+            // Take the lock again, so we can check our local fields.
+            aGuard.lock();
+            if ( impl_isDisposed_nothrow(aGuard) )
+                // somebody took the chance to dispose us -> bail out
+                return;
+            DocumentEventListener* pListener = m_pListener;
             ScriptDocument aDocument( xDocument );
-            {
-                // the listener implementations usually require the 
SolarMutex, so lock it here.
-                // But ensure the proper order of locking the solar and the 
own mutex
-                aGuard.unlock();
-                SolarMutexGuard aSolarGuard;
-                std::unique_lock aGuard2( m_aMutex );
+            // We cannot call the listener while holding our mutex because the 
listener
+            // call might trigger an event which call back into us.
+            aGuard.unlock();
 
-                if ( impl_isDisposed_nothrow(aGuard2) )
-                    // somebody took the chance to dispose us -> bail out
-                    return;
+            (pListener->*aEvent.listenerMethod)( aDocument );
 
-                (m_pListener->*aEvent.listenerMethod)( aDocument );
-            }
             break;
         }
     }

Reply via email to