This bug was filed from the Socorro interface and is report bp-64ffbe71-a83d-42c9-be45-c676c0180313. ============================================================= Top 10 frames of crashing thread: 0 xul.dll mozilla::dom::ServiceWorkerPrivate::StoreISupports dom/serviceworkers/ServiceWorkerPrivate.cpp:1959 1 xul.dll mozilla::dom::`anonymous namespace'::SWRUpdateRunnable::Run dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp:386 2 xul.dll mozilla::ThrottledEventQueue::Inner::ExecuteRunnable xpcom/threads/ThrottledEventQueue.cpp:193 3 xul.dll mozilla::ThrottledEventQueue::Inner::Executor::Run xpcom/threads/ThrottledEventQueue.cpp:79 4 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1040 5 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:97 6 xul.dll mozilla::ipc::MessagePumpForChildProcess::Run ipc/glue/MessagePump.cpp:301 7 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:319 8 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:299 9 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:157 ============================================================= this is a low volume crash signature starting to show up at the start of the 60.0b cycle with MOZ_RELEASE_ASSERT(mWorkerPrivate).

This is almost assuredly fallout from bug 1432846. The mWorkerPrivate in the SWP is cleared when TerminateWorker is called, even if the service worker thread is still held alive. So the check for if mPromiseProxy has cleaned up is not adequate to ensure that StoreISupports() is safe to call. Probably the easiest fix is to make a MaybeStoreISupports() that does nothing if the mWorkerPrivate is nullptr.

I'm going to steal this.

Catalin, this patch makes StoreISupports fallible. If we can't store the timer we just cancel it immediately and let the update destruct due to de-ref.

Comment on attachment 8961391 [details] [diff] [review] Make service worker self-update fallibly store its timer on the private and cleanup if that fails. r=catalinb Review of attachment 8961391 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for fixing this.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8d0bff47a499 Make service worker self-update fallibly store its timer on the private and cleanup if that fails. r=catalinb

Is this worth a backport to Beta?

Sure.

Comment on attachment 8961391 [details] [diff] [review] Make service worker self-update fallibly store its timer on the private and cleanup if that fails. r=catalinb Approval Request Comment [Feature/Bug causing the regression]: Bug 1432846 [User impact if declined]: Low frequency assertion. In beta/release the user may see other crashes or leaks if this condition is hit. [Is this code covered by automated tests?]: Its a race condition, so we don't really have tests for it. We have overall service worker regression tests. [Has the fix been verified in Nightly?]: Its landed in nightly. [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: I don't think so. [Is the change risky?]: Low risk. [Why is the change risky/not risky?]: This patch makes a method that expects a certain state instead perform a runtime check for that state. This should be relatively safe. [String changes made/needed]: None

Comment on attachment 8961391 [details] [diff] [review] Make service worker self-update fallibly store its timer on the private and cleanup if that fails. r=catalinb service worker fix, beta60+, should be in 60.0b8