Closed Bug 407034 Opened 17 years ago Closed 16 years ago

JS_Assert "!rt->gcRunning" unbinding link elements in cycle collector with JS protocol handlers

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: roc, Assigned: peterv)

References

Details

(Keywords: crash)

Attachments

(1 file, 3 obsolete files)

Here's a stack:

#0  JS_Assert (s=0x11026d0 "!rt->gcRunning", file=0x1101e0c
"/Users/roc/mozilla-checkin/mozilla/js/src/jsgc.c", ln=1336) at
/Users/roc/mozilla-checkin/mozilla/js/src/jsutil.c:63
#1  0x0105d639 in js_NewGCThing (cx=0x30b75840, flags=7, nbytes=8) at
/Users/roc/mozilla-checkin/mozilla/js/src/jsgc.c:1336
#2  0x01018e36 in JS_NewExternalString (cx=0x30b75840, chars=0x3f797f80,
length=35, type=0) at /Users/roc/mozilla-checkin/mozilla/js/src/jsapi.c:2513
#3  0x12a244f7 in XPCConvert::NativeData2JS (ccx=@0xbfffcce0, d=0xbfffcdc4,
s=0xbfffcfd4, type=@0xbfffce49, iid=0xbfffce18, scope=0x34abb060, pErr=0x0) at
/Users/roc/mozilla-checkin/mozilla/js/src/xpconnect/src/xpcconvert.cpp:409
#4  0x12a3f68a in nsXPCWrappedJSClass::CallMethod (this=0x34b78510,
wrapper=0x34b787f0, methodIndex=6, info=0x20e8838, nativeParams=0xbfffcfd4) at
/Users/roc/mozilla-checkin/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1341
#5  0x12a3828f in nsXPCWrappedJS::CallMethod (this=0x34b787f0, methodIndex=6,
info=0x20e8838, params=0xbfffcfd4) at
/Users/roc/mozilla-checkin/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp:567
#6  0x01380c8c in PrepareAndDispatch (self=0x34b78830, methodIndex=6,
args=0xbfffd0f4) at
/Users/roc/mozilla-checkin/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcstubs_unixish_x86.cpp:93
#7  0x01380de0 in nsXPTCStubBase::Stub6 (this=0x34b78830) at
../../../../../../dist/include/xpcom/xptcstubsdef.inc:8
#8  0x2b1b0a19 in nsIOService::NewURI (this=0x2926510, aSpec=@0xbfffd22c,
aCharset=0x44942be8 "UTF-8", aBaseURI=0x34752f40, result=0xbfffd384) at
/Users/roc/mozilla-checkin/mozilla/netwerk/base/src/nsIOService.cpp:500
#9  0x002536b8 in NS_NewURI (result=0xbfffd384, spec=@0xbfffd22c,
charset=0x44942be8 "UTF-8", baseURI=0x34752f40, ioService=0x2926510) at
../../../dist/include/xpcom/nsCOMPtr.h:127
#10 0x13e29903 in NS_NewURI (result=0xbfffd384, spec=@0xbfffd2ec,
charset=0x44942be8 "UTF-8", baseURI=0x34752f40, ioService=0x2926510) at
../../dist/include/xpcom/nsServiceManagerUtils.h:138
#11 0x13929dd1 in nsContentUtils::NewURIWithDocumentCharset
(aResult=0xbfffd384, aSpec=@0xbfffd2ec, aDocument=0x42567800,
aBaseURI=0x34752f40) at
/Users/roc/mozilla-checkin/mozilla/content/base/src/nsContentUtils.cpp:1829
#12 0x139eccbb in nsGenericHTMLElement::GetHrefURIForAnchors (this=0x35372250,
aURI=0xbfffd384) at
/Users/roc/mozilla-checkin/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1315
#13 0x139ecd56 in nsGenericHTMLElement::IsHTMLLink (this=0x35372250,
aURI=0xbfffd384) at
/Users/roc/mozilla-checkin/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1292
#14 0x139f591e in nsHTMLAnchorElement::IsLink (this=0x35372250,
aURI=0xbfffd384) at
/Users/roc/mozilla-checkin/mozilla/content/html/content/src/nsHTMLAnchorElement.cpp:294
#15 0x1395000d in nsDocument::ForgetLink (this=0x42567800, aContent=0x35372250)
at /Users/roc/mozilla-checkin/mozilla/content/base/src/nsDocument.cpp:5904
#16 0x139f6667 in nsHTMLAnchorElement::UnbindFromTree (this=0x35372250,
aDeep=1, aNullParent=1) at
/Users/roc/mozilla-checkin/mozilla/content/html/content/src/nsHTMLAnchorElement.cpp:204
#17 0x1396dc17 in nsGenericElement::cycleCollection::Unlink (this=0x13ffc140,
p=0x353721b0) at
/Users/roc/mozilla-checkin/mozilla/content/base/src/nsGenericElement.cpp:3361
#18 0x0137f116 in nsCycleCollector::CollectWhite (this=0xa7000) at
/Users/roc/mozilla-checkin/mozilla/xpcom/base/nsCycleCollector.cpp:1542
#19 0x0137fb93 in nsCycleCollector::BeginCollection (this=0xa7000) at
/Users/roc/mozilla-checkin/mozilla/xpcom/base/nsCycleCollector.cpp:2252
#20 0x0137fbd2 in nsCycleCollector_beginCollection () at
/Users/roc/mozilla-checkin/mozilla/xpcom/base/nsCycleCollector.cpp:2694
#21 0x12a0ad3e in XPCCycleCollectGCCallback (cx=0x30b75840,
status=JSGC_MARK_END) at
/Users/roc/mozilla-checkin/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:428
#22 0x010607d0 in js_GC (cx=0x30b75840, gckind=GC_NORMAL) at
/Users/roc/mozilla-checkin/mozilla/js/src/jsgc.c:2526
#23 0x01018c1f in JS_GC (cx=0x30b75840) at
/Users/roc/mozilla-checkin/mozilla/js/src/jsapi.c:2383
#24 0x12a07476 in nsXPConnect::Collect (this=0x292d9f0) at
/Users/roc/mozilla-checkin/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:518
#25 0x0137fcb1 in nsCycleCollector::Collect (this=0xa7000, aTryCollections=1)
at /Users/roc/mozilla-checkin/mozilla/xpcom/base/nsCycleCollector.cpp:2132
#26 0x0137fd54 in nsCycleCollector_collect () at
/Users/roc/mozilla-checkin/mozilla/xpcom/base/nsCycleCollector.cpp:2688
#27 0x13b266c2 in nsJSContext::CC () at
/Users/roc/mozilla-checkin/mozilla/dom/src/base/nsJSEnvironment.cpp:3313
#28 0x13b26783 in nsJSContext::MaybeCC (aHigherProbability=0) at
/Users/roc/mozilla-checkin/mozilla/dom/src/base/nsJSEnvironment.cpp:3335
#29 0x13b293b1 in nsUserActivityObserver::Observe (this=0x30b75180,
aSubject=0x0, aTopic=0x13dc8c1c "user-interaction-active", aData=0x0) at
/Users/roc/mozilla-checkin/mozilla/dom/src/base/nsJSEnvironment.cpp:279
#30 0x01325e2f in nsObserverList::NotifyObservers (this=0x226893c,
aSubject=0x0, aTopic=0x13dc8c1c "user-interaction-active", someData=0x0) at
/Users/roc/mozilla-checkin/mozilla/xpcom/ds/nsObserverList.cpp:128
#31 0x013264d9 in nsObserverService::NotifyObservers (this=0x2921bd0,
aSubject=0x0, aTopic=0x13dc8c1c "user-interaction-active", someData=0x0) at
/Users/roc/mozilla-checkin/mozilla/xpcom/ds/nsObserverService.cpp:181
#32 0x139c3ec2 in nsUITimerCallback::Notify (this=0x30ba4f20,
aTimer=0x30ba1650) at
/Users/roc/mozilla-checkin/mozilla/content/events/src/nsEventStateManager.cpp:208
#33 0x01372c4b in nsTimerImpl::Fire (this=0x30ba1650) at
/Users/roc/mozilla-checkin/mozilla/xpcom/threads/nsTimerImpl.cpp:403
#34 0x01372e4f in nsTimerEvent::Run (this=0x34918080) at
/Users/roc/mozilla-checkin/mozilla/xpcom/threads/nsTimerImpl.cpp:487
#35 0x0136ec37 in nsThread::ProcessNextEvent (this=0x2912840, mayWait=0,
result=0xbfffdad4) at
/Users/roc/mozilla-checkin/mozilla/xpcom/threads/nsThread.cpp:490

