341966 - Crash [@ nsSupportsArrayEnumerator::~nsSupportsArrayEnumerator] within iterator_close

Status: RESOLVED WORKSFORME

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

If I use Random JS with its default parameters in a Mac debug build, I usually get the following crash sometime between 400 and 500.  It involves GC so I'm not sure how to go about making a testcase.

Thread 0 Crashed:
0   libxpcom_core.dylib       	0x0132f134 nsSupportsArrayEnumerator::~nsSupportsArrayEnumerator [in-charge]() + 60 (nsSupportsArrayEnumerator.cpp:50)
1   libxpcom_core.dylib       	0x0132f3f8 nsSupportsArrayEnumerator::Release() + 340 (nsSupportsArrayEnumerator.cpp:53)
2   libxpconnect.dylib        	0x02a1e38c nsXPCComponents_Interfaces::NewEnumerate(nsIXPConnectWrappedNative*, JSContext*, JSObject*, unsigned, long*, long*, int*) + 1292 (xpccomponents.cpp:216)
3   libxpconnect.dylib        	0x02a61420 XPC_WN_JSOp_Enumerate(JSContext*, JSObject*, JSIterateOp, long*, long*) + 556 (xpcwrappednativejsops.cpp:1198)
4   libmozjs.dylib            	0x0108cc60 iterator_close + 328 (jsiter.c:131)
5   libmozjs.dylib            	0x010609e4 ExecuteCloseHooks + 472 (jsgc.c:860)
6   libmozjs.dylib            	0x01064538 js_GC + 4420 (jsgc.c:2629)
7   libmozjs.dylib            	0x01062f2c js_ForceGC + 132 (jsgc.c:2101)
8   libmozjs.dylib            	0x01010454 JS_GC + 116 (jsapi.c:1908)
9   libmozjs.dylib            	0x010104f8 JS_MaybeGC + 144 (jsapi.c:1975)
10  libgklayout.dylib         	0x0d34578c nsJSContext::ScriptEvaluated(int) + 260 (nsJSEnvironment.cpp:2828)
11  libgklayout.dylib         	0x0d345a4c nsJSContext::ScriptExecuted() + 60 (nsJSEnvironment.cpp:2904)
12  libxpconnect.dylib        	0x02a4c394 AutoScriptEvaluate::~AutoScriptEvaluate [in-charge]() + 344 (xpcwrappedjsclass.cpp:106)
13  libxpconnect.dylib        	0x02a50780 nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) + 9236 (xpcwrappedjsclass.cpp:1667)
14  libxpconnect.dylib        	0x02a461b8 nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) + 164 (xpcwrappedjs.cpp:465)
15  libxpcom_core.dylib       	0x013aaf38 PrepareAndDispatch + 1516 (xptcstubs_ppc_rhapsody.cpp:184)
16  libxpcom_core.dylib       	0x013abab8 SharedStub + 112 (xptcstubsdef.inc:256)
17  libnsappshell.dylib       	0x07ca8890 nsContentTreeOwner::SetStatusWithContext(unsigned, nsAString_internal const&, nsISupports*) + 328 (nsContentTreeOwner.cpp:434)
18  libnsappshell.dylib       	0x07ca4dc4 nsContentTreeOwner::SetStatus(unsigned, unsigned short const*) + 148 (nsContentTreeOwner.cpp:460)
19  libgklayout.dylib         	0x0d36666c nsGlobalWindow::SetStatus(nsAString_internal const&) + 484 (nsGlobalWindow.cpp:2451)
20  libxpcom_core.dylib       	0x013ab278 _XPTC_InvokeByIndex + 216 (xptcstubsdef.inc:256)
21  libxpcom_core.dylib       	0x013aa930 XPTC_InvokeByIndex + 56 (xptcinvoke_ppc_rhapsody.cpp:144)
22  libxpconnect.dylib        	0x02a55740 XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) + 5252 (xpcwrappednative.cpp:2154)
23  libxpconnect.dylib        	0x02aa3f7c XPCWrappedNative::SetAttribute(XPCCallContext&) + 36 (xpcwrappednativejsops.cpp:1993)
24  libxpconnect.dylib        	0x02a5f450 XPC_WN_GetterSetter(JSContext*, JSObject*, unsigned, long*, long*) + 496 (xpcwrappednativejsops.cpp:1474)
25  libmozjs.dylib            	0x010688cc js_Invoke + 4928 (jsinterp.c:1328)
26  libmozjs.dylib            	0x01068dcc js_InternalInvoke + 444 (jsinterp.c:1422)
27  libmozjs.dylib            	0x01069254 js_InternalGetOrSet + 888 (jsinterp.c:1482)
28  libmozjs.dylib            	0x010a5b80 js_SetProperty + 1280 (jsobj.c:3329)
29  libmozjs.dylib            	0x0107b834 js_Interpret + 66964 (jsinterp.c:3769)
30  libmozjs.dylib            	0x01068964 js_Invoke + 5080 (jsinterp.c:1347)
31  libmozjs.dylib            	0x01068dcc js_InternalInvoke + 444 (jsinterp.c:1422)
32  libmozjs.dylib            	0x0101ab74 JS_CallFunctionValue + 76 (jsapi.c:4348)
33  libgklayout.dylib         	0x0d34fed8 nsJSContext::CallEventHandler(nsISupports*, void*, void*, nsIArray*, nsIVariant**) + 884 (nsJSEnvironment.cpp:1585)
34  libgklayout.dylib         	0x0d37dee8 nsGlobalWindow::RunTimeout(nsTimeout*) + 2064 (nsGlobalWindow.cpp:6491)
35  libgklayout.dylib         	0x0d37e47c nsGlobalWindow::TimerCallback(nsITimer*, void*) + 72 (nsGlobalWindow.cpp:6812)
36  libxpcom_core.dylib       	0x013970dc nsTimerImpl::Fire() + 896 (nsTimerImpl.cpp:401)
37  libxpcom_core.dylib       	0x01397e10 nsTimerEvent::Run() + 504 (nsTimerImpl.cpp:486)
38  libxpcom_core.dylib       	0x013915b4 nsThread::ProcessNextEvent(int, int*) + 796 (nsThread.cpp:483)
39  libxpcom_core.dylib       	0x0130eda4 NS_ProcessNextEvent_P(nsIThread*, int) + 172 (nsThreadUtils.cpp:225)
40  libwidget_mac.dylib       	0x061aa514 nsBaseAppShell::Run() + 156 (nsBaseAppShell.cpp:152)
41  libtoolkitcomps.dylib     	0x06d224a8 nsAppStartup::Run() + 220 (nsAppStartup.cpp:171)
42  XUL                       	0x0020ff9c XRE_main + 8740 (nsAppRunner.cpp:2349)
43  org.mozilla.firefox       	0x00002c78 main + 56 (nsBrowserApp.cpp:61)
44  org.mozilla.firefox       	0x00002498 _start + 340 (crt.c:272)
45  org.mozilla.firefox       	0x00002340 start + 60

Comment 1

[Jesse Ruderman]

<Jesse_> mrbkap: is it likely that you guys will figure out [this bug] without a reduced testcase?
<Jesse_> mrbkap: i think i know what would be involved in making a reduced testcase but it would take me almost a day
<mrbkap> So, igor and brendan have been talking about moving the calls to close hooks around a bunch, so it's possible that that bug will be magically fixed with whatever they come up with.

I'm marking this as depending on bug 341821, "Case for running close hooks outside GC".

Comment 2

[Jesse Ruderman]

Attachment: small testcase

A function returns while iterating through Components.interfaces.  Firefox crashes after 1 or 2 GCs.

This testcase requires Venkman and UniversalXPConnect to force GCs.  (Venkman does work on trunk, surprisingly enough.)

Comment 3

[Jesse Ruderman]

Attachment: small testcase

With typo fix.

Comment 4

[Jesse Ruderman]

Yesterday's nightly crashes, but today's doesn't.  My debug build also doesn't crash any more.  Fixed by the patch in bug 341877, "GC hazard with for-in loop"?

Comment 5

[Igor Bukanov]

Taking this to investigate why fixes for bug 341877 apparently addressed this bug as well.

Comment 6

[Igor Bukanov]

Some benchmarking data

To measure timing I use the following test for an optimized build of js shell:

function test(N)
{
	var array = [];
	for (var i = 0; i != N; ++i) {
		array.push(new Iterator([]));
	}

	array = null;
	var now = Date.now;
	var time0 = now();
	gc();
	var time1 = now();
	gc();
	var time2 = now();
	print("Times: GC1="+(time1-time0)+" GC2="+(time2 - time1)+" Total="+(time2-time0));
}

test(1000*1000);


Typical output on my Pentium-M 1.1GH laptop with Fedora Core 5:

Before the patch:

Times: GC1=1566 GC2=637 Total=2203

After the patch:

Times: GC1=978 GC2=1 Total=979

That shows that not only that the total time to collect the garbage more then 2 times smaller but also that marking the iterator is almost 2 times longer then finalizing it. 

Comment 7

[Igor Bukanov]

Please ignore the previous comment: wrong bug again :(((

Comment 8

[Igor Bukanov]

The fix for bug 341877 could affect address this bug directly since it dealt with rooting of iterators for objects that does not provide its own  JSCLASS_NEW_ENUMERATE hook. Thus extra rooting that patch provided did not affect the enumeration from the test case which is provided by object with own enumerating hook. 

But what exactly fixed the bug then?

Comment 9

[Daniel Veditz [:dveditz]]

Can we close this bug WFM, or are you still investigating?

Comment 10

[Igor Bukanov]

(In reply to comment #9)
> Can we close this bug WFM, or are you still investigating?
> 

No, so WFM.