https://bugs.documentfoundation.org/show_bug.cgi?id=173208

--- Comment #8 from Branislav Klocok <[email protected]> ---
This bug can now be reproduced without any extension, without crash recovery,
and without the
startup path described in comment 0. Two concurrent UNO calls against a plain
office instance are
enough, and I am attaching a script that does it.

The mechanism is a lock-order inversion between the SolarMutex and the private
mutex of
ModuleUIConfigurationManagerSupplier.

In framework/source/uiconfiguration/moduleuicfgsupplier.cxx,
ModuleUIConfigurationManagerSupplier::getUIConfigurationManager() takes the
supplier's own
std::mutex and holds it across the whole body, including the call to
ModuleUIConfigurationManager::createDefault(). The constructor of
ModuleUIConfigurationManager, in
framework/source/uiconfiguration/moduleuiconfigurationmanager.cxx, acquires the
SolarMutex. So a
thread that asks the supplier for a manager that has not been built yet takes
the supplier mutex
first and the SolarMutex second.

The opposite order exists in the document loading path.
LayoutManager::implts_reset() in
framework/source/layoutmanager/layoutmanager.cxx calls
getUIConfigurationManager() when the module
of a frame changes, and it runs while the loading thread holds the SolarMutex.
That thread
therefore takes the SolarMutex first and wants the supplier mutex second.

Because the supplier keeps one mutex for the whole module map, the two sides do
not even have to
be interested in the same module. Asking for any module whose manager has not
been constructed yet
is enough to block every other caller of getUIConfigurationManager().

Here is the state of the hung process, captured with gdb while both calls were
stuck. One worker is
inside the constructor holding the supplier mutex and waiting for the
SolarMutex, another worker
holds the SolarMutex and waits on a mutex it never gets, and the main thread
never gets the
SolarMutex at all:

  Thread A (cppu_threadpool)  __lll_lock_wait -> pthread_mutex_lock ->
osl_acquireMutex
                              -> comphelper::SolarMutex::doAcquire
                              ->
com_sun_star_comp_framework_ModuleUIConfigurationManager_get_implementation
  Thread B (cppu_threadpool)  __lll_lock_wait -> pthread_mutex_lock ->
libmergedlo
  Thread 1 (main)             Application::Execute -> Application::Yield ->
QtInstance::ImplYield
                              -> pthread_cond_wait

The attached script starts its own office with a throwaway user profile on a
private port, connects
over URP, and then issues loadComponentFromURL together with a supplier lookup
for a different
module a few tens of milliseconds later. It walks through a handful of modules
and delays because
the collision is a race. In my runs the office deadlocked in every attempt of a
three round series,
though not always at the same step of the sequence. Tested on 26.8.0.3 on
openSUSE Tumbleweed,
Plasma 6 Wayland, Qt6 VCL plugin. The code in question reads the same on master
and on
libreoffice-26-8, and the same construction under the supplier's own lock is
present as far back as
5.4, so this does not look like a regression.

Two corrections to what I filed earlier, both of which matter for triage.

The first is the precondition. Condition (a) in comment 0,
RecoveryInfo/Crashed=true, is not
required. Comment 6 already reported a second occurrence without it, and the
reproducer above does
not touch recovery at all.

The second is the blocking call. My original backtrace pointed at a pyuno
thread in
Desktop.getCurrentComponent(), and I built the summary of this bug around it.
That thread was
queued behind the deadlock rather than part of it. The cycle described above
was first identified
by the maintainer of the extension I had originally blamed, when he reproduced
the same hang in his
own test harness; I then verified the two lock orders in the LibreOffice
sources and built the
extension free reproducer attached here. The extension has since changed so
that it no longer calls
UNO from its own threads during startup, which removes its trigger, but the
inversion inside
framework is untouched and any UNO client can still hit it.

Given that, the current summary of this bug is misleading. I would like to
change it to something
like "Deadlock between SolarMutex and the ModuleUIConfigurationManagerSupplier
mutex: the supplier
constructs ModuleUIConfigurationManager while holding its own lock". If you
prefer a different
wording, or would rather keep the history and open a separate report, say so
and I will follow that.

As for a fix, the straightforward one is to stop constructing under the
supplier's own lock: release
it before ModuleUIConfigurationManager::createDefault(), take it again
afterwards, and drop the
instance if another thread won the race in the meantime. That keeps the map
consistent and removes
the inner SolarMutex acquisition from the locked region.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Reply via email to