See discussion in bug 402966. This seems to be triggered by "irc:" URIs --- I have Chatzilla installed.
Flags: blocking1.9?
Boris's comment #13 from bug 402966:

> Given that we're unlinking the element, and the element IsInDoc(), that means
> that the document is being unlinked too, right?
>
> It might make sense to skip the unbind/remove here and have the document handle
> teardown of the whole DOM in that case.  Then the document could set a flag
> indicating that it's in such teardown and nsHTMLAnchorElement could check it
> before doing the ForgetLink thing.
>
> In fact, that might be a good idea in other cases where calling ForgetLink is
> pointless (places where the doc clears the hashtable anyway).

We already do most of that. In particular, document teardown clears the link map before unbinding the content, so ForgetLink doesn't do any work in that case:
http://mxr.mozilla.org/seamonkey/source/content/base/src/nsDocument.cpp#5891

So all we need here is some way to ensure the map is cleared before we unbind the content. "skip the unbind/remove here and have the document handle teardown of the whole DOM in that case" would be one way to do it. I don't really have a clue how to implement that, though.
Summary: JS_Assert "!rt->gcRunning" unbinding link elements with JS protocol handlers → JS_Assert "!rt->gcRunning" unbinding link elements in cycle collector with JS protocol handlers
three proposals (all should be implemented)
1. at frame 5 (and all similar entry points), xpconnect checks JSRuntime is GC running, and if it is, it returns a failure nsresult.
2. we add code to tell the eventloop "don't allow nesting now". It's probably best if we provide a stack helper class which people can use to tell the event loop that. when the eventloop is told not to allow nesting and someone asks to create a nested loop, it returns a failure nsresult.
(this case is for the other version of crashing which is where something causes gecko to reenter painting when it isn't happy and such, ask roc for details.)
3. code called from Unlink (frame 16 here) needs to be treated as if it's called from a destructor (the exact details of this are something bz gets to worry about)
Peter, you get this one :)
Assignee: nobody → peterv
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Severity: normal → critical
Keywords: crash
I'm seeing this, but with a different (but still external) protocol:

