342074 - JSD crashes on exit (in debug, somewhere else in opt) when there's two clients [@ JSD_ASSERT_VALID_CONTEXT]

Status: RESOLVED FIXED

(Other Applications Graveyard :: Venkman JS Debugger, defect)

Description

[:Gijs (he/him)]

jsdService::Pause should nullcheck mCx

I'll try to get a patch finished tonight.

I fail at noting the crasher frame correctly in the summary. Someone with time & knowledge, please fix. Stack attached shortly.

Comment 1

[:Gijs (he/him)]

>	jsd3250.dll!JSD_ASSERT_VALID_CONTEXT(JSDContext * jsdc=0x00000000)  Line 65 + 0x3	C
 	jsd3250.dll!JSD_SetErrorReporter(JSDContext * jsdc=0x00000000, unsigned int (JSDContext *, JSContext *, const char *, JSErrorReport *, void *)* reporter=0x00000000, void * callerdata=0x00000000)  Line 866 + 0x9	C
 	jsd3250.dll!jsdService::Pause(unsigned int * _rval=0x0012c88c)  Line 2590 + 0x10	C++
 	xpcom_core.dll!XPTC_InvokeByIndex(nsISupports * that=0x00da0958, unsigned int methodIndex=0x00000022, unsigned int paramCount=0x00000001, nsXPTCVariant * params=0x0012c88c)  Line 102	C++
 	xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2155 + 0x1e	C++
 	xpc3250.dll!XPC_WN_CallMethod(JSContext * cx=0x02c3b5d0, JSObject * obj=0x03941fe8, unsigned int argc=0x00000000, long * argv=0x0587c854, long * vp=0x0012cb68)  Line 1445 + 0xe	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x02c3b5d0, unsigned int argc=0x00000000, unsigned int flags=0x00000000)  Line 1188 + 0x20	C
 	js3250.dll!js_Interpret(JSContext * cx=0x02c3b5d0, unsigned char * pc=0x0394a1da, long * result=0x0012d6b4)  Line 3583 + 0xf	C
 	js3250.dll!js_Invoke(JSContext * cx=0x02c3b5d0, unsigned int argc=0x00000001, unsigned int flags=0x00000002)  Line 1208 + 0x13	C
 	xpc3250.dll!nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS * wrapper=0x0393cd38, unsigned short methodIndex=0x0007, const nsXPTMethodInfo * info=0x0393d368, nsXPTCMiniVariant * nativeParams=0x0012da18)  Line 1414 + 0x14	C++
 	xpc3250.dll!nsXPCWrappedJS::CallMethod(unsigned short methodIndex=0x0007, const nsXPTMethodInfo * info=0x0393d368, nsXPTCMiniVariant * params=0x0012da18)  Line 468	C++
 	xpcom_core.dll!PrepareAndDispatch(nsXPTCStubBase * self=0x0393cd38, unsigned int methodIndex=0x00000007, unsigned int * args=0x0012dae0, unsigned int * stackBytesToPop=0x0012dad0)  Line 117 + 0x1c	C++
 	xpcom_core.dll!SharedStub()  Line 147	C++
 	xpcom_core.dll!XPTC_InvokeByIndex(nsISupports * that=0x0393cd38, unsigned int methodIndex=0x00000007, unsigned int paramCount=0x00000001, nsXPTCVariant * params=0x0012dc38)  Line 102	C++
 	xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2155 + 0x1e	C++
 	xpc3250.dll!XPC_WN_CallMethod(JSContext * cx=0x02c3b5d0, JSObject * obj=0x03941868, unsigned int argc=0x00000001, long * argv=0x0587c7a4, long * vp=0x0012df14)  Line 1445 + 0xe	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x02c3b5d0, unsigned int argc=0x00000001, unsigned int flags=0x00000000)  Line 1188 + 0x20	C
 	js3250.dll!js_Interpret(JSContext * cx=0x02c3b5d0, unsigned char * pc=0x032ef7f2, long * result=0x0012ea60)  Line 3583 + 0xf	C
 	js3250.dll!js_Invoke(JSContext * cx=0x02c3b5d0, unsigned int argc=0x00000000, unsigned int flags=0x00000002)  Line 1208 + 0x13	C
 	js3250.dll!js_InternalInvoke(JSContext * cx=0x02c3b5d0, JSObject * obj=0x036604d8, long fval=0x03734778, unsigned int flags=0x00000000, unsigned int argc=0x00000000, long * argv=0x00000000, long * rval=0x0012eb70)  Line 1285 + 0x14	C
 	js3250.dll!JS_CallFunctionValue(JSContext * cx=0x02c3b5d0, JSObject * obj=0x036604d8, long fval=0x03734778, unsigned int argc=0x00000000, long * argv=0x00000000, long * rval=0x0012eb70)  Line 4184 + 0x1f	C
 	gklayout.dll!nsXBLProtoImplAnonymousMethod::Execute(nsIContent * aBoundElement=0x0343a828)  Line 344 + 0x1a	C++
 	gklayout.dll!nsXBLPrototypeBinding::BindingDetached(nsIContent * aBoundElement=0x0343a828)  Line 398 + 0x12	C++
 	gklayout.dll!nsXBLBinding::ExecuteDetachedHandler()  Line 797	C++
 	gklayout.dll!ExecuteDetachedHandler(void * aBinding=0x03664ee0, void * aClosure=0x00000000)  Line 827	C++
 	xpcom_core.dll!nsVoidArray::EnumerateForwards(int (void *, void *)* aFunc=0x02476950, void * aData=0x00000000)  Line 648 + 0x15	C++
 	gklayout.dll!nsBindingManager::ExecuteDetachedHandlers()  Line 839	C++
 	gklayout.dll!nsGlobalWindow::HandleDOMEvent(nsPresContext * aPresContext=0x02cbf710, nsEvent * aEvent=0x0012ecfc, nsIDOMEvent * * aDOMEvent=0x0012ecc0, unsigned int aFlags=0x00000007, nsEventStatus * aEventStatus=0x0012ecf8)  Line 1623	C++
 	gklayout.dll!DocumentViewerImpl::PageHide(int aIsUnload=0x00000001)  Line 1206 + 0x23	C++
 	docshell.dll!nsDocShell::FirePageHideNotification(int aIsUnload=0x00000001)  Line 923	C++
 	docshell.dll!nsDocShell::Destroy()  Line 3480	C++
 	appshell.dll!nsXULWindow::Destroy()  Line 514	C++
 	appshell.dll!nsWebShellWindow::Destroy()  Line 850 + 0x9	C++
 	appshell.dll!nsWebShellWindow::HandleEvent(nsGUIEvent * aEvent=0x0012f00c)  Line 408	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012f00c, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1349 + 0xa	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012f00c)  Line 1370	C++
 	gkwidget.dll!nsWindow::DispatchStandardEvent(unsigned int aMsg=0x00000065)  Line 1389 + 0xf	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=0x00000010, unsigned int wParam=0x00000000, long lParam=0x00000000, long * aRetValue=0x0012f4bc)  Line 4501	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000707ce, unsigned int msg=0x00000010, unsigned int wParam=0x00000000, long lParam=0x00000000)  Line 1538 + 0x1b	C++
 	user32.dll!77d18734() 	
 	user32.dll!77d18816() 	
 	user32.dll!77d1b4c0() 	
 	user32.dll!77d1b50c() 	
 	ntdll.dll!7c90eae3() 	
 	user32.dll!77d194be() 	
 	user32.dll!77d1b42d() 	
 	js3250.dll!js_MarkGCThing(JSContext * cx=0x0012f680, void * thing=0x77d1b3f9, void * arg=0x000707ce)  Line 1455 + 0x9	C
 	ntdll.dll!7c910d5c() 	
 	user32.dll!77d1b393() 	
 	gkwidget.dll!nsWindow::DefaultWindowProc(HWND__ * hWnd=0x000707ce, unsigned int msg=0x00000112, unsigned int wParam=0x0000f060, long lParam=0x000004ff)  Line 1564	C++
 	user32.dll!77d18734() 	
 	user32.dll!77d18816() 	
 	user32.dll!77d1c63f() 	
 	user32.dll!77d1c665() 	
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000707ce, unsigned int msg=0x00000112, unsigned int wParam=0x0000f060, long lParam=0x000004ff)  Line 1545 + 0x1f	C++
 	user32.dll!77d18734() 	
 	user32.dll!77d18816() 	
 	user32.dll!77d1b4c0() 	
 	user32.dll!77d1b50c() 	
 	ntdll.dll!7c90eae3() 	
 	user32.dll!77d194be() 	
 	user32.dll!77d1b42d() 	
 	user32.dll!77d18734() 	
 	user32.dll!77d184fc() 	
 	user32.dll!77d185a4() 	
 	user32.dll!77d1b3f9() 	
 	user32.dll!77d1b393() 	
 	gkwidget.dll!nsWindow::DefaultWindowProc(HWND__ * hWnd=0x000707ce, unsigned int msg=0x000000a1, unsigned int wParam=0x00000014, long lParam=0x000004ff)  Line 1564	C++
 	user32.dll!77d18734() 	
 	user32.dll!77d18816() 	
 	user32.dll!77d1c63f() 	
 	user32.dll!77d1c665() 	
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000707ce, unsigned int msg=0x000000a1, unsigned int wParam=0x00000014, long lParam=0x000004ff)  Line 1545 + 0x1f	C++
 	user32.dll!77d18734() 	
 	user32.dll!77d18816() 	
 	user32.dll!77d189cd() 	
 	user32.dll!77d19402() 	
 	user32.dll!77d18a10() 	
 	gkwidget.dll!nsAppShell::Run()  Line 135	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 151 + 0x1a	C++
 	firefox.exe!XRE_main(int argc=0x00000001, char * * argv=0x00b86ee0, const nsXREAppData * aAppData=0x00423090)  Line 2376 + 0x23	C++
 	firefox.exe!main(int argc=0x00000001, char * * argv=0x00b86ee0)  Line 61 + 0x12	C++
 	firefox.exe!mainCRTStartup()  Line 398 + 0x11	C
 	kernel32.dll!7c816d4f() 	
 	kernel32.dll!7c8399f3()

