tdf#103852 avoid clipboard deadlock

Deallocate the XTransferable object async using AsyncCallback
(that uses Application::PostUserEvent) which executes the
callback in a thread-safe way on the main thread. This avoids
a deadlock at deallocation so that the XTransferable.

Modify AsyncCallback to not hold the SolarMutexGuard because
Application::PostUserEvent is considered thread-safe.

Document Application::PostUserEvent thread-safety

Reviewed-on: https://gerrit.libreoffice.org/31126
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
(cherry picked from commit bdd108cd72e630189c360c5327c480c1d64d55b1)

Change-Id: I4237a1cf380e8be66b3eefc393a58bb4853bf4e1
Reviewed-on: https://gerrit.libreoffice.org/31168
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-by: Michael Stahl <mstahl@redhat.com>

4 files changed, 45 insertions, 3 deletions

diff --git a/dtrans/source/win32/dtobj/XTDataObject.cxx b/dtrans/source/win32/dtobj/XTDataObject.cxx
index 51108a16c925..993574196a4f 100644
--- a/dtrans/source/win32/dtobj/XTDataObject.cxx
+++ b/dtrans/source/win32/dtobj/XTDataObject.cxx
@@ -25,8 +25,11 @@
 #include "DTransHelper.hxx"
 #include "TxtCnvtHlp.hxx"
 #include <com/sun/star/datatransfer/clipboard/XClipboardEx.hpp>
+#include "com/sun/star/awt/AsyncCallback.hpp"
+#include "com/sun/star/awt/XCallback.hpp"
 #include "FmtFilter.hxx"
 #include <comphelper/processfactory.hxx>
+#include <cppuhelper/implbase.hxx>
 
 #if defined _MSC_VER
 #pragma warning(push,1)
@@ -52,6 +55,33 @@ using namespace com::sun::star::datatransfer::clipboard;
 using namespace com::sun::star::uno;
 using namespace com::sun::star::lang;
 
+namespace
+{
+
+/**
+   We need to destroy XTransferable in the main thread to avoid dead lock
+   when locking in the clipboard thread. So we transfer the ownership of the
+   XTransferable reference to this object and release it when the callback
+   is executed in main thread.
+*/
+class AsyncDereference : public cppu::WeakImplHelper<css::awt::XCallback>
+{
+    Reference<XTransferable> maTransferable;
+
+public:
+    AsyncDereference(css::uno::Reference<css::datatransfer::XTransferable> const & rTransferable)
+        : maTransferable(rTransferable)
+    {}
+
+    virtual void SAL_CALL notify(css::uno::Any const &)
+        throw (css::uno::RuntimeException, std::exception) override
+    {
+        maTransferable.set(nullptr);
+    }
+};
+
+}
+
 // a helper class that will be thrown by the function validateFormatEtc
 
 class CInvalidFormatEtcException
@@ -67,12 +97,20 @@ CXTDataObject::CXTDataObject( const Reference< XComponentContext >& rxContext,
                               const Reference< XTransferable >& aXTransferable )
     : m_nRefCnt( 0 )
     , m_XTransferable( aXTransferable )
+    , m_XComponentContext( rxContext )
     , m_bFormatEtcContainerInitialized( sal_False )
     , m_DataFormatTranslator( rxContext )
     , m_FormatRegistrar( rxContext, m_DataFormatTranslator )
 {
 }
 
+CXTDataObject::~CXTDataObject()
+{
+    css::awt::AsyncCallback::create(m_XComponentContext)->addCallback(
+        new AsyncDereference(m_XTransferable),
+        css::uno::Any());
+}
+
 // IUnknown->QueryInterface
 
 STDMETHODIMP CXTDataObject::QueryInterface( REFIID iid, LPVOID* ppvObject )
diff --git a/dtrans/source/win32/dtobj/XTDataObject.hxx b/dtrans/source/win32/dtobj/XTDataObject.hxx
index 3d8d71c2c24a..f9e98e86ebd7 100644
--- a/dtrans/source/win32/dtobj/XTDataObject.hxx
+++ b/dtrans/source/win32/dtobj/XTDataObject.hxx
@@ -64,7 +64,7 @@ class CXTDataObject : public IDataObject
 public:
     CXTDataObject( const css::uno::Reference< css::uno::XComponentContext >& rxContext,
                    const css::uno::Reference< css::datatransfer::XTransferable >& aXTransferable );
-    virtual ~CXTDataObject() {}
+    virtual ~CXTDataObject();
 
     // ole interface implementation
 
@@ -118,6 +118,7 @@ private:
 private:
     LONG m_nRefCnt;
     css::uno::Reference< css::datatransfer::XTransferable >      m_XTransferable;
+    css::uno::Reference< css::uno::XComponentContext>            m_XComponentContext;
     CFormatEtcContainer                                          m_FormatEtcContainer;
     sal_Bool                                                     m_bFormatEtcContainerInitialized;
     CDataFormatTranslator                                        m_DataFormatTranslator;
diff --git a/include/vcl/svapp.hxx b/include/vcl/svapp.hxx
index 7d190b9f83fa..9f888736345b 100644
--- a/include/vcl/svapp.hxx
+++ b/include/vcl/svapp.hxx
@@ -776,9 +776,13 @@ public:
 
      User events allow for the deferreal of work to later in the main-loop - at idle.
 
+     Execution of the deferred work is thread-safe which means all the tasks are executed
+     serially, so no thread-safety locks between tasks are necessary.
+
      @param     rLink           Link to event callback function
      @param     pCaller         Pointer to data sent to the event by the caller. Optional.
      @param     bReferenceLink  If true - hold a VclPtr<> reference on the Link's instance.
+                                Taking the reference is guarded by a SolarMutexGuard.
 
      @return the event ID used to post the event.
     */
diff --git a/toolkit/source/awt/asynccallback.cxx b/toolkit/source/awt/asynccallback.cxx
index 2d30afedc206..9268cb558119 100644
--- a/toolkit/source/awt/asynccallback.cxx
+++ b/toolkit/source/awt/asynccallback.cxx
@@ -88,8 +88,7 @@ void SAL_CALL AsyncCallback::addCallback(const css::uno::Reference< css::awt::XC
 {
     if ( Application::IsInMain() )
     {
-        SolarMutexGuard aSolarGuard;
-
+        // NOTE: We don't need SolarMutexGuard here as Application::PostUserEvent is thread-safe
         CallbackData* pCallbackData = new CallbackData( xCallback, aData );
         Application::PostUserEvent( LINK( this, AsyncCallback, Notify_Impl ), pCallbackData );
     }