diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2020-06-23 14:53:02 +0200 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2020-06-23 16:39:58 +0200 |
commit | ba86099d3c4804cc7e0958c9a89fbdee29456ecf (patch) | |
tree | 2b8aadd65d697d1a08520daf2111f8aedcf241b9 /binaryurp/source | |
parent | 4c7a3286fd05b1939acb45b2333c1ee927b0d4db (diff) |
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@redhat.com>
Diffstat (limited to 'binaryurp/source')
-rw-r--r-- | binaryurp/source/bridge.cxx | 21 |
1 files changed, 19 insertions, 2 deletions
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 >()); } |