feed:http://john.jubjubs.net/comments/feed/

It appears to happen when I read John Lilly's blog and then close the tab.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007121017 Minefield/3.0b2pre

Here's the stack, in case it is helpful:

Assertion failure: !rt->gcRunning, at c:/builds/trunk/mozilla/js/src/jsgc.c:1336

 	ntdll.dll!7c901230() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
 	js3250.dll!JS_Assert(const char * s=0x0059b94c, const char * file=0x0059b924, int ln=1336)  Line 59	C
>	js3250.dll!js_NewGCThing(JSContext * cx=0x02082308, unsigned int flags=7, unsigned int nbytes=8)  Line 1336 + 0x24 bytes	C
 	js3250.dll!JS_NewExternalString(JSContext * cx=0x02082308, unsigned short * chars=0x06612c78, unsigned int length=34, int type=0)  Line 2514 + 0x12 bytes	C
 	xpc3250.dll!XPCConvert::NativeData2JS(XPCCallContext & ccx={...}, long * d=0x0012ef58, const void * s=0x0012f138, const nsXPTType & type={...}, const nsID * iid=0x0012f00c, JSObject * scope=0x0db10ae0, unsigned int * pErr=0x00000000)  Line 409 + 0x18 bytes	C++
 	xpc3250.dll!nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS * wrapper=0x068dc698, unsigned short methodIndex=6, const XPTMethodDescriptor * info=0x02009d78, nsXPTCMiniVariant * nativeParams=0x0012f138)  Line 1342 + 0x31 bytes	C++
 	xpc3250.dll!nsXPCWrappedJS::CallMethod(unsigned short methodIndex=6, const XPTMethodDescriptor * info=0x02009d78, nsXPTCMiniVariant * params=0x0012f138)  Line 568	C++
 	xpcom_core.dll!PrepareAndDispatch(nsXPTCStubBase * self=0x068dc508, unsigned int methodIndex=6, unsigned int * args=0x0012f1f8, unsigned int * stackBytesToPop=0x0012f1e8)  Line 114 + 0x21 bytes	C++
 	xpcom_core.dll!SharedStub()  Line 142	C++
 	necko.dll!nsIOService::NewURI(const nsACString_internal & aSpec={...}, const char * aCharset=0x09dad6b0, nsIURI * aBaseURI=0x08166300, nsIURI * * result=0x0012f3bc)  Line 500 + 0x29 bytes	C++
 	necko.dll!nsIOService::NewURI(const nsACString_internal & aSpec={...}, const char * aCharset=0x09dad6b0, nsIURI * aBaseURI=0x08166300, nsIURI * * result=0x0012f3bc)  Line 500 + 0x29 bytes	C++
 	gklayout.dll!NS_NewURI(nsIURI * * result=0x0012f3bc, const nsACString_internal & spec={...}, const char * charset=0x09dad6b0, nsIURI * baseURI=0x08166300, nsIIOService * ioService=0x00c86560)  Line 127 + 0x1e bytes	C++
 	gklayout.dll!NS_NewURI(nsIURI * * result=0x0012f3bc, const nsAString_internal & spec={...}, const char * charset=0x09dad6b0, nsIURI * baseURI=0x08166300, nsIIOService * ioService=0x00c86560)  Line 138 + 0x22 bytes	C++
 	gklayout.dll!nsContentUtils::NewURIWithDocumentCharset(nsIURI * * aResult=0x0012f3bc, const nsAString_internal & aSpec={...}, nsIDocument * aDocument=0x075e1f90, nsIURI * aBaseURI=0x08166300)  Line 1829 + 0x3c bytes	C++
 	gklayout.dll!nsGenericHTMLElement::GetHrefURIForAnchors(nsIURI * * aURI=0x0012f3bc)  Line 1315 + 0x28 bytes	C++
 	gklayout.dll!nsGenericHTMLElement::IsHTMLLink(nsIURI * * aURI=0x0012f3bc)  Line 1294	C++
 	gklayout.dll!nsHTMLAnchorElement::IsLink(nsIURI * * aURI=0x0012f3bc)  Line 295	C++
 	gklayout.dll!nsDocument::ForgetLink(nsIContent * aContent=0x06743ac8)  Line 5898 + 0x28 bytes	C++
 	gklayout.dll!nsHTMLAnchorElement::UnbindFromTree(int aDeep=1, int aNullParent=0)  Line 207	C++
 	gklayout.dll!nsGenericElement::UnbindFromTree(int aDeep=1, int aNullParent=0)  Line 2133	C++
 	gklayout.dll!nsGenericHTMLElement::UnbindFromTree(int aDeep=1, int aNullParent=0)  Line 1172	C++
 	gklayout.dll!nsGenericElement::UnbindFromTree(int aDeep=1, int aNullParent=0)  Line 2133	C++
 	gklayout.dll!nsGenericHTMLElement::UnbindFromTree(int aDeep=1, int aNullParent=0)  Line 1172	C++
 	gklayout.dll!nsGenericElement::UnbindFromTree(int aDeep=1, int aNullParent=0)  Line 2133	C++
 	gklayout.dll!nsGenericHTMLElement::UnbindFromTree(int aDeep=1, int aNullParent=0)  Line 1172	C++
 	gklayout.dll!nsGenericElement::UnbindFromTree(int aDeep=1, int aNullParent=1)  Line 2133	C++
 	gklayout.dll!nsGenericHTMLElement::UnbindFromTree(int aDeep=1, int aNullParent=1)  Line 1172	C++
 	gklayout.dll!nsHTMLBodyElement::UnbindFromTree(int aDeep=1, int aNullParent=1)  Line 478	C++
 	gklayout.dll!nsGenericElement::cycleCollection::Unlink(void * p=0x04fe5598)  Line 3362	C++
 	xpcom_core.dll!nsCycleCollector::CollectWhite(GCGraph & graph={...})  Line 1510 + 0x1a bytes	C++
 	xpcom_core.dll!nsCycleCollector::DoCollect()  Line 2217 + 0xc bytes	C++
 	xpcom_core.dll!nsCycleCollector_doCollect()  Line 2651 + 0x14 bytes	C++
 	xpc3250.dll!XPCCycleCollectGCCallback(JSContext * cx=0x02082308, JSGCStatus status=JSGC_MARK_END)  Line 424 + 0xf bytes	C++
 	js3250.dll!js_GC(JSContext * cx=0x02082308, JSGCInvocationKind gckind=GC_NORMAL)  Line 2526 + 0x11 bytes	C
 	js3250.dll!JS_GC(JSContext * cx=0x02082308)  Line 2383 + 0xb bytes	C
 	xpc3250.dll!nsXPConnect::Collect()  Line 510 + 0xa bytes	C++
 	xpcom_core.dll!nsCycleCollector::Collect(unsigned int aTryCollections=1)  Line 2094 + 0x13 bytes	C++
 	xpcom_core.dll!nsCycleCollector_collect()  Line 2645 + 0x16 bytes	C++
 	gklayout.dll!nsJSContext::CC()  Line 3313 + 0x6 bytes	C++
 	gklayout.dll!nsJSContext::MaybeCC(int aHigherProbability=0)  Line 3336	C++
 	gklayout.dll!nsUserActivityObserver::Observe(nsISupports * aSubject=0x00000000, const char * aTopic=0x02b95810, const unsigned short * aData=0x00000000)  Line 279 + 0x9 bytes	C++
 	xpcom_core.dll!nsObserverList::NotifyObservers(nsISupports * aSubject=0x00000000, const char * aTopic=0x02b95810, const unsigned short * someData=0x00000000)  Line 129	C++
 	xpcom_core.dll!nsObserverService::NotifyObservers(nsISupports * aSubject=0x00000000, const char * aTopic=0x02b95810, const unsigned short * someData=0x00000000)  Line 184	C++
 	gklayout.dll!nsUITimerCallback::Notify(nsITimer * aTimer=0x03060f60)  Line 210	C++
 	xpcom_core.dll!nsTimerImpl::Fire()  Line 404	C++
 	xpcom_core.dll!nsTimerEvent::Run()  Line 489	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012f984)  Line 511	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00c02dd0, int mayWait=1)  Line 227 + 0x16 bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 154 + 0xc bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 170 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=3, char * * argv=0x00bdd638, const nsXREAppData * aAppData=0x00bdda28)  Line 3145 + 0x25 bytes	C++
 	firefox.exe!main(int argc=3, char * * argv=0x00bdd638)  Line 153 + 0x12 bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	firefox.exe!mainCRTStartup()  Line 403	C
 	kernel32.dll!7c816fd7() 	
 	js3250.dll!js_EmitTree(JSContext * cx=0x006b006f, JSCodeGenerator * cg=0x006e0065, JSParseNode * pn=0x0022003d)  Line 5045 + 0x13 bytes	C
 	js3250.dll!js_InitRuntimeScriptState(JSRuntime * rt=0x006b006f)  Line 992 + 0x1a bytes	C
 	js3250.dll!js_InitRuntimeScriptState(JSRuntime * rt=0x006b006f)  Line 992 + 0x1a bytes	C
 	js3250.dll!js_InitRuntimeScriptState(JSRuntime * rt=0x006b006f)  Line 992 + 0x1a bytes	C
 	js3250.dll!js_InitRuntimeScriptState(JSRuntime * rt=0x006b006f)  Line 992 + 0x1a bytes	C