Comment 2

[:Gijs (he/him)]

Attachment: Patch against branch

Patch to nullcheck mCx before we let the world die on us.

I'm still checking whether that actually fixes the problem. Meanwhile, this is also happily broken on trunk just as much:
http://lxr.mozilla.org/seamonkey/source/js/jsd/jsd_xpc.cpp#2599

It *should* apply on trunk too. Should.

Comment 3

[:Gijs (he/him)]

Comment on attachment 226229 [details] [diff] [review]
Patch against branch

Motion to add a flag bugzilla-you-suck+

Really targeting those requests now...

Comment 4

[:Gijs (he/him)]

Tested on branch, does indeed fix the crasher (I'm ignoring the loooad of assertions for some better time some other day - at least we're not dead anymore).

Comment 5

[:Gijs (he/him)]

*** Bug 339531 has been marked as a duplicate of this bug. ***

Comment 6

[timeless]

Comment on attachment 226229 [details] [diff] [review]
Patch against branch

patch both functions and just get a review from me. i'm a real peer and this is a fast review.

Comment 7

[:Gijs (he/him)]

Attachment: New Patch w/ ::Unpause patched

New patch with ::Unpause changed as well. Not sure how necessary that is considering this checks mOn (though I suppose returning NS_OK when you haven't done anything is not entirely right).

Comment 8

[:Gijs (he/him)]

Checked in, marking FIXED.

Comment 9

[:Gijs (he/him)]

Comment on attachment 226300 [details] [diff] [review]
New Patch w/ ::Unpause patched

I'm wondering if this could land on the 1.8.0 branch. It's a crash fix, it's extremely low risk (just a null check, in code that's only used by js debuggers (so not all that often)). Not having this fix in my normal browser (which I do use for checking modifications to Venkman and Firebug) does annoy the hell out of me since it'll crash on the closing of almost every session (due to me using the both of those extensions, see comment #0).

Comment 10

[Jay Patel [:jay]]

Comment on attachment 226300 [details] [diff] [review]
New Patch w/ ::Unpause patched

Missed the 1.8.0.5 train, please try to get this in for the next release.

Comment 11

[Jay Patel [:jay]]

Comment on attachment 226300 [details] [diff] [review]
New Patch w/ ::Unpause patched

Dveditz said he can check this in, so approved for 1.8.0 branch, a=jay for drivers.

Comment 12

[Daniel Veditz [:dveditz]]

Checked in on 1.8.0 branch

Comment 13

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch by code inspection.