[Libreoffice-commits] core.git: binaryurp/source

[Stephan Bergmann (via logerrit)]

 binaryurp/source/bridge.cxx |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

New commits:
commit ba86099d3c4804cc7e0958c9a89fbdee29456ecf
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Jun 23 14:53:02 2020 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Jun 23 16:39:58 2020 +0200

    HACK to decouple URP release calls from all other threads
    
    Abandoned <b9ecec7c74687ed5a9470cffb7d02e0e6e83107e> "Don't call out to UNO with
    SolarMutex locked" documents a deadlock where a synchronous
    documentEventOccurred call made with SolarMutex locked evokes an asynchronous
    release call back (serviced on a different physical thread, but which blocks the
    original thread) that then wants to acquire the SolarMutex.  While we usually
    appear to get away with wrapping those UNO calls in SolarMutexReleaser (though
    knowing all too well that that is nothing but a bad hack that may well cause
    crashes and deadlocks at least in theory), the place in
    SfxBaseModel::postEvent_Impl was obviously too sensitive for that hack:  It did
    cause enough different crashes (e.g., hitting
    
      assert(pSchedulerData == pMostUrgent);
    
    in Scheduler::ProcessTaskScheduling, vcl/source/app/scheduler.cxx) and deadlocks
    (e.g., different threads now taking the SolarMutex and JobExecutor's mutex in
    framework/source/jobs/jobexecutor.cxx in different orders) to make me search for
    a different "fix", so I came up with this hack instead.
    
    Change-Id: Icd26926279cb86ce529edb4544a3ec0bc9a8b108
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96946
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/binaryurp/source/bridge.cxx b/binaryurp/source/bridge.cxx
index 99e6cafd6f9f..2534dfa1a873 100644
--- a/binaryurp/source/bridge.cxx
+++ b/binaryurp/source/bridge.cxx
@@ -976,9 +976,26 @@ void Bridge::sendProtPropRequest(
 void Bridge::makeReleaseCall(
     OUString const & oid, css::uno::TypeDescription const & type)
 {
-    AttachThread att(getThreadPool());
+    //HACK to decouple the processing of release calls from all other threads.  Normally, sending
+    // the release request should use the current thread's TID (via AttachThread), so that that
+    // asynchronous request would be processed by a physical thread that is paired with the physical
+    // thread processing the normal synchronous call stack (see ThreadIdHashMap in
+    // cppu/source/threadpool/threadpool.hxx).  However, that can lead to deadlock when a thread
+    // illegally makes a synchronous UNO call with the SolarMutex locked (e.g.,
+    // SfxBaseModel::postEvent_Impl in sfx2/source/doc/sfxbasemodel.cxx doing documentEventOccurred
+    // and notifyEvent calls), and while that call is on the stack the remote side sends back some
+    // release request on the same logical UNO thread for an object that wants to acquire the
+    // SolarMutex in its destructor (e.g., SwXTextDocument in sw/inc/unotxdoc.hxx holding its
+    // m_pImpl via an sw::UnoImplPtr).  While the correct approach would be to not make UNO calls
+    // with the SolarMutex (or any other mutex) locked, fixing that would probably be a heroic
+    // effort.  So for now live with this hack, hoping that it does not introduce any new issues of
+    // its own:
+    static auto const tid = [] {
+            static sal_Int8 const id[] = {'r', 'e', 'l', 'e', 'a', 's', 'e', 'h', 'a', 'c', 'k'};
+            return rtl::ByteSequence(id, SAL_N_ELEMENTS(id));
+        }();
     sendRequest(
-        att.getTid(), oid, type,
+        tid, oid, type,
         css::uno::TypeDescription("com.sun.star.uno.XInterface::release"),
         std::vector< BinaryAny >());
 }