1224941 - Crash in ServiceWorkerManager's FireControllerChangeOnDocument()

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

While working on a service worker for my blog I ran into this crash.  It triggers when we update due to a navigation.

Assertion failure: w, at /Users/bkelly/devel/hg/mozilla-central/dom/workers/ServiceWorkerManager.cpp:3533
#01: nsRunnableMethodImpl<void (mozilla::dom::workers::ServiceWorkerManager::*)(mozilla::dom::workers::ServiceWorkerRegistrationInfo*), true, mozilla::dom::workers::ServiceWorkerRegistrationInfo*>::Run()[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2adbcdb]
#02: nsThread::ProcessNextEvent(bool, bool*)[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0xf4379]
#03: NS_ProcessPendingEvents(nsIThread*, unsigned int)[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x13691f]
#04: nsBaseAppShell::NativeEventCallback()[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2d0ff74]
#05: nsAppShell::ProcessGeckoEvents(void*)[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2d7a16e]
#06: __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x7f5b1]

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Don't crash during ServiceWorker life cycle event dispatch if window is gone. r=baku

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Hmm, there may be more to this.  The attached patch avoided comment 0 crash, but then we hit this assert:

Assertion failure: aState == ServiceWorkerState::Redundant, at /Users/bkelly/devel/hg/mozilla-central/dom/workers/ServiceWorkerManager.cpp:4510
#01: mozilla::dom::workers::ContinueLifecycleRunnable::Run()[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2ae5628]
#02: nsThread::ProcessNextEvent(bool, bool*)[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0xf43b9]
#03: NS_ProcessPendingEvents(nsIThread*, unsigned int)[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x13695f]
#04: nsBaseAppShell::NativeEventCallback()[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2d0ff84]
#05: nsAppShell::ProcessGeckoEvents(void*)[/Users/bkelly/devel/hg/mozilla-central/obj-x86_64-apple-darwin13.4.0-debug/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2d7a17e]
#06: __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x7f5b1]
#07: __CFRunLoopDoSources0[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x70c62]
#08: __CFRunLoopRun[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x703ef]

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

I've done a lot more testing today and I haven't been able to reproduce the assert in comment 2.  I think maybe we should just add some more info to the assertion so we can investigate it the next time it happens.

The patch in comment 1 definitely helps avoid some frequent crashes in my testing, though.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb10bb0d6777

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8687700 [details] [diff] [review]
Don't crash during ServiceWorker life cycle event dispatch if window is gone. r=baku

Approval Request Comment
[Feature/regressing bug #]: Service workers
[User impact if declined]: Crashes under certain race conditions when installing a service worker.  Most noticeable if an update triggers during navigations.
[Describe test coverage new/current, TreeHerder]: Difficult to trigger in an automated test due to the race condition.  I did a lot of local testing with this patch and did not see the crash trigger again.
[Risks and why]: Minimal.  Only affects service workers.
[String/UUID change made/needed]: None.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/eb10bb0d6777

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8687700 [details] [diff] [review]
Don't crash during ServiceWorker life cycle event dispatch if window is gone. r=baku

Crash fix that was locally verified, seems safe to uplift to Aurora44.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2c5f51e5d0af