Closed
Bug 1334480
Opened 8 years ago
Closed 8 years ago
Crash in mozilla::dom::workers::ServiceWorkerRegistrationInfo::MaybeScheduleTimeCheckAndUpdate
Categories
(Core :: DOM: Service Workers, defect)
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: bkelly, Assigned: bkelly)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
asuth
:
review+
jcristau
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1329716 +++
This bug was filed from the Socorro interface and is
report bp-ad9c3775-7eca-410e-81c1-e513b2170106.
=============================================================
This is still happening for some reason:
https://crash-stats.mozilla.com/report/index/31ce71c2-90c4-4c6d-aa18-e99372170127
I think we should do the defensive nullptr checking for now. In theory something in WorkerRun() could clear mRegistration anyway, so it seems reasonable. I just can't see where its actually being cleared unfortunately.
Assignee | ||
Comment 1•8 years ago
|
||
This adds a nullptr check and an extra diagnostic assertion.
It also removes a shadowed mRegistration variable in the fetch subclass. Not sure if that is directly related, but its unnecessary and makes it harder to read.
Attachment #8831139 -
Flags: review?(bugmail)
Updated•8 years ago
|
Attachment #8831139 -
Flags: review?(bugmail) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/33b5dfbf99f4
Don't try to update a nullptr registration. r=asuth
Assignee | ||
Updated•8 years ago
|
status-firefox51:
--- → wontfix
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox-esr45:
--- → wontfix
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8831139 [details] [diff] [review]
Don't try to update a nullptr registration. r=asuth
Approval Request Comment
[Feature/Bug causing the regression]: Service workers
[User impact if declined]: Low frequency crashes
[Is this code covered by automated tests?]: Many tests, but they don't trigger this race condition.
[Has the fix been verified in Nightly?]: Race condition is difficult to manually verify, so no.
[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 just adds a nullptr check and a diagnostic assertion.
[String changes made/needed]: None
Attachment #8831139 -
Flags: approval-mozilla-beta?
Attachment #8831139 -
Flags: approval-mozilla-aurora?
Comment 4•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 5•8 years ago
|
||
Comment on attachment 8831139 [details] [diff] [review]
Don't try to update a nullptr registration. r=asuth
null-check mRegistration in ServiceWorkerPrivate, for aurora53 and beta52
Attachment #8831139 -
Flags: approval-mozilla-beta?
Attachment #8831139 -
Flags: approval-mozilla-beta+
Attachment #8831139 -
Flags: approval-mozilla-aurora?
Attachment #8831139 -
Flags: approval-mozilla-aurora+
Comment 6•8 years ago
|
||
bugherder uplift |
Comment 7•8 years ago
|
||
bugherder uplift |
Updated•8 years ago
|
Flags: qe-verify-
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•