Open Bug 1573072 Opened 5 years ago Updated 2 years ago

nsThread shouldn't need to dispatch a task to be shutdown when used with a nsThreadPool

Categories

(Core :: XPCOM, defect)

defect

Tracking

()

People

(Reporter: jya, Unassigned)

References

(Regression)

Details

(Keywords: regression)

Issue encountered is described in bug 1567170.

When a nsThreadPool no longer needs a thread, it fires a task on the main thread to shutdown that thread (https://searchfox.org/mozilla-central/rev/ec806131cb7bcd1c26c254d25cd5ab8a61b2aeb6/xpcom/threads/nsThreadPool.cpp#151)

This was done because nsThread::Shutdown starts an EventLoop to ensure that there are no tasks pending on that nsThread and that shutdown as completed.
https://searchfox.org/mozilla-central/rev/ec806131cb7bcd1c26c254d25cd5ab8a61b2aeb6/xpcom/threads/nsThread.cpp#900.

However, when used in a nsThreadPool, the only time a nsThread gets to be shutdown is when that nsThread happens to now be unused by the nsThreadPool (though it is possible a tasks got redispatched by the last task running, but that can be easily checked).

This prevent the use of a nsThreadPool from code running continuously on the main thread as used by fuzzing in bug 1567170 without spinning a new event loop.

nsThreadPool doesn't always need to fire a new task to shutdown that thread, particularly if that thread was never used; nsThread::Shutdown doesn't need to unconditionally spin a new event loop if there are no tasks pending or if no task ever got queued.

This is not a regression; nsThreadPool has always worked this way.

Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Has Regression Range: --- → yes
Keywords: regression
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.