steps to reproduce:

1) open http://john.jubjubs.net/2007/11/27/mozilla-firefox-market-share/ in a tab
2) close that tab

Sometimes it appears it take a few times before we hit the JS_Assert
I'm seeing this with yet another slightly different testcase...
With the newsgroups reader, tested actually on SeaMonkey:
1 - subscribe to mozilla.dev.planning
2 - start reading "Thoughts on next release after Gecko 1.9 (a modest proposal)" thread
3 - read on until bsmedberg and Martijn sequence of emails
4 - switch between them (with 'f' and 'b')

I'm viewing message bodies as plain text, attachments inline, viewing normal headers.
The messages with these IDs are likely to crash:
<mailman.1251.1198113156.3345.dev-planning@lists.mozilla.org>
<19ednRvPUNlkWPTanZ2dnUVZ_hGdnZ2d@mozilla.org>

It doesn't crash with any other combination I tried, but this way it crashes with the following stack trace:
#0  0x00002ba0a0032765 in raise () from /lib/libc.so.6
#1  0x00002ba0a00341c0 in abort () from /lib/libc.so.6
#2  0x00002ba09b433696 in JS_Assert (s=<value optimized out>, file=<value optimized out>, ln=<value optimized out>)
    at /home/asrail/mozbuilds/mozilla/js/src/jsutil.c:63
