[Libreoffice-commits] core.git: dtrans/source include/vcl toolkit/source

[Tomaž Vajngerl]

 dtrans/source/win32/dtobj/XTDataObject.cxx |   33 +++++++++++++++++++++++++++++
 dtrans/source/win32/dtobj/XTDataObject.hxx |    3 +-
 include/vcl/svapp.hxx                      |    4 +++
 toolkit/source/awt/asynccallback.cxx       |    3 --
 4 files changed, 40 insertions(+), 3 deletions(-)

New commits:
commit bdd108cd72e630189c360c5327c480c1d64d55b1
Author: Tomaž Vajngerl <tomaz.vajngerl at collabora.co.uk>
Date:   Wed Nov 23 17:01:46 2016 +0100

    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
    
    Change-Id: I4237a1cf380e8be66b3eefc393a58bb4853bf4e1
    Reviewed-on: https://gerrit.libreoffice.org/31126
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Tomaž Vajngerl <quikee at gmail.com>

diff --git a/dtrans/source/win32/dtobj/XTDataObject.cxx b/dtrans/source/win32/dtobj/XTDataObject.cxx
index 3ca9325..1c13d34 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)
@@ -82,6 +85,28 @@ void SAL_CALL setupStgMedium( const FORMATETC& fetc,
         OSL_ASSERT( false );
 }
 
+/**
+   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
@@ -174,12 +199,20 @@ CXTDataObject::CXTDataObject( const Reference< XComponentContext >& rxContext,
                               const Reference< XTransferable >& aXTransferable )
     : m_nRefCnt( 0 )
     , m_XTransferable( aXTransferable )
+    , m_XComponentContext( rxContext )
     , m_bFormatEtcContainerInitialized( 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 8bf3c10..db3496d 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
 
@@ -103,6 +103,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;
     bool                                                         m_bFormatEtcContainerInitialized;
     CDataFormatTranslator                                        m_DataFormatTranslator;
diff --git a/include/vcl/svapp.hxx b/include/vcl/svapp.hxx
index 2719224..9b930b3 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 95467f9..3860207 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 );
     }