summaryrefslogtreecommitdiff
path: root/binaryurp/source
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2020-06-23 14:53:02 +0200
committerStephan Bergmann <sbergman@redhat.com>2020-06-23 16:39:58 +0200
commitba86099d3c4804cc7e0958c9a89fbdee29456ecf (patch)
tree2b8aadd65d697d1a08520daf2111f8aedcf241b9 /binaryurp/source
parent4c7a3286fd05b1939acb45b2333c1ee927b0d4db (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.cxx21
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 >());
}