core.git: Branch 'distro/vector/vector-24.2' - vcl/win

Mike Kaganski (via logerrit) Tue, 26 Nov 2024 21:15:05 -0800

 vcl/win/dtrans/WinClipboard.hxx         |    2 +-
 vcl/win/dtrans/XNotifyingDataObject.cxx |   24 ++++++++++++++++++++----
 vcl/win/dtrans/XNotifyingDataObject.hxx |    2 +-
 3 files changed, 22 insertions(+), 6 deletions(-)


New commits:
commit d1e1123a59fcf6c581f4bc234075948075d698fd
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Nov 26 23:08:04 2024 +0500
Commit:     Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Wed Nov 27 09:19:10 2024 +0500

    Release CXNotifyingDataObject in separate thread
    
    The reported deadlocking situation at paste was:
    
    1. Thread CMtaOleClipboard::run() is handling 
CXNotifyingDataObject::Release,
    which calls CWinClipboard::onReleaseDataObject before deleting self, and 
that
    eventually tries to lock solar mutex:
    
      sal3!osl_acquireMutex+0x49 [sal\osl\w32\mutex.cxx @ 65]
      vclplug_winlo!osl::Mutex::acquire+0xa [include\osl\mutex.hxx @ 63]
      vclplug_winlo!SalYieldMutex::doAcquire+0x91 [vcl\winpp\salinst.cxx @ 148]
      vcllo!osl::Guard<comphelper::SolarMutex>::{ctor}+0xe 
[include\osl\mutex.hxx @ 144]
      vcllo!SolarMutexGuard::{ctor}+0x1b [includecl\svapp.hxx @ 1340]
      vcllo!TransferableHelper::lostOwnership+0x36 [vcl\source  reelist 
ransfer.cxx @ 487]
      vclplug_winlo!CXNotifyingDataObject::lostOwnership+0x61 
[vcl\win\dtrans\XNotifyingDataObject.cxx @ 140]
      vclplug_winlo!CWinClipboard::onReleaseDataObject+0x57 
[vcl\win\dtrans\WinClipboard.cxx @ 364]
      vclplug_winlo!CXNotifyingDataObject::Release+0x36 
