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.grandin@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.grandin@collabora.co.uk>

1 files changed, 17 insertions, 11 deletions

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;
         }
     }