#3  0x00002ba09b3e597b in js_NewGCThing (cx=0xa817d0, flags=7, nbytes=16)
    at /home/asrail/mozbuilds/mozilla/js/src/jsgc.c:1336
#4  0x00002ba09b3b78f4 in JS_NewExternalString (cx=0xa817d0, chars=0xa0006b0, length=24, type=0)
    at /home/asrail/mozbuilds/mozilla/js/src/jsapi.c:2527
#5  0x00002ba0a8a5e47c in XPCConvert::NativeData2JS (ccx=@0x7fff0fb48aa0, d=0x7fff0fb48988, s=<value optimized out>,
    type=@0x7fff0fb48a9b, iid=0x7fff0fb48bb0, scope=0x2aaabdc64c80, pErr=0x0)
    at /home/asrail/mozbuilds/mozilla/js/src/xpconnect/src/xpcconvert.cpp:409
#6  0x00002ba0a8a71df2 in nsXPCWrappedJSClass::CallMethod (this=0x9ac90c0, wrapper=<value optimized out>, methodIndex=6,
    info=0xa14200, nativeParams=0x7fff0fb48c60)
    at /home/asrail/mozbuilds/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1341
#7  0x00002ba09bb66298 in PrepareAndDispatch (self=0x8609e30, methodIndex=<value optimized out>,
    args=<value optimized out>, gpregs=0x7fff0fb48d40, fpregs=0x7fff0fb48d70)
    at /home/asrail/mozbuilds/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:151
