1241841 - Assertion failure: isEmpty(), at dist/include/mozilla/LinkedList.h:328

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P1)

Description

[Jan Keromnes [:janx]]

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.

Comment 1

[Jan Keromnes [:janx]]

Nicolas, could you please have a look?

Comment 2

[Nicolas B. Pierron [:nbp]]

I will forward this issue to terrence.

Comment 3

[Alexandre Poirot [:ochameau]]

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

Comment 4

[Jan Keromnes [:janx]]

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

Comment 5

[Jan Keromnes [:janx]]

(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).

Comment 6

[Jan Keromnes [:janx]]

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

Comment 7

[Jan de Mooij [:jandem]]

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

Someone needs to take this.

Comment 8

[Jan Keromnes [:janx]]

Attachment: 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

Comment 9

[Jan Keromnes [:janx]]

Looks green enough.

Comment 10

[Pulsebot]

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

Comment 11

[Carsten Book [:Tomcat]]

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

Comment 12

[Terrence Cole [:terrence]]

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

Comment 13

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

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

Comment 14

[Eddy Bruel [:ejpbruel]]

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.

Comment 15

[Eddy Bruel [:ejpbruel]]

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.

Comment 16

[Eddy Bruel [:ejpbruel]]

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

Comment 17

[Eddy Bruel [:ejpbruel]]

Attachment: Minimal STR

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.

Comment 18

[Eddy Bruel [:ejpbruel]]

Attachment: Minimal STR

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

Comment 19

[Jan Keromnes [:janx]]

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?

Comment 20

[Eddy Bruel [:ejpbruel]]

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.

Comment 21

[Eddy Bruel [:ejpbruel]]

Attachment: Clear the worker's debugger event queue before destroying its context.

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

Comment 22

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 23

[Eddy Bruel [:ejpbruel]]

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

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f513fdef559

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2f513fdef559

Comment 26

[Ryan VanderMeulen [:RyanVM]]

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

Comment 27

[Eddy Bruel [:ejpbruel]]

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 28

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 29

[Liz Henry (:lizzard) (relman/hg->git project)]

Marking affected for 46.

Comment 30

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/08f158aefcce

Comment 31

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/08f158aefcce

Comment 32

[Ryan VanderMeulen [:RyanVM]]

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 34

[Sylvestre Ledru [:Sylvestre]]

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

Comment 35

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

https://hg.mozilla.org/releases/mozilla-esr45/rev/b0055bb88df8