Assertion failure: IsEmpty(), at obj-x86_64-pc-linux-gnu/dist/include/mozilla/Queue.h:57
Categories
(Core :: XPCOM, defect)
Tracking
()
People
(Reporter: allstars.chh, Unassigned)
References
Details
Attachments
(1 file)
1.66 KB,
patch
|
Details | Diff | Splinter Review |
When I dispatch task to XPCOM thread pool, I met an assertion in shutdown.
Please apply my attachment for how to reproduce it.
- apply my attachment and build with debug enabled
- ./mach run to launch the browser
- close the browser
then you could find the assertion
The detail is,
In startHandlingCompressionTask I dispatch a task to XPCOM thread pool
https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/js/src/vm/HelperThreads.cpp#1677
(I submit the task in
https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/js/src/gc/GC.cpp#4137 for simplicity)
then nsThreadPool::Shutdown is called,
but the nsThreadPool::mEvents still has one task not dispatched yet.
then ~nsThreadPool calls ~EventQueue, ~EventQueueInternal, finally ~Queue
The IsEmpty() is false triggers the assertion.
https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/xpcom/threads/Queue.h#57
The stack trace is in https://pastebin.com/XK3P3SSr
SM would call CancelOffThreadComression() in shutdown, but the task has already been dispatched.
https://searchfox.org/mozilla-central/source/js/src/vm/Runtime.cpp#272
Kris, can you suggest what's the correct behavior for those RunableTasks which have been dispatched to XPCOM thread pool?
If they should be discarded, can SpiderMonkey knows they are discarded?
Thanks
Reporter | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
This is asserting because it still has something in its event queue, which should not be happening, so something is going wrong between the last batch of tasks getting enqueued and the thread pool being shut down. It's possible that nsThreadPool is somehow getting stuck after dispatch, and I don't see anywhere in nsThreadPool's shutdown routine where it makes certain that its event queue was emptied, because it assumes there are threads to process the event queue. There's no way these tasks themselves are racing, so I know this has to be a problem with the EventQueue being unable to create new threads to service tasks.
When I looked at the dispatches sent at shutdown it looks like HelperThreadPool
is receiving dispatches after nsThreadManager
has cleaned up the nsThreads for shutdown. If there are no new threads, then an nsThreadPool can't spin up new threads to process work. I've seen shutdown races from this in nsThread before where either a) a thread still processing events refuses to shut down or b) a thread pool refuses to shut down because it's still got work sitting in its event queue, and we look to have a problem with the latter.
I think the best way to fix this is to either handle shutdown tasks synchronously or dispatch them to something internal to Spidermonkey. We could signal to XPCJSRuntime
to drop the callback before nsThreadManager
can shut down or we can indicate to HelperThreadPool
in some way to stop accepting new dispatches during shutdown.
Comment 2•5 years ago
|
||
(In reply to Kris Wright :KrisWright from comment #1)
If SpiderMonkey is going to use the XPCOM thread pool then we need to make sure the thread pool is not shut down until after the JS engine. Kris, is there a way to make that happen?
Comment 3•5 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #2)
(In reply to Kris Wright :KrisWright from comment #1)
If SpiderMonkey is going to use the XPCOM thread pool then we need to make sure the thread pool is not shut down until after the JS engine. Kris, is there a way to make that happen?
This is a bit complicated, because the thread manager shuts down before XPCOM enters shutdown mode, and JS shuts down after the final shutdown CC. Simply telling threads to shut down after JS_ShutDown
causes other weird issues with the CC since nsThreads shouldn't be active at this point, and since I'm not super familiar with the CC I'm not sure where the problems are coming from. While the thread pool itself isn't shut down, it can't make a thread to service its event queue either so this will keep happening until the underlying issue is addressed. Off the top of my head I can think of a few potential solutions:
- Split up
nsThreadManager::Shutdown
into two stages, and don't reject the creation of new threads to service Spidermonkey until after JS shutdown - Have
nsThreadManager
handle the Spidermonkey thread pool, and somehow deal with post-shutdown dispatches after nsThreadManager shuts down - Differentiate between one-off threads and threads belonging to an
nsThreadPool
, and tellnsThreadManager
to letnsThreadPool
deal with its own threads' shutdown whennsThreadPool::ShutDown()
is called
I also filed bug 1633548 about the event queues, since this is the second time I've come across this kind of shutdown hang.
CC'ing Nathan in case he has any thoughts when he gets back.
Reporter | ||
Comment 5•5 years ago
|
||
I'll try Kris' suggestions from comment 3.
Reporter | ||
Comment 6•5 years ago
|
||
I tried shutdown the thread after JS_Shutdown, but found when nsThread::Shutdown is called, it will need CC to run some FreeSnowWhite, but CC has been shutdown and causes assertion.
Comment 7•5 years ago
|
||
(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #6)
It's trying to run the AsyncFreeSnowWhite runnable (part of the CC) after the CC has been shutdown. I don't think this should be necessary at this point . It is dispatched in XPCJSRuntime::DispatchDeferredDeletion. Maybe you could check what is causing this to happen?
Reporter | ||
Comment 8•5 years ago
•
|
||
I filed a separate bug for addressing Kris' comment 3, bug 1648776.
As I thought this bug and bug 1633548 filed by Kris, are focus on the problems there are still some Runnable left in the event queue while nsThreadPool is shutdown. Also my comment 0 no longer applies because I landed Bug 1634720 later.
And comment 2 and comment 3 are discussing we need a thread pool with available threads until JS has been shutdown.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 9•4 years ago
|
||
We're trying to dispatch to TaskController now
Description
•