#8  0x00002ba09bb655c3 in SharedStub () at ../../../../../../dist/include/xpcom/xptcstubsdef.inc:255
#9  0x00002ba0a938adae in nsIOService::NewURI (this=0x79f7f0, aSpec=@0x7fff0fb48f20, aCharset=0xa410308 "UTF-8",
    aBaseURI=0x9ceefd8, result=0x7fff0fb49050) at /home/asrail/mozbuilds/mozilla/netwerk/base/src/nsIOService.cpp:500
#10 0x00002aaaadb5ab2d in NS_NewURI (result=0x7fff0fb49050, spec=@0x7fff0fb48f20, charset=0xa410308 "UTF-8",
    baseURI=0x9ceefd8, ioService=0x79f7f0) at ../../../dist/include/necko/nsNetUtil.h:127
#11 0x00002aaaadc06edd in NS_NewURI (result=0x7fff0fb49050, spec=<value optimized out>, charset=0xa410308 "UTF-8",
    baseURI=0x9ceefd8, ioService=0x79f7f0) at ../../../dist/include/necko/nsNetUtil.h:138
#12 0x00002aaaaddd1d6e in nsGenericHTMLElement::GetHrefURIForAnchors (this=0x15a6ae0, aURI=0x7fff0fb49050)
    at /home/asrail/mozbuilds/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1315
#13 0x00002aaaaddd1dc8 in nsGenericHTMLElement::IsHTMLLink (this=0x15a6ae0, aURI=0x7fff0fb49050)
    at /home/asrail/mozbuilds/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1292
#14 0x00002aaaadd4b3cb in nsDocument::ForgetLink (this=<value optimized out>, aContent=0x15a6ae0)
    at /home/asrail/mozbuilds/mozilla/content/base/src/nsDocument.cpp:5862
#15 0x00002aaaadddb0d8 in nsHTMLAnchorElement::UnbindFromTree (this=0x15a6ae0, aDeep=1, aNullParent=0)
    at /home/asrail/mozbuilds/mozilla/content/html/content/src/nsHTMLAnchorElement.cpp:204
#16 0x00002aaaadd6ed0f in nsGenericElement::UnbindFromTree (this=0x15a6708, aDeep=<value optimized out>,
    aNullParent=<value optimized out>) at /home/asrail/mozbuilds/mozilla/content/base/src/nsGenericElement.cpp:2169
#17 0x00002aaaaddd361c in nsGenericHTMLElement::UnbindFromTree (this=0x15a6708, aDeep=1, aNullParent=0)
    at /home/asrail/mozbuilds/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1171
#18 0x00002aaaadd6ed0f in nsGenericElement::UnbindFromTree (this=0x15a65e0, aDeep=<value optimized out>,
    aNullParent=<value optimized out>) at /home/asrail/mozbuilds/mozilla/content/base/src/nsGenericElement.cpp:2169
#19 0x00002aaaaddd361c in nsGenericHTMLElement::UnbindFromTree (this=0x15a65e0, aDeep=1, aNullParent=1)
    at /home/asrail/mozbuilds/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1171
