Closed Bug 1334480 Opened 8 years ago Closed 8 years ago

Crash in mozilla::dom::workers::ServiceWorkerRegistrationInfo::MaybeScheduleTimeCheckAndUpdate

Categories

(Core :: DOM: Service Workers, defect)

x86
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox-esr45 --- wontfix
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

+++ 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.
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)
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
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?
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
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+
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: