The crash occurs on Linux Debug builds right after an (otherwise successful) JS mochitest run. It happens on try, and I was able to reproduce it locally. Example on treeherder with the crash: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1835e1e733d4&selectedJob=15741981 Stack trace from `gdb` (with patches from bug 1212797 applied): $ ./mach mochitest devtools/client/aboutdebugging/test/browser_service_workers_timeout.js --debugger=gdb [...] #7 0x00007fffea8f785e in js::gc::GCRuntime::gcCycle (this=0x7fffdd436420, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DESTROY_RUNTIME) at /c/gecko-dev/js/src/jsgc.cpp:6349 #6 0x00007fffea8f6f78 in js::gc::GCRuntime::incrementalCollectSlice (this=0x7fffdd436420, budget=..., reason=JS::gcreason::DESTROY_RUNTIME) at /c/gecko-dev/js/src/jsgc.cpp:6158 #5 0x00007fffea8f52e3 in js::gc::GCRuntime::endSweepPhase (this=0x7fffdd436420, destroyingRuntime=true) at /c/gecko-dev/js/src/jsgc.cpp:5651 #4 0x00007fffea8eea8e in js::gc::GCRuntime::sweepZones (this=0x7fffdd436420, fop=0x7fffffffbb30, destroyingRuntime=true) at /c/gecko-dev/js/src/jsgc.cpp:3812 #3 0x00007fffea8ee741 in JS::Zone::sweepCompartments (this=0x7fffd9ff4000, fop=0x7fffffffbb30, keepAtleastOne=false, destroyingRuntime=true) at /c/gecko-dev/js/src/jsgc.cpp:3771 #2 0x00007fffea907438 in js_delete<JSCompartment> (p=0x7fffd9ea4800) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/js/Utility.h:375 #1 0x00007fffea8a1ddb in JSCompartment::~JSCompartment (this=0x7fffd9ea4800, __in_chrg=<optimized out>) at /c/gecko-dev/js/src/jscompartment.cpp:94 #0 0x00007fffea8c2443 in mozilla::LinkedList<js::UnboxedLayout>::~LinkedList (this=0x7fffd9ea4bd0, __in_chrg=<optimized out>) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/mozilla/LinkedList.h:328 Assertion failure: isEmpty(), at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/mozilla/LinkedList.h:328 Program received signal SIGSEGV, Segmentation fault. 0x00007fffea8c2443 in mozilla::LinkedList<js::UnboxedLayout>::~LinkedList (this=0x7fffd9ea4bd0, __in_chrg=<optimized out>) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/mozilla/LinkedList.h:328 328 ~LinkedList() { MOZ_ASSERT(isEmpty()); } It looks like the same symptom as many other bugs like 808379, 857050, 889857, 1141379, 1191259... and it currently prevents us (the devtools team) from getting ServiceWorker-related tests past try.

Nicolas, could you please have a look?

I will forward this issue to terrence.

Jan, Could you post a rebased/merged patch to ease testing this assertion?

[Tracking Requested - why for this release]: Blocking new Service Worker & Push features that we'd like to announce with Developer Edition 47. (In reply to Alexandre Poirot [:ochameau] from comment #3) > Jan, Could you post a rebased/merged patch to ease testing this assertion? Good idea, I will try to come up with a minimal test that reproduces this failure in the coming days.

(In reply to Jan Keromnes [:janx] from comment #4) > (In reply to Alexandre Poirot [:ochameau] from comment #3) > > Jan, Could you post a rebased/merged patch to ease testing this assertion? > > Good idea, I will try to come up with a minimal test that reproduces this > failure in the coming days. I didn't have time to do this yet and next week I will be on holidays. If nobody picks up this bug I'll try some things again when I come back. (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan)) > See Also: → bug 1246059 Actually that bug looks pretty similar, thank you for filing it! The stack trace looks a little bit different, and the crash happens in Firefox directly (for this bug here the crash happens at the end of a mochitest).

Here is more information about the crash: (gdb) thread [Current thread is 1 (Thread 0x7ffff7fcf780 (LWP 7459))] (gdb) where full #0 mozilla::LinkedList<js::UnboxedLayout>::~LinkedList (this=0x7fffd94e63d0) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/mozilla/LinkedList.h:328 No locals. #1 0x00007fffeb0712f8 in JSCompartment::~JSCompartment (this=0x7fffd94e6000) at /c/gecko-dev/js/src/jscompartment.cpp:115 rt = 0x7fffdca34000 #2 0x00007fffeb0af273 in js_delete<JSCompartment> (p=0x7fffd94e6000) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/js/Utility.h:375 No locals. #3 0x00007fffeb0b7e6b in JS::Zone::sweepCompartments (this=0x7fffd9577000, fop=0x7fffffffb780, keepAtleastOne=false, destroyingRuntime=true) at /c/gecko-dev/js/src/jsgc.cpp:3779 comp = 0x7fffd94e6000 dontDelete = false rt = 0x7fffdca34000 callback = 0x7fffe5941200 <CompartmentDestroyedCallback(JSFreeOp*, JSCompartment*)> read = 0x7fffb8e73028 end = 0x7fffb8e73270 write = 0x7fffb8e73000 foundOne = false #4 0x00007fffeb0b82c4 in js::gc::GCRuntime::sweepZones (this=0x7fffdca34428, fop=0x7fffffffb780, destroyingRuntime=true) at /c/gecko-dev/js/src/jsgc.cpp:3820 unlock = {lock = @0x7fffffffb5d8, _mCheckNotUsedAsTemporary = {mStatementDone = true}} zone = 0x7fffd9577000 lock = {runtime_ = 0x7fffdca34000, wasUnlocked_ = {value = true}, _mCheckNotUsedAsTemporary = {mStatementDone = true}} callback = 0x7fffe597b670 <XPCStringConvert::FreeZoneCache(JS::Zone*)> read = 0x7fffb8227618 end = 0x7fffb8227688 write = 0x7fffb8227608 #5 0x00007fffeb0bf603 in js::gc::GCRuntime::endSweepPhase (this=0x7fffdca34428, destroyingRuntime=true) at /c/gecko-dev/js/src/jsgc.cpp:5655 ap = {stats = @0x7fffdca34758, task = 0x0, phase = js::gcstats::PHASE_DESTROY, enabled = true} threadIsSweeping = {threadData_ = 0x7fffdca34010} ap = {stats = @0x7fffdca34758, task = 0x0, phase = js::gcstats::PHASE_SWEEP, enabled = true} fop = {<JSFreeOp> = {runtime_ = 0x7fffdca34000}, freeLaterList = {<js::SystemAllocPolicy> = {<No data fields>}, static kElemIsPod = true, static kMaxInlineBytes = 1024, static kInlineCapacity = 0, static kInlineBytes = 1, mBegin = 0x7fffffffb7a8, mLength = 0, mCapacity = 0, mReserved = 0, mStorage = {u = { mBytes = "", mDummy = 140737132101632}}, mEntered = false, static sMaxInlineStorage = 0}, jitPoisonRanges = {<js::SystemAllocPolicy> = {<No data fields>}, static kElemIsPod = false, static kMaxInlineBytes = 1024, static kInlineCapacity = 0, static kInlineBytes = 1, mBegin = 0x7fffffffb7d8, mLength = 0, mCapacity = 0, mReserved = 0, mStorage = {u = {mBytes = "", mDummy = 140737137141760}}, mEntered = false, static sMaxInlineStorage = 0}, threadType = js::MainThread} #6 0x00007fffeb0c17ff in js::gc::GCRuntime::incrementalCollectSlice (this=0x7fffdca34428, budget=..., reason=JS::gcreason::DESTROY_RUNTIME) at /c/gecko-dev/js/src/jsgc.cpp:6164 copy = {runtime = 0x7fffdca34000} slice = {runtime = 0x7fffdca34000} destroyingRuntime = true initialState = js::gc::NO_INCREMENTAL useZeal = false #7 0x00007fffeb0c235b in js::gc::GCRuntime::gcCycle (this=0x7fffdca34428, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DESTROY_RUNTIME) at /c/gecko-dev/js/src/jsgc.cpp:6355 notify = {gc_ = @0x7fffdca34428} session = {lock = {runtime = 0x7fffdca34000, _mCheckNotUsedAsTemporary = { mStatementDone = true}}, runtime = 0x7fffdca34000, prevState = JS::Idle, pseudoFrame = {profiler_ = 0x7fffdca37a60, sizeBefore_ = {value = 1}, _mCheckNotUsedAsTemporary = {mStatementDone = true}}} prevState = js::gc::NO_INCREMENTAL #8 0x00007fffeb0c2b33 in js::gc::GCRuntime::collect (this=0x7fffdca34428, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DESTROY_RUNTIME) at /c/gecko-dev/js/src/jsgc.cpp:6461 wasReset = false repeatForDeadZone = false logGC = {logger = 0x7ffff6b38de0, payload = {event = 0x7fff00000005, id = TraceLogger_GC}, isEvent = false, executed = false, prev = 0x0, _mCheckNotUsedAsTemporary = { mStatementDone = true}} av = {gc = 0x7fffdca34428, restartPreVerifier = false} aept = {gc_ = @0x7fffdca34428} asz = {rt_ = 0x7fffdca34000} agc = {stats = @0x7fffdca34758} repeat = false #9 0x00007fffeb0b5e66 in js::gc::GCRuntime::gc (this=0x7fffdca34428, gckind=GC_NORMAL, reason=JS::gcreason::DESTROY_RUNTIME) at /c/gecko-dev/js/src/jsgc.cpp:6519 No locals. #10 0x00007fffeb3db18e in JSRuntime::~JSRuntime (this=0x7fffdca34000) at /c/gecko-dev/js/src/vm/Runtime.cpp:416 oldCount = {value = 0} #11 0x00007fffeb052353 in js_delete<JSRuntime> (p=0x7fffdca34000) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/js/Utility.h:375 No locals. #12 0x00007fffeb052325 in JS_DestroyRuntime (rt=0x7fffdca34000) at /c/gecko-dev/js/src/jsapi.cpp:482 No locals. #13 0x00007fffe47d8ce9 in mozilla::CycleCollectedJSRuntime::~CycleCollectedJSRuntime ( this=0x7fffdf772800) at /c/gecko-dev/xpcom/base/CycleCollectedJSRuntime.cpp:496 No locals. #14 0x00007fffe5930153 in XPCJSRuntime::~XPCJSRuntime (this=0x7fffdf772800) at /c/gecko-dev/js/xpconnect/src/XPCJSRuntime.cpp:1673 rtPrivate = 0x7fffdca52000 #15 0x00007fffe592fbd9 in XPCJSRuntime::~XPCJSRuntime (this=0x7fffdf772800) at /c/gecko-dev/js/xpconnect/src/XPCJSRuntime.cpp:1609 No locals. #16 0x00007fffe59a2712 in nsXPConnect::~nsXPConnect (this=0x7fffdcb554c0) at /c/gecko-dev/js/xpconnect/src/nsXPConnect.cpp:99 No locals. #17 0x00007fffe59a2629 in nsXPConnect::~nsXPConnect (this=0x7fffdcb554c0) at /c/gecko-dev/js/xpconnect/src/nsXPConnect.cpp:71 No locals. #18 0x00007fffe59a24aa in nsXPConnect::Release (this=0x7fffdcb554c0) at /c/gecko-dev/js/xpconnect/src/nsXPConnect.cpp:39 count = 0 #19 0x00007fffe59a28da in nsXPConnect::ReleaseXPConnectSingleton () at /c/gecko-dev/js/xpconnect/src/nsXPConnect.cpp:147 cnt = 140737488338624 xpc = 0x7fffdcb554c0 #20 0x00007fffe5944ee9 in xpcModuleDtor () at /c/gecko-dev/js/xpconnect/src/XPCModule.cpp:22 No locals. #21 0x00007fffe9044722 in LayoutModuleDtor () at /c/gecko-dev/layout/build/nsLayoutModule.cpp:1397 No locals. #22 0x00007fffe48c094d in nsComponentManagerImpl::KnownModule::~KnownModule (this=0x7fffe13c5840) at /c/gecko-dev/xpcom/components/nsComponentManager.h:242 No locals. #23 0x00007fffe48c08ed in nsAutoPtr<nsComponentManagerImpl::KnownModule>::~nsAutoPtr ( this=0x7ffff6b104e0) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/nsAutoPtr.h:78 No locals. #24 0x00007fffe48c08b5 in nsTArrayElementTraits<nsAutoPtr<nsComponentManagerImpl::KnownModule> >::Destruct (aE=0x7ffff6b104e0) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/nsTArray.h:523 No locals. #25 0x00007fffe48c0856 in nsTArray_Impl<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayInfallibleAllocator>::DestructRange (this=0x7ffff6b736f0, aStart=0, aCount=65) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/nsTArray.h:2014 iter = 0x7ffff6b104e0 iend = 0x7ffff6b10610 #26 0x00007fffe48c07ba in nsTArray_Impl<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayInfallibleAllocator>::RemoveElementsAt (this=0x7ffff6b736f0, aStart=0, aCount=65) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/nsTArray.h:1656 No locals. #27 0x00007fffe48bcb55 in nsTArray_Impl<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayInfallibleAllocator>::Clear (this=0x7ffff6b736f0) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/nsTArray.h:1666 No locals. #28 0x00007fffe48b7a58 in nsComponentManagerImpl::Shutdown (this=0x7ffff6b735c0) at /c/gecko-dev/xpcom/components/nsComponentManager.cpp:940 No locals. #29 0x00007fffe492a846 in mozilla::ShutdownXPCOM (aServMgr=0x0) at /c/gecko-dev/xpcom/build/XPCOMInit.cpp:974 rv = NS_OK moduleLoaders = {mRawPtr = 0x0} #30 0x00007fffe492a195 in NS_ShutdownXPCOM (aServMgr=0x7ffff6b735c8) at /c/gecko-dev/xpcom/build/XPCOMInit.cpp:793 No locals. #31 0x00007fffe99dd69e in ScopedXPCOMStartup::~ScopedXPCOMStartup (this=0x7fffdf70b418) at /c/gecko-dev/toolkit/xre/nsAppRunner.cpp:1436 appStartup = {mRawPtr = 0x7fffd945c880} #32 0x00007fffe99eabfe in mozilla::DefaultDelete<ScopedXPCOMStartup>::operator() ( this=0x7fffffffc440, aPtr=0x7fffdf70b418) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/mozilla/UniquePtr.h:482 No locals. #33 0x00007fffe99eaae2 in mozilla::UniquePtr<ScopedXPCOMStartup, mozilla::DefaultDelete<ScopedXPCOMStartup> >::reset (this=0x7fffffffc440, aPtr=0x0) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/mozilla/UniquePtr.h:309 old = 0x7fffdf70b418 #34 0x00007fffe99ea7ed in mozilla::UniquePtr<ScopedXPCOMStartup, mozilla::DefaultDelete<ScopedXPCOMStartup> >::operator=(decltype(nullptr)) (this=0x7fffffffc440) at /c/gecko-dev/obj-x86_64-unknown-linux-gnu/dist/include/mozilla/UniquePtr.h:279 No locals. #35 0x00007fffe99e7211 in XREMain::XRE_main (this=0x7fffffffc410, argc=5, argv=0x7fffffffda08, aAppData=0x7fffffffc6b8) at /c/gecko-dev/toolkit/xre/nsAppRunner.cpp:4406 log = {<No data fields>} aLocal = 0 '\000' profilerGuard = {<No data fields>} sampler_raii4334 = {_mCheckNotUsedAsTemporary = {mStatementDone = true}, mHandle = 0x7ffff6bd6000} rv = NS_OK ioInterposerGuard = {<No data fields>} exit = false result = 0 appInitiatedRestart = false #36 0x00007fffe99e7974 in XRE_main (argc=5, argv=0x7fffffffda08, aAppData=0x7fffffffc6b8, aFlags=0) at /c/gecko-dev/toolkit/xre/nsAppRunner.cpp:4482 main = {mNativeApp = {mRawPtr = 0x7fffe133c320}, mProfileSvc = {mRawPtr = 0x7fffe1366830}, mProfD = {mRawPtr = 0x7fffe134b0c0}, mProfLD = {mRawPtr = 0x7fffe134b0c0}, mProfileLock = { mRawPtr = 0x7fffdf704ee0}, mRemoteService = {mRawPtr = 0x0}, mScopedXPCOM = { mTuple = {<mozilla::detail::PairHelper<ScopedXPCOMStartup*, mozilla::DefaultDelete<ScopedXPCOMStartup>, 1, 0>> = {<mozilla::DefaultDelete<ScopedXPCOMStartup>> = {<No data fields>}, mFirstA = 0x0}, <No data fields>}}, mAppData = {mRawPtr = 0x7ffff6b4d680}, mDirProvider = <error reading variable> result = 32767

See also bug 1232891. Shu, was that the bug that should be fixed by hueyfixing debugger objects (bug 1084626)? Someone needs to take this.

I bisected the crash by commenting out lines from my service worker registration related patches in bug 1212797, and it turns out that calling `RootActor.listServiceWorkerRegistrations()` caused its `ServiceWorkerRegistrationList` to call `ServiceWorkerManager.addListener(this)`, but `ServiceWorkerManager.removeListener(this)` was then never called until shutdown. My current patch fixes that. Leaving needinfo on Terrence FYI, because I'm not sure that a GC crash is the expected behavior even when we leak a listener. https://treeherder.mozilla.org/#/jobs?repo=try&revision=29117147ae93

Looks green enough.

(In reply to Jan Keromnes [:janx] from comment #8) > Created attachment 8719515 [details] [diff] [review] > Remove the RootActor's service worker registration listener on disconnect. > > I bisected the crash by commenting out lines from my service worker > registration related patches in bug 1212797, and it turns out that calling > `RootActor.listServiceWorkerRegistrations()` caused its > `ServiceWorkerRegistrationList` to call > `ServiceWorkerManager.addListener(this)`, but > `ServiceWorkerManager.removeListener(this)` was then never called until > shutdown. My current patch fixes that. > > Leaving needinfo on Terrence FYI, because I'm not sure that a GC crash is > the expected behavior even when we leak a listener. > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=29117147ae93 For chrome code, absolutely! When content code does something like that we have to deal with it, but there's no reason that we should have to re-do the GC and take a massive shutdown performance hit for a leak in code that is directly under our control.

Jan, could you please verify this issue is fixed as expected? Thanks!

FWIW, I'm still seeing intermittent crashes for browser_dbg_worker-window.js on Linux x64 debug, like in this try push I've done today: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0329274dba9c&selectedJob=17052433 That particular test shouldn't use the listener that we were leaking here.

I'm running into this problem again with bug 1119490. Unless I found a new way to leak listeners with those patches, there is still something weird going on here.

I can consistently reproduce the crash locally with the following patch in bug 1119490: https://bug1119490.bmoattachments.org/attachment.cgi?id=8720463 That patch takes the URL constructor, which we exposed to the WorkerDebugger's GlobalScope object in an earlier patch, and makes it a per-module global for the CommonJS modules used in the debugger server. We only use the URL constructor in a one place at the moment, namely devtools/server/actor/utils/TabSources.js. Could we somehow be leaking the URL constructor, and could that be the cause for this crash? (It seems like, even if we *were* leaking the URL constructor, it should be destroyed gracefully when the worker debugger's global scope is destroyed).

Here's a patch that applies cleanly to mozilla-central and reliably causes browser_dbg_worker-window.js to crash on this assertion in a debug build (I only tested locally on OS X, but the try run above fails on every other platform, so it should be reproducible there as well). Note that the platform parts of this patch already landed on fx-team.

Looks like the platform changes have been merged to m-c from fx-team. Rebased the patch.

Hi Ritu, I confirm that the issue was fixed for my use-case by the attached patch, but Eddy is seeing the same symptom for a different reason. Eddy, do you want to take over this bug because you're seeing the same symptom, or do you want to open a different bug for your queued worker runnable problem?

I spent most of my evening yesterday debugging this in rr, with substantial help from both terrence and khuey (I couldn't have done it without your help. Thanks again!), and I think we managed to pinpoint the root cause of the issue. The problem is that when we terminate the worker, there will sometimes still be a debugger runnable on the debugger event queue. In particular, an instance of DebuggerImmediateRunnable. This runnable holds a strong reference to a DOM function, which in turn holds a reference to it's global object. This causes us to leak the world. Since the lifetime of runnables is managed manually, the cycle collector was unable to detect this particular cycle. Since we only leak if that runnable still happens to be on the queue, this explains the intermittent nature of this crash. The solution is, of course, to reimplement everything in Rust.

Either that, or we could land this patch, which explicitly clears the debugger's event queue before destroying the context :-)

Comment on attachment 8723575 [details] [diff] [review] Clear the worker's debugger event queue before destroying its context. Review of attachment 8723575 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/RuntimeService.cpp @@ +2686,5 @@ > + // strong reference to the debugger global scope. These runnables are not > + // visible to the cycle collector, so we need to make sure to clear the > + // debugger event queue before we try to destroy the context. If we don't, > + // the garbage collector will crash. > + mWorkerPrivate->ClearDebuggerEventQueue(); I think you should put this after JS_DestroyContext.

Try run for this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c2a31f1022a

Can you nominate this for Aurora & ESR45 approval to fix bug 1230981? Assuming this is sufficiently low-risk, of course :)

Comment on attachment 8723575 [details] [diff] [review] Clear the worker's debugger event queue before destroying its context. Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: This patchs prevents Firefox from crashing on shutdown when you're using the worker debugger. [Describe test coverage new/current, TreeHerder]: Hard to test in general, because the exact behavior is timing sensitive in a way that we have no direct control over. However, we have a single test, browser_dbg_worker-window.js, that was crashing intermittenyly due to this bug. [Risks and why]: No significant risks. [String/UUID change made/needed]:

Comment on attachment 8723575 [details] [diff] [review] Clear the worker's debugger event queue before destroying its context. Let's give this a try, as it should prevent a shutdown crash and also fix an annoying intermittent test failure.

Marking affected for 46.

Comment on attachment 8723575 [details] [diff] [review] Clear the worker's debugger event queue before destroying its context. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes a variety of intermittent failures. User impact if declined: This patch prevents Firefox from crashing on shutdown when you're using the worker debugger. Fix Landed on Version: 47, already uplifted to 46 as well. Risk to taking this patch (and alternatives if risky): Oranges for the duration of the ESR45 cycle. String or UUID changes made by this patch: None

Comment on attachment 8723575 [details] [diff] [review] Clear the worker's debugger event queue before destroying its context. Simplify the life of sheriff, taking it. Should be in 45.1.0