#20 0x00002aaaadd6c88e in nsGenericElement::cycleCollection::Unlink (this=<value optimized out>, p=<value optimized out>)
    at /home/asrail/mozbuilds/mozilla/content/base/src/nsGenericElement.cpp:3403
#21 0x00002ba09bb63b99 in nsCycleCollector::CollectWhite (this=<value optimized out>, graph=<value optimized out>)
    at /home/asrail/mozbuilds/mozilla/xpcom/base/nsCycleCollector.cpp:1510
#22 0x00002ba09bb64245 in nsCycleCollector::DoCollect (this=0x2ba0a7690010)
    at /home/asrail/mozbuilds/mozilla/xpcom/base/nsCycleCollector.cpp:2217
#23 0x00002ba0a8a47c8c in XPCCycleCollectGCCallback (cx=0xa817d0, status=4480)
    at /home/asrail/mozbuilds/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:430
#24 0x00002ba09b3e46f6 in js_GC (cx=0xa817d0, gckind=GC_NORMAL) at /home/asrail/mozbuilds/mozilla/js/src/jsgc.c:2526
#25 0x00002ba0a8a46e37 in nsXPConnect::Collect (this=0x777bb0)
    at /home/asrail/mozbuilds/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:516
#26 0x00002ba09bb6431f in nsCycleCollector::Collect (this=0x2ba0a7690010, aTryCollections=1)
    at /home/asrail/mozbuilds/mozilla/xpcom/base/nsCycleCollector.cpp:2094
#27 0x00002aaaaded4f6d in nsJSContext::CC () at /home/asrail/mozbuilds/mozilla/dom/src/base/nsJSEnvironment.cpp:3313
#28 0x00002aaaaded4fd1 in nsJSContext::MaybeCC (aHigherProbability=<value optimized out>)
    at /home/asrail/mozbuilds/mozilla/dom/src/base/nsJSEnvironment.cpp:3335
#29 0x00002aaaaded5152 in nsJSContext::Notify (this=0x111b0d0, timer=<value optimized out>)
    at /home/asrail/mozbuilds/mozilla/dom/src/base/nsJSEnvironment.cpp:3376
#30 0x00002ba09bb58a72 in nsTimerImpl::Fire (this=0x1f2d2c0)
    at /home/asrail/mozbuilds/mozilla/xpcom/threads/nsTimerImpl.cpp:403
#31 0x00002ba09bb58ba5 in nsTimerEvent::Run (this=0x2aaab41f0980)
    at /home/asrail/mozbuilds/mozilla/xpcom/threads/nsTimerImpl.cpp:487
#32 0x00002ba09bb54eac in nsThread::ProcessNextEvent (this=0x69b620, mayWait=1, result=0x7fff0fb4972c)
    at /home/asrail/mozbuilds/mozilla/xpcom/threads/nsThread.cpp:510
#33 0x00002ba09bb0c27f in NS_ProcessNextEvent_P (thread=0x1180, mayWait=1) at nsThreadUtils.cpp:227
#34 0x00002aaaac05664a in nsBaseAppShell::Run (this=0x818a80)
    at /home/asrail/mozbuilds/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:154
#35 0x00002aaaace00144 in nsAppStartup::Run (this=0x8593b0)
    at /home/asrail/mozbuilds/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:181
#36 0x00002ba09b696fd8 in XRE_main (argc=<value optimized out>, argv=<value optimized out>, aAppData=<value optimized out>)
    at /home/asrail/mozbuilds/mozilla/toolkit/xre/nsAppRunner.cpp:3172
#37 0x0000000000400e13 in main (argc=5, argv=0x7fff0fb49ec8) at /home/asrail/mozbuilds/mozilla/suite/app/nsSuiteApp.cpp:99

Interesting enough, it crashed with a slightly different stacktrace just after submitting the last comment.

Changing platform, as I've hit this on Linux.

Adding as blocker of bug 333078 (XPCOM Cycle Collector mega bug).
OS: Mac OS X → All
Hardware: PC → All
Keywords: dogfood, regression
Removing dogfood keyword, since this bug is older than the regression I'm hitting.  I guess I'm hitting bug 410323.
Keywords: dogfood, regression
Attached patch v1 (obsolete) — Splinter Review
Applies on top of the patch for bug 402966.
Attached patch v1.1 (obsolete) — Splinter Review
Attachment #295538 - Attachment is obsolete: true
Depends on: 402966
Attached patch v1.1 (obsolete) — Splinter Review
Attachment #295794 - Attachment is obsolete: true
Comment on attachment 296345 [details] [diff] [review]
v1.1

