Only hold clipboard mutexes to obtain the values, to avoid deadlocks

Change-Id: I36a2f6e444b8a5397b96d3cba475a8d06ea86459
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165089
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 30 insertions, 19 deletions

diff --git a/vcl/win/dtrans/MtaOleClipb.cxx b/vcl/win/dtrans/MtaOleClipb.cxx
index fcf16df1adf1..a62f4a1c0c72 100644
--- a/vcl/win/dtrans/MtaOleClipb.cxx
+++ b/vcl/win/dtrans/MtaOleClipb.cxx
@@ -707,25 +707,32 @@ unsigned __stdcall CMtaOleClipboard::clipboardChangedNotifierThreadProc(void* pP
         MsgWaitForMultipleObjects(2, pInst->m_hClipboardChangedNotifierEvents, false, INFINITE,
                                   QS_ALLINPUT | QS_ALLPOSTMESSAGE);
 
-        std::unique_lock aGuard2( pInst->m_ClipboardChangedEventCountMutex );
-
-        if ( pInst->m_ClipboardChangedEventCount > 0 )
+        bool hadEvents;
         {
-            pInst->m_ClipboardChangedEventCount--;
-            if ( 0 == pInst->m_ClipboardChangedEventCount )
-                ResetEvent( pInst->m_hClipboardChangedEvent );
-
-            aGuard2.unlock( );
+            std::unique_lock aGuard2(pInst->m_ClipboardChangedEventCountMutex);
+            hadEvents = pInst->m_ClipboardChangedEventCount > 0;
+            if (hadEvents)
+            {
+                pInst->m_ClipboardChangedEventCount--;
+                if (0 == pInst->m_ClipboardChangedEventCount)
+                    ResetEvent(pInst->m_hClipboardChangedEvent);
+            }
+        }
 
-            // nobody should touch m_pfncClipViewerCallback while we do
-            std::unique_lock aClipViewerGuard( pInst->m_pfncClipViewerCallbackMutex );
+        if (hadEvents)
+        {
+            LPFNC_CLIPVIEWER_CALLBACK_t pClipViewerCallback;
+            {
+                // nobody should touch m_pfncClipViewerCallback while we do
+                // but don't hold the mutex while calling the callback itself: it could deadlock
+                std::unique_lock aClipViewerGuard(pInst->m_pfncClipViewerCallbackMutex);
+                pClipViewerCallback = pInst->m_pfncClipViewerCallback;
+            }
 
             // notify all clipboard listener
-            if ( pInst->m_pfncClipViewerCallback )
-                pInst->m_pfncClipViewerCallback( );
+            if (pClipViewerCallback)
+                pClipViewerCallback();
         }
-        else
-            aGuard2.unlock( );
     }
 
     return 0;
diff --git a/vcl/win/dtrans/WinClipboard.cxx b/vcl/win/dtrans/WinClipboard.cxx
index 1a8eaea151b5..5f46bebecfd2 100644
--- a/vcl/win/dtrans/WinClipboard.cxx
+++ b/vcl/win/dtrans/WinClipboard.cxx
@@ -370,13 +370,17 @@ void CWinClipboard::unregisterClipboardViewer() { m_MtaOleClipboard.registerClip
 
 void WINAPI CWinClipboard::onClipboardContentChanged()
 {
-    osl::MutexGuard aGuard(s_aClipboardSingletonMutex);
+    rtl::Reference<CWinClipboard> pWinClipboard;
+    {
+        // Only hold the mutex to obtain a safe reference to the impl, to avoid deadlock
+        osl::MutexGuard aGuard(s_aClipboardSingletonMutex);
+        pWinClipboard.set(s_pCWinClipbImpl);
+    }
 
-    // reassociation to instance through static member
-    if (nullptr != s_pCWinClipbImpl)
+    if (pWinClipboard)
     {
-        s_pCWinClipbImpl->m_foreignContent.clear();
-        s_pCWinClipbImpl->notifyAllClipboardListener();
+        pWinClipboard->m_foreignContent.clear();
+        pWinClipboard->notifyAllClipboardListener();
     }
 }