PollJSSampling holds the per-thread profiler data lock across a blocking JS-engine call, deadlocking the sampler during worker JSContext teardown
Categories
(Core :: Gecko Profiler, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox155 | --- | fixed |
People
(Reporter: florian, Assigned: florian, NeedInfo)
References
Details
(Whiteboard: [fp])
Attachments
(1 file)
When the Gecko profiler samples with the js feature, a DOM Worker tearing down its JSContext can deadlock the whole process.
profiler_clear_js_context() calls PollJSSamplingForCurrentThread(), which takes the thread's ThreadRegistration RW data lock (via WithLockedRWOnThread) and, while holding it, calls PollJSSampling() -> js::EnableContextProfilingStack(cx, false) -> GeckoProfilerRuntime::enable(false) -> ReleaseAllJITCode -> CancelOffThreadIonCompile, which blocks waiting for off-thread Ion compilation to drain.
Meanwhile the SamplerThread holds the global profiler mutex (gPSMutex) and blocks acquiring that same thread's RW data lock (GetLockedRWFromAnyThread). Every other thread that needs gPSMutex to register/unregister -- including the JS helper thread that would let the Ion-compile cancel finish -- is then stuck. Circular wait:
- worker: holds its RW data lock -> waits on JS helper threads
- sampler: holds
gPSMutex-> waits on the worker's RW data lock - helper/register threads: wait on
gPSMutex
The code already deliberately drops gPSMutex before the JS-engine call ("Drop profiler mutex for call into JS engine"), but doesn't extend that discipline to the per-thread RW data lock.
browser/components/qrcode/test/xpcshell/test_QRCodeWorker.js reliably reproduces this on try (Windows xpcshell). On its own such a timeout is just retried as an intermittent, so it is invisible today. Bug 2052432 (upload a minidump for a force-killed hung xpcshell process on timeout) reports the force-killed process's dump as a failure, turning this deadlock into a perma-fail -- which is why that patch was backed out.
Candidate fixes:
- A: do the JS-sampling state bookkeeping under the RW lock, but perform the
js::Enable*calls without it (root cause; no lost samples). - B: make the sampler
TryLockthe per-thread data and skip a thread whose lock is held (contained; drops the occasional stack sample). - C: make disabling JS profiling not synchronously cancel off-thread Ion compiles (SpiderMonkey side).
|
Assignee | |
Comment 1•19 days ago
|
||
A DOM Worker clearing its JSContext ran PollJSSampling() under its
ThreadRegistration data lock, and PollJSSampling calls into the JS engine
(js::EnableContextProfilingStack -> ReleaseAllJITCode ->
CancelOffThreadIonCompile), which blocks on the JS helper threads. The
SamplerThread holds gPSMutex and blocks acquiring that same data lock, so every
thread that needs gPSMutex to register/unregister -- including the helper the
worker waits on -- is stuck. Circular wait.
Split PollJSSampling into TakeJSSamplingChange() (flips mJSSampling under the
data lock) and ApplyJSSamplingChange() (makes the js::Enable* calls). The
current-thread poll path now takes the data lock only for the state flip and
applies the JS-engine change with no lock held, mirroring the existing 'drop
the global profiler mutex before calling into the JS engine' discipline.
|
||
Updated•19 days ago
|
|
||
Updated•9 days ago
|
|
||
Updated•9 days ago
|
|
||
Comment 4•8 days ago
|
||
Backed out for causing failures at JitScript.h.
Backout link: https://hg.mozilla.org/integration/autoland/rev/d10a24ae44ea
|
||
Comment 6•6 days ago
|
||
| bugherder | ||
|
|
||
Comment 7•6 days ago
|
||
| bugherder | ||
|
|
||
Comment 8•6 days ago
|
||
| bugherder | ||
|
|
||
Comment 9•6 days ago
|
||
| bugherder | ||
Description
•