This moves unlinking of JS objects into the Root method of participant helpers and calls Unlink only after the GC has ended. Unlinking held JS objects doesn't call back into JS, we're just nulling pointers to objects that are being collected by the GC. Calling Unlink could call back into JS, so needs to happen after GC (from JSGC_END). I opted for overloading the Root callback instead of adding another callback for unlinking JS objects (which would have to be called for all objects, even those that don't hold JS objects).

nsJSEventListener is misnamed, it also holds non-JS objects so it needs to use HoldScriptObject/DropScriptObjects. However, it should only release the JS objects from Root, other types of script ojects should still be released from Unlink. A bit ugly, but I don't think there's an easy way to fix this cleanly.

We need to be able to get the participant helper and the cycle collection nsISupports pointer of the nsXPCWrappedJS even when it has released its JS objects (so need to move the IsValid check in QueryInterface).
Attachment #296345 - Flags: superreview?(jonas)
Attachment #296345 - Flags: review?(dbaron)
Comment on attachment 296345 [details] [diff] [review]
v1.1

Is there a way that you can assert that all the expected unlinking of JS objects has happened by the time Root is done?

>+//NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsXULPrototypeNode, AddRef)

Why is this here?
And you could explain the nsJSEventListener changes?
(In reply to comment #14)
> (From update of attachment 296345 [details] [diff] [review])
> Is there a way that you can assert that all the expected unlinking of JS
> objects has happened by the time Root is done?

Maybe by tracing the objects and asserting when we get a NoteScriptChild callback for a JS object.

> >+//NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsXULPrototypeNode, AddRef)
> 
> Why is this here?

Just something I forgot to delete. Also, I should probably make NS_IMPL_CYCLE_COLLECTION_ROOT_BEGIN_NATIVE take the rooting function as an argument (to make it more similar to NS_IMPL_CYCLE_COLLECTION_ROOT_BEGIN and to NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE).
(In reply to comment #15)
> And you could explain the nsJSEventListener changes?

There's two ways to hold script objects: nsContentUtils::HoldScriptObject/DropScriptObjects for script objects of any script language (including JS) and nsContentUtils::HoldJSObjects/DropJSObjects (callable through NS_HOLD_JS_OBJECTS/NS_DROP_JS_OBJECTS) for JS objects. nsContentUtils::HoldScriptObject/DropScriptObjects just calls nsContentUtils::HoldJSObjects/DropJSObjects for JS. I missed that when I added cycle collection to it, but nsJSEventListener holds script objects of languages other than JS, so we need to use nsContentUtils::HoldScriptObject/DropScriptObjects. For JS objects we need to release from Root and for other languages from Unlink. So in Root we can use NS_DROP_JS_OBJECTS if the language is JS, in Unlink we use DropScriptObjects.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.9 M11
It seems like this would crash a lot given that the ROOT_BEGIN / ROOT_END macros don't write out a call to nsXPCOMCycleCollectionParticipant::Root.  Shouldn't they?

My other main concern here is maintainability.  Maybe nsCycleCollectionParticipant::Root should be renamed to RootAndUnlinkJS to make things less confusing?

Other than that, I think this looks fine.
... so r=dbaron (or r+sr=dbaron) with those issues addressed.
(In reply to comment #18)
> It seems like this would crash a lot given that the ROOT_BEGIN / ROOT_END
> macros don't write out a call to nsXPCOMCycleCollectionParticipant::Root. 
> Shouldn't they?

They do the work that nsXPCOMCycleCollectionParticipant::Root does themselves (NS_ADDREF(s)), but I'll make them call nsXPCOMCycleCollectionParticipant::Root instead, that seems better.

> My other main concern here is maintainability.  Maybe
> nsCycleCollectionParticipant::Root should be renamed to RootAndUnlinkJS to make
> things less confusing?

I don't like this approach a whole lot, but adding another callback seems worse :-/. I'll rename.
No longer blocks: 415001
Depends on: 415001
Attachment #296345 - Attachment is obsolete: true
Attachment #300566 - Flags: superreview+
Attachment #300566 - Flags: review+
Attachment #296345 - Flags: superreview?(jonas)
Attachment #296345 - Flags: review?(dbaron)
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: