425811 - ActionMonkey: browser doesn't start (selflock in GC -> postcollect -> CycleCollector -> DestroyContext -> GC)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

The deadlock is actually around gc->m_callbackListLock, not gc->m_lock.

^C
Program received signal SIGINT, Interrupt.
0xffff027f in __spin_lock ()
(gdb) info threads
* 1 process 54663 local thread 0x2e3f  0xffff027f in __spin_lock ()
(gdb) where
#0  0xffff027f in __spin_lock ()
#1  0x003c25e3 in MMgc::GCSpinLock::Acquire (this=0x850754) at GCSpinLockMac.h:89
#2  0x003c2615 in MMgc::GCAcquireSpinlock::GCAcquireSpinlock (this=0xbfffa934, spinlock=@0x850754) at GCSpinLockMac.h:131
#3  0x003c8478 in MMgc::GC::CollectWithBookkeeping (this=0x850400, callerHoldsLock=false, callerHasActiveRequest=true) at /Users/jason/dev/am/actionmonkey-merge/js/tamarin/MMgc/GC.cpp:431
#4  0x0015e6b1 in js_GC (cx=0x7ce760, gckind=GC_NORMAL) at /Users/jason/dev/am/actionmonkey-merge/js/src/jsgc.cpp:1634
#5  0x0013d821 in js_DestroyContext (cx=0x7ce760, mode=JSDCM_FORCE_GC) at /Users/jason/dev/am/actionmonkey-merge/js/src/jscntxt.cpp:420
#6  0x0012422f in JS_DestroyContext (cx=0x0) at /Users/jason/dev/am/actionmonkey-merge/js/src/jsapi.cpp:1018
#7  0x0a10429e in nsXPConnect::ReleaseJSContext (this=0x7288f0, aJSContext=0x7ce760, noGC=0) at /Users/jason/dev/am/actionmonkey-merge/js/src/xpconnect/src/nsXPConnect.cpp:2085
#8  0x0c97e7d8 in nsJSContext::Unlink (this=0x7ce6b0) at /Users/jason/dev/am/actionmonkey-merge/dom/src/base/nsJSEnvironment.cpp:1221
#9  0x0c97e888 in nsJSContext::cycleCollection::Unlink (this=0x0, p=0x7ce6b0) at /Users/jason/dev/am/actionmonkey-merge/dom/src/base/nsJSEnvironment.cpp:1232
#10 0x002c0825 in nsCycleCollector::CollectWhite (this=0x680000) at /Users/jason/dev/am/actionmonkey-merge/xpcom/base/nsCycleCollector.cpp:1660
#11 0x002c0958 in nsCycleCollector::FinishCollection (this=0x680000) at /Users/jason/dev/am/actionmonkey-merge/xpcom/base/nsCycleCollector.cpp:2434
#12 0x002c09b1 in nsCycleCollector_finishCollection () at /Users/jason/dev/am/actionmonkey-merge/xpcom/base/nsCycleCollector.cpp:2916
#13 0x0a1034f2 in XPCCycleCollectGCCallback (cx=0x754b90, status=JSGC_END) at /Users/jason/dev/am/actionmonkey-merge/js/src/xpconnect/src/nsXPConnect.cpp:450
#14 0x0015eb75 in JSGC::postcollect (this=0x14e002a4) at /Users/jason/dev/am/actionmonkey-merge/js/src/jsgc.cpp:1051
#15 0x003c894b in MMgc::GC::CollectWithBookkeeping (this=0x850400, callerHoldsLock=false, callerHasActiveRequest=true) at /Users/jason/dev/am/actionmonkey-merge/js/tamarin/MMgc/GC.cpp:573
#16 0x0015e6b1 in js_GC (cx=0x754b90, gckind=GC_NORMAL) at /Users/jason/dev/am/actionmonkey-merge/js/src/jsgc.cpp:1634
#17 0x001254e4 in JS_GC (cx=0x754b90) at /Users/jason/dev/am/actionmonkey-merge/js/src/jsapi.cpp:2428
#18 0x0a103e80 in nsXPConnect::Collect (this=0x7288f0) at /Users/jason/dev/am/actionmonkey-merge/js/src/xpconnect/src/nsXPConnect.cpp:526
#19 0x002c12a6 in nsCycleCollector::Collect (this=0x680000, aTryCollections=5) at /Users/jason/dev/am/actionmonkey-merge/xpcom/base/nsCycleCollector.cpp:2250
#20 0x002c1621 in nsCycleCollector::Shutdown (this=0x754b90) at /Users/jason/dev/am/actionmonkey-merge/xpcom/base/nsCycleCollector.cpp:2471
#21 0x002c1654 in nsCycleCollector_shutdown () at /Users/jason/dev/am/actionmonkey-merge/xpcom/base/nsCycleCollector.cpp:2932
#22 0x00242561 in NS_ShutdownXPCOM_P (servMgr=0x7158d4) at /Users/jason/dev/am/actionmonkey-merge/xpcom/build/nsXPComInit.cpp:783
#23 0x00093962 in ScopedXPCOMStartup::~ScopedXPCOMStartup (this=0xbfffee6c) at /Users/jason/dev/am/actionmonkey-merge/toolkit/xre/nsAppRunner.cpp:909
#24 0x00095cb2 in ShowProfileManager (aProfileSvc=0x7135b0, aNative=0x713970) at /Users/jason/dev/am/actionmonkey-merge/toolkit/xre/nsAppRunner.cpp:1771
#25 0x00097bc3 in SelectProfile (aResult=0xbffff460, aNative=0x713970, aStartOffline=<value temporarily unavailable, due to optimizations>, aProfileName=0xbffff244) at /Users/jason/dev/am/actionmonkey-merge/toolkit/xre/nsAppRunner.cpp:2071
#26 0x0009e1e9 in XRE_main (argc=1, argv=0xbffff588, aAppData=0x70d9c0) at /Users/jason/dev/am/actionmonkey-merge/toolkit/xre/nsAppRunner.cpp:2878
#27 0x00002870 in main (argc=1, argv=0x70b4f0) at /Users/jason/dev/am/actionmonkey-merge/browser/app/nsBrowserApp.cpp:158

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: v1

Much the easiest fix is to ditch the lock around the callback list.  This means changing the API to forbid applications from adding or removing callbacks except during GC initialization and teardown.  The new restriction only applies to MMGC_THREADSAFE builds; I think that's OK.

A more permissive scheme might be possible, something like

  @access ReadWrite(exclusiveGC,
    initializing || (request && m_callbackListLock))

but I have other stuff to think about :)

Comment 2

[Jim Blandy :jimb]

Is there some way that AddCallback and RemoveCallback could abort if the new condition you're suggesting isn't satisfied?

Comment 3

[Jason Orendorff [:jorendorff]]

As discussed on IRC, I don't think it's worth it.  There are other APIs where that kind of mistake is much more likely and just as hard to detect.  If we want to improve assertion coverage (a noble goal), there are better places to direct energy.

Pushed changeset aefa1cf524c3 to actionmonkey-tamarin.