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

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

+++ 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.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Don't try to update a nullptr registration. r=asuth

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.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/33b5dfbf99f4
Don't try to update a nullptr registration. r=asuth

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

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

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/33b5dfbf99f4

Comment 5

[Julien Cristau [:jcristau]]

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

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1d9713abc2cf

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0f54c5ef73d6