Closed Bug 1546500 Opened 5 months ago Closed 5 months ago

Don't dispatch sync thread Shutdown runnables

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: kmag, Assigned: kmag)

Details

Attachments

(1 file)

There's no reason for thread shutdown runnables to use the synchronous Shutdown method. Doing so creates a nested event loop that waits for the thread to finish shutting down, which serves no purpose when the caller isn't waiting for the call to finish, and can lead to multiple levels of nested event loops in some circumstances.

https://hg.mozilla.org/integration/mozilla-inbound/rev/42e6d724f90fd315967aa06ddfd2afbb88e177fd
Bug 1546500: Avoid dispatching synchronous thread shutdown runnables. r=froydnj

Comment on attachment 9060219 [details]
Bug 1546500: Avoid dispatching synchronous thread shutdown runnables. r=froydnj

Beta/Release Uplift Approval Request

  • User impact if declined: There are a number of shutdown crashes with multiple levels of nested event loops from synchronous thread shutdown runnables. There's a good chance that this patch will fix those crashes, and if it doesn't, it will at least give us better insight into the cause.
  • Is this code covered by automated tests?: Unknown
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): There should be no significant change in behavior from these patches except in the pathological cases that we're currently seeing in shutdown crashes.
  • String changes made/needed: None
Attachment #9060219 - Flags: approval-mozilla-beta?

Comment on attachment 9060219 [details]
Bug 1546500: Avoid dispatching synchronous thread shutdown runnables. r=froydnj

Not landed on mozilla-central yet but the tests on inbound are green and given that we go to build today and that we are nearing the end of the beta cycle, I am taking this patch in 67 beta 14 to give us some time to evaluate the impact on shutdown crashes. Thanks.

Attachment #9060219 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.