[vcl\win\dtrans\XNotifyingDataObject.cxx @ 77]
      combase!Ordinal230+0x1c9b
      ...
      combase!Ordinal87+0x3d19
      USER32!DispatchMessageW+0x741
      USER32!DispatchMessageW+0x201
      vclplug_winlo!CMtaOleClipboard::run+0x18f [vcl\win\dtrans\MtaOleClipb.cxx 
@ 657]
    
    2. Thread CMtaOleClipboard::clipboardChangedNotifierThreadProc() is handling
    the respective event, calling CWinClipboard::handleClipboardContentChanged,
    which notifies listeners, including SwClipboardChangeListener, which, in its
    changedContents, has locked solar mutex, and requests the clipboard content,
    which eventually calls CMtaOleClipboard::getClipboard posting a message to
    the CMtaOleClipboard::run() thread, and waiting the result:
    
      vclplug_winlo!`anonymous-namespace'::Win32Condition::wait+0x3b 
[vcl\win\dtrans\MtaOleClipb.cxx @ 92]
      vclplug_winlo!CMtaOleClipboard::getClipboard+0x220 
[vcl\win\dtrans\MtaOleClipb.cxx @ 355]
      vclplug_winlo!CWinClipboard::getIDataObject+0x84 
[vcl\win\dtrans\WinClipboard.cxx @ 161]
      vclplug_winlo!CDOTransferable::tryToGetIDataObjectIfAbsent+0x56 
[vcl\win\dtrans\DOTransferable.cxx @ 413]
      vclplug_winlo!CDOTransferable::getClipboardData+0x79 
[vcl\win\dtrans\DOTransferable.cxx @ 425]
      vclplug_winlo!CDOTransferable::getTransferData+0x163 
[vcl\win\dtrans\DOTransferable.cxx @ 252]
      sotlo!GetTransferableAction_Impl+0x13d [sot\sourceaseormats.cxx @ 1352]
      sotlo!SotExchange::GetExchangeAction+0xde [sot\sourceaseormats.cxx @ 
1429]
      swlo!SwTransferable::IsPaste+0xba [sw\source\uibase\dochdl\swdtflvr.cxx @ 
1402]
      swlo!SwClipboardChangeListener::changedContents+0xac 
[sw\source\uibase\uiview\uivwimp.cxx @ 315]
      
vclplug_winlo!comphelper::OInterfaceContainerHelper4<com::sun::star::datatransfer::clipboard::XClipboardListener>::NotifySingleListener<com::sun::star::datatransfer::clipboard::ClipboardEvent>::operator()+0xc
 [include      
vclplug_winlo!comphelper::OInterfaceContainerHelper4<com::sun::star::datatransfer::clipboard::XClipboardListener>::forEach<comphelper::OInterfaceContainerHelper4<com::sun::star::datatransfer::clipboard::XClipboardListener>::NotifySingleListener<com::sun::star::datatransfer::clipboard::ClipboardEvent>
 >+0xb6 [include      
vclplug_winlo!comphelper::OInterfaceContainerHelper4<com::sun::star::datatransfer::clipboard::XClipboardListener>::notifyEach+0x28
 [include      vclplug_winlo!CWinClipboard::handleClipboardContentChanged+0x156 
[vcl\win\dtrans\WinClipboard.cxx @ 303]
      vclplug_winlo!CWinClipboard::onClipboardContentChanged+0x5c 
[vcl\win\dtrans\WinClipboard.cxx @ 387]
      vclplug_winlo!CMtaOleClipboard::clipboardChangedNotifierThreadProc+0x172 
[vcl\win\dtrans\MtaOleClipb.cxx @ 699]
    
    This change tries to untie this, by creating a dedicated thread to call
    CWinClipboard::onReleaseDataObject and delete CXNotifyingDataObject outside
    of its Release (and of the CMtaOleClipboard::run() thread), so that locking
    happening in that process doesn't deadlock functional threads.
    
    Change-Id: I93da6fe79225319a84b332c81716793f4d9fb05d
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177363
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>

diff --git a/vcl/win/dtrans/WinClipboard.hxx b/vcl/win/dtrans/WinClipboard.hxx
index 83d01da65e05..a2607b7e221d 100644
--- a/vcl/win/dtrans/WinClipboard.hxx
+++ b/vcl/win/dtrans/WinClipboard.hxx
@@ -52,7 +52,7 @@ class CWinClipboard final
                                                  
css::datatransfer::clipboard::XFlushableClipboard,
                                                  css::lang::XServiceInfo>
 {
-    friend STDMETHODIMP_(ULONG) CXNotifyingDataObject::Release();
+    friend CXNotifyingDataObject::~CXNotifyingDataObject();
 
     css::uno::Reference<css::uno::XComponentContext> m_xContext;
     const OUString m_itsName;
diff --git a/vcl/win/dtrans/XNotifyingDataObject.cxx 
b/vcl/win/dtrans/XNotifyingDataObject.cxx
index aab168f043d3..c5110f6a283e 100644
--- a/vcl/win/dtrans/XNotifyingDataObject.cxx
+++ b/vcl/win/dtrans/XNotifyingDataObject.cxx
@@ -40,6 +40,12 @@ CXNotifyingDataObject::CXNotifyingDataObject(
 {
 }
 
+CXNotifyingDataObject::~CXNotifyingDataObject()
+{
+    if (m_pWinClipImpl)
+        m_pWinClipImpl->onReleaseDataObject(this);
+}
+
 STDMETHODIMP CXNotifyingDataObject::QueryInterface( REFIID iid, void** 
ppvObject )
 {
     if ( nullptr == ppvObject )
@@ -64,6 +70,17 @@ STDMETHODIMP_(ULONG) CXNotifyingDataObject::AddRef( )
     return static_cast< ULONG >( InterlockedIncrement( &m_nRefCnt ) );
 }
 
+namespace
+{
+// delete CXNotifyingDataObject is a dedicated thread. It calls 
CWinClipboard::onReleaseDataObject,
+// which may lock solar mutex, and if called in CMtaOleClipboard::run() 
thread, may deadlock.
+unsigned __stdcall releaseAsyncProc(void* p)
+{
+    delete static_cast<CXNotifyingDataObject*>(p);
+    return 0;
+}
+}
+
 STDMETHODIMP_(ULONG) CXNotifyingDataObject::Release( )
 {
     ULONG nRefCnt =
@@ -71,10 +88,9 @@ STDMETHODIMP_(ULONG) CXNotifyingDataObject::Release( )
 
     if ( 0 == nRefCnt )
     {
-        if ( m_pWinClipImpl )
-            m_pWinClipImpl->onReleaseDataObject( this );
-
-        delete this;
+        auto handle = _beginthreadex(nullptr, 0, releaseAsyncProc, this, 0, 
nullptr);
+        assert(handle);
+        CloseHandle(reinterpret_cast<HANDLE>(handle));
     }
 
     return nRefCnt;
diff --git a/vcl/win/dtrans/XNotifyingDataObject.hxx 
b/vcl/win/dtrans/XNotifyingDataObject.hxx
index 408413a5de17..8a70024d2a0f 100644
--- a/vcl/win/dtrans/XNotifyingDataObject.hxx
+++ b/vcl/win/dtrans/XNotifyingDataObject.hxx
@@ -47,7 +47,7 @@ public:
         const css::uno::Reference< 
css::datatransfer::clipboard::XClipboardOwner >& aXClipOwner,
         CWinClipboard* const theWinClipoard);
 
-    virtual ~CXNotifyingDataObject() {}
+    virtual ~CXNotifyingDataObject();
 
     // ole interface implementation

core.git: Branch 'distro/vector/vector-24.2' - vcl/win

Reply via email to