[Libreoffice-commits] core.git: Branch 'distro/vector/vector-5.4' - embeddedobj/source

[Libreoffice Gerrit user]

 embeddedobj/source/msole/olecomponent.cxx |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

New commits:
commit f64219ec6698bef8b598268b8a0fa2db839a6c1f
Author:     Miklos Vajna <vmiklos at collabora.co.uk>
AuthorDate: Wed Nov 7 12:24:47 2018 +0100
Commit:     Miklos Vajna <vmiklos at collabora.co.uk>
CommitDate: Thu Nov 8 09:29:59 2018 +0100

    embeddedobj win32: avoid owning a lock while calling out to event listeners
    
    The deadlock happens from time to time, when converting documents
    containing OLE objects via remote UNO (from Java) -- after the
    conversion, when closing the document.
    
    The relevant stacktraces are:
    
    >Debug.ListCallStack /ShowLineOffset /AllThreads
    Callstack for Thread 8 (Thread Id: 32912 (0x8090)):
    ...
     6      sal3.dll!osl_acquireMutex(_oslMutexImpl * Mutex) Line 75
     7      [Inline Frame] emboleobj.dll!osl::Mutex::acquire() Line 56
     8      [Inline Frame] emboleobj.dll!osl::Guard<osl::Mutex>::{ctor}(osl::Mutex &) Line 129
     9      emboleobj.dll!OleComponent::OnClose_Impl() Line 1399
     10     emboleobj.dll!OleWrapperAdviseSink::OnClose() Line 119
    
    Callstack for Thread 11 (Thread Id: 21088 (0x5260)):
    ...
     11     ole32.dll!00007fffc5e44e83()
     12     [Inline Frame] emboleobj.dll!OleComponent::CloseObject() Line 1012
     13     emboleobj.dll!OleComponent::Dispose() Line 484
     14     emboleobj.dll!OleComponent::close(unsigned char bDeliverOwnership) Line 1463
     15     emboleobj.dll!OleEmbeddedObject::GetRidOfComponent() Line 239
     16     emboleobj.dll!OleEmbeddedObject::Dispose() Line 275
     17     emboleobj.dll!OleEmbeddedObject::close(unsigned char bDeliverOwnership) Line 497
    ...
     26     swlo.dll!SwXTextDocument::close(unsigned char bDeliverOwnership) Line 617
    
    OleComponent::OnClose_Impl() taking a lock is fine, but
    OleComponent::close() takes a lock and then later it still calls out
    (via OleComponent::CloseObject()), which is a no-go.
    
    Fix the problem by making sure that callers of Dispose() own no lock at
    the time of the function call, and taking the lock in Dispose() only
    after the CloseObject() call (which invokes event listeners).
    
    Change-Id: I53befee21478188c7f79723b7d7596e66077d1c2
    Reviewed-on: https://gerrit.libreoffice.org/63014
    Tested-by: Jenkins
    Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
    (cherry picked from commit fe83001574b4c8dbab166d8abb8014f78424278a)

diff --git a/embeddedobj/source/msole/olecomponent.cxx b/embeddedobj/source/msole/olecomponent.cxx
index 8c2dd853adfd..94c00df45912 100644
--- a/embeddedobj/source/msole/olecomponent.cxx
+++ b/embeddedobj/source/msole/olecomponent.cxx
@@ -454,8 +454,9 @@ OleComponent::~OleComponent()
 
     if ( m_pOleWrapClientSite || m_pImplAdviseSink || m_pInterfaceContainer || m_bOleInitialized )
     {
-        ::osl::MutexGuard aGuard( m_aMutex );
+        ::osl::ClearableMutexGuard aGuard( m_aMutex );
         m_refCount++;
+        aGuard.clear();
         try {
             Dispose();
         } catch( const uno::Exception& ) {}
@@ -475,12 +476,15 @@ OleComponent::~OleComponent()
 
 void OleComponent::Dispose()
 {
-    // the mutex must be locked before this method is called
     if ( m_bDisposed )
         return;
 
+    // Call CloseObject() without m_aMutex locked, since it will call
+    // IOleObject::Close(), which can call event listeners, which can run on a
+    // different thread.
     CloseObject();
 
+    osl::MutexGuard aGuard(m_aMutex);
     if ( m_pOleWrapClientSite )
     {
         m_pOleWrapClientSite->disconnectOleComponent();
@@ -1414,7 +1418,7 @@ void OleComponent::OnClose_Impl()
 
 void SAL_CALL OleComponent::close( sal_Bool bDeliverOwnership )
 {
-    ::osl::MutexGuard aGuard( m_aMutex );
+    ::osl::ClearableMutexGuard aGuard( m_aMutex );
     if ( m_bDisposed )
         throw lang::DisposedException(); // TODO
 
@@ -1459,6 +1463,7 @@ void SAL_CALL OleComponent::close( sal_Bool bDeliverOwnership )
             }
         }
     }
+    aGuard.clear();
 
     Dispose();
 }