Closed Bug 1331656 Opened 3 years ago Closed 3 years ago

Crash in mozilla::dom::workers::ServiceWorkerManager::ServiceWorkerManager

Categories

(Core :: DOM: Service Workers, defect, critical)

x86
Windows 8
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox-esr45 --- disabled
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-16cf0d7c-7e8a-4d17-b79a-0c4682170115.
=============================================================

I *think* this crash might be due to BackgroundChild::GetOrCreateForCurrentThread(this) failing, although its a bit mysterious.  It seems like some of the stack is elided or inlined for some reason.

Anyway, lets move GetOrCreateForCurrentThread() to Init() and handle its failure accordingly.
Andrea, this tries to make ServiceWorkerManager better handle PBackground initialization failure.  While PBackground crashes content processes in this case, its possible for us to hit this path in the parent process or in non-e10s mode.  (Like the crash we see above.)  I'm not 100% sure this will fix the crash, but its the best I can come up with here and seems reasonable.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a9eb69fe843a095734fd5b53d4660987f849ebf
Attachment #8827586 - Flags: review?(amarchesini)
Attachment #8827586 - Flags: review?(amarchesini) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/191e51321f2d
Make ServiceWorkerManager handle PBackground init failure. r=baku
Comment on attachment 8827586 [details] [diff] [review]
Make ServiceWorkerManager handle PBackground init failure. r=baku

Approval Request Comment
[Feature/Bug causing the regression]: Service workers
[User impact if declined]: Low frequency crashes.  Probably limited to shutdown crashes.
[Is this code covered by automated tests?]: Our automated tests do not trigger this crash.
[Has the fix been verified in Nightly?]: We don't have steps to reproduce, so it cannot be manually verified.
[Needs manual test from QE? If yes, steps to reproduce]:  No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Minimal risk
[Why is the change risky/not risky?]: This patch attempts to fix the problem by not assuming PBackground successfully initializes.  Instead we handle the PBackground init failure using an existing failure path.  It should have no functional change in the typical success path.
[String changes made/needed]: None
Attachment #8827586 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/191e51321f2d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8827586 [details] [diff] [review]
Make ServiceWorkerManager handle PBackground init failure. r=baku

possible crash fix in ServiceWorkerManager, aurora52+
Attachment #8827586 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.