294846 - misplaced null checks in NS_ScriptErrorReporter

Status: RESOLVED FIXED

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

Description

[timeless]

bug 220603 introduced a new way to crash for a certain error path:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/dom/src/base/nsJSEnvironment.cpp&rev=1.203&mark=180,194,162-166,230-236#177

as it happens, we were already crashing:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/dom/src/base/nsJSEnvironment.cpp&rev=1.84&mark=131,133,135,137,103-110,147-158#127

since the code for the most part tries fairly hard to handle the case where
there's no report, i'd like to fix it to actually work.

Comment 1

[timeless]

Attachment: rearrange null checks and don't try to send a dom error for OOM

the oom path i followed to notice these problems:

	report->errorNumber	0x00000089	unsigned int
(JSMSG_OUT_OF_MEMORY)

>	gklayout.dll!NS_ScriptErrorReporter(JSContext * cx=0x019b49f8, const
char * message=0x012d9d5c, JSErrorReport * report=0x0012e834)  Line 250    C++
	js3250.dll!js_ReportOutOfMemory(JSContext * cx=0x019b49f8, const
JSErrorFormatString * (void *, const char *, const unsigned int)*
callback=0x011f1e97)  Line 746 + 0xf C
	js3250.dll!JS_ReportOutOfMemory(JSContext * cx=0x019b49f8)  Line 4211 +
0xe	C
	js3250.dll!js_NewGCThing(JSContext * cx=0x019b49f8, unsigned int
flags=0x00000000, unsigned int nbytes=0x00000008)  Line 719 + 0x9      C
	js3250.dll!js_NewObject(JSContext * cx=0x019b49f8, JSClass *
clasp=0x012e0c58, JSObject * proto=0x01831168, JSObject * parent=0x01831160) 
Line 1880 + 0xd      C
	js3250.dll!js_NewFunction(JSContext * cx=0x019b49f8, JSObject *
funobj=0x00000000, int (JSContext *, JSObject *, unsigned int, long *, long *)*
native=0x0129d690, unsigned int nargs=0x00000001, unsigned int
flags=0x00000000, JSObject * parent=0x01831160, JSAtom * atom=0x010e1600)  Line
1934 + 0x14	 C
	js3250.dll!js_DefineFunction(JSContext * cx=0x019b49f8, JSObject *
obj=0x01831160, JSAtom * atom=0x010e1600, int (JSContext *, JSObject *,
unsigned int, long *, long *)* native=0x0129d690, unsigned int
nargs=0x00000001, unsigned int attrs=0x00000000)  Line 1996 + 0x1f    C
	js3250.dll!JS_DefineFunction(JSContext * cx=0x019b49f8, JSObject *
obj=0x01831160, const char * name=0x012c689c, int (JSContext *, JSObject *,
unsigned int, long *, long *)* call=0x0129d690, unsigned int nargs=0x00000001,
unsigned int attrs=0x00000000)	Line 3233 + 0x1d  C
	js3250.dll!JS_DefineFunctions(JSContext * cx=0x019b49f8, JSObject *
obj=0x01831160, JSFunctionSpec * fs=0x012fa4b8)  Line 3215 + 0x2a   C
	js3250.dll!js_InitStringClass(JSContext * cx=0x019b49f8, JSObject *
obj=0x01831160)  Line 2440 + 0x12   C
	js3250.dll!JS_ResolveStandardClass(JSContext * cx=0x019b49f8, JSObject
* obj=0x01831160, long id=0x0106c4bc, int * resolved=0x0012eb90)  Line 1424 +
0xb	   C
	gklayout.dll!nsWindowSH::NewResolve(nsIXPConnectWrappedNative *
wrapper=0x03a98d38, JSContext * cx=0x019b49f8, JSObject * obj=0x01831160, long
id=0x0106c4bc, unsigned int flags=0x00000010, JSObject * * objp=0x0012ecb8, int
* _retval=0x0012ec3c)  Line 5129 + 0x16  C++
	xpc3250.dll!XPC_WN_Helper_NewResolve(JSContext * cx=0x019b49f8,
JSObject * obj=0x01831160, long idval=0x0106c4bc, unsigned int
flags=0x00000010, JSObject * * objp=0x0012ed38)  Line 951 + 0x45 C++
	js3250.dll!js_LookupPropertyWithFlags(JSContext * cx=0x019b49f8,
JSObject * obj=0x01831160, long id=0x01079868, unsigned int flags=0x00000010,
JSObject * * objp=0x0012ed8c, JSProperty * * propp=0x0012ed80)	Line 2524 +
0x4c C
	js3250.dll!js_FindConstructor(JSContext * cx=0x019b49f8, JSObject *
start=0x00000000, const char * name=0x012c50b4, long * vp=0x0012edbc)  Line
1984 + 0x1b	C
	js3250.dll!GetClassPrototype(JSContext * cx=0x019b49f8, JSObject *
scope=0x00000000, const char * name=0x012c50b4, JSObject * * protop=0x0012ee20)
 Line 3625 + 0x15    C
	js3250.dll!js_NewObject(JSContext * cx=0x019b49f8, JSClass *
clasp=0x012fa528, JSObject * proto=0x00000000, JSObject * parent=0x00000000) 
Line 1864 + 0x17     C
	js3250.dll!js_StringToObject(JSContext * cx=0x019b49f8, JSString *
str=0x01830838)  Line 2667 + 0x12    C
	js3250.dll!js_ValueToObject(JSContext * cx=0x019b49f8, long
v=0x0183083c, JSObject * * objp=0x0012ee6c)  Line 3686 + 0x10	    C
	js3250.dll!js_ValueToNonNullObject(JSContext * cx=0x019b49f8, long
v=0x0183083c)  Line 3708 + 0x11      C
	js3250.dll!js_Interpret(JSContext * cx=0x019b49f8, unsigned char *
pc=0x019a1c5d, long * result=0x0012f7f0)  Line 3437 + 0x7e   C
	js3250.dll!js_Execute(JSContext * cx=0x019b49f8, JSObject *
chain=0x01831160, JSScript * script=0x019a1bb8, JSStackFrame * down=0x00000000,
unsigned int flags=0x00000000, long * result=0x0012f8f8)  Line 1550 + 0x13  C
	js3250.dll!JS_EvaluateUCScriptForPrincipals(JSContext * cx=0x019b49f8,
JSObject * obj=0x01831160, JSPrincipals * principals=0x01967c24, const unsigned
short * chars=0x019a1080, unsigned int length=0x0000055e, const char *
filename=0x019abf98, unsigned int lineno=0x00000001, long * rval=0x0012f8f8) 
Line 3813 + 0x19    C
	gklayout.dll!nsJSContext::EvaluateString(const nsAString &
aScript={...}, void * aScopeObject=0x01831160, nsIPrincipal *
aPrincipal=0x01967c20, const char * aURL=0x019abf98, unsigned int
aLineNo=0x00000001, const char * aVersion=0x012c5854, nsAString *
aRetValue=0x00000000, int * aIsUndefined=0x0012f9c8)  Line 1038 + 0x43	   C++
	gklayout.dll!nsScriptLoader::EvaluateScript(nsScriptLoadRequest *
aRequest=0x019abe48, const nsString & aScript={...})  Line 723	      C++
	gklayout.dll!nsScriptLoader::ProcessRequest(nsScriptLoadRequest *
aRequest=0x019abe48)  Line 629 + 0x13 C++
	gklayout.dll!nsScriptLoader::OnStreamComplete(nsIStreamLoader *
aLoader=0x03ae68c8, nsISupports * aContext=0x019abe48, unsigned int
aStatus=0x00000000, unsigned int stringLen=0xffffffff, const unsigned char *
string=0x03a74627)  Line 975   C++
	necko.dll!nsStreamLoader::OnStopRequest(nsIRequest *
request=0x03ae61a8, nsISupports * ctxt=0x019abe48, unsigned int
aStatus=0x00000000)  Line 137	   C++
	necko.dll!nsStreamListenerTee::OnStopRequest(nsIRequest *
request=0x03ae61a8, nsISupports * context=0x019abe48, unsigned int
status=0x00000000)  Line 66	   C++
	necko.dll!nsHttpChannel::OnStopRequest(nsIRequest * request=0x03a73f70,
nsISupports * ctxt=0x00000000, unsigned int status=0x00000000)	Line 3825      
C++
	necko.dll!nsInputStreamPump::OnStateStop()  Line 507	C++
	necko.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream *
stream=0x03a73d18)  Line 343 + 0xb	  C++
	xpcom_core.dll!nsInputStreamReadyEvent::EventHandler(PLEvent *
plevent=0x03a7405c)  Line 120	 C++
	xpcom_core.dll!PL_HandleEvent(PLEvent * self=0x03a7405c)  Line 698 +
0xa	   C
	xpcom_core.dll!PL_ProcessPendingEvents(PLEventQueue * self=0x010e85d0) 
Line 633 + 0x9	C
	xpcom_core.dll!_md_EventReceiverProc(HWND__ * hwnd=0x000c108a, unsigned
int uMsg=0x0000c118, unsigned int wParam=0x00000000, long lParam=0x010e85d0) 
Line 1435 + 0x9   C
	user32.dll!_InternalCallWinProc@20()  + 0x28	
	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7	
	user32.dll!_DispatchMessageWorker@8()  + 0xdc	
	user32.dll!_DispatchMessageA@4()  + 0xf 
	mfc71d.dll!AfxInternalPumpMessage()  Line 188	C++
	mfc71d.dll!CWinThread::PumpMessage()  Line 916	C++
	mfc71d.dll!CWinThread::Run()  Line 637 + 0xb	C++
	mfc71d.dll!CWinApp::Run()  Line 701	C++
	mfc71d.dll!AfxWinMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ *
hPrevInstance=0x00000000, char * lpCmdLine=0x00142384, int nCmdShow=0x0000000a)
 Line 49 + 0xb	C++
	mfcembed.exe!WinMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ *
hPrevInstance=0x00000000, char * lpCmdLine=0x00142384, int nCmdShow=0x0000000a)
 Line 25 C++
	mfcembed.exe!WinMainCRTStartup()  Line 390 + 0x39	C
	kernel32.dll!_BaseProcessStart@4()  + 0x23

Comment 2

[timeless]

Attachment: rearrange null checks and don't try to send a dom error for OOM - include a definition of OOM...

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 184643 [details] [diff] [review]
rearrange null checks and don't try to send a dom error for OOM - include a definition of OOM...

r+sr=jst, but please add a comment regarding why we don't want to report OOM
error messages to the console.

Comment 4

[timeless]

the dom and jsconsole paths can never work because the act of trying to report
to these targets involves js (and creating a couple of js objects), when you're
out of memory (generally this is a spidermonkey controlled resource and not real
os vm), that js fails which triggers a new asynchronous error report. the result
is amusing, you lose all your cpu to continuous attempts at reporting oom. you
also lose any evidence of the original problems as your 250 element
consoleservice buffer is *very* quickly wiped. note that you don't get stack
overflow because the dispatch to consolelisteners is async.

but what you don't get is an actual exception which anyone in js can possibly
handle usefully.

i will include a note akin to this in the code too.

Comment 5

[timeless]

Comment on attachment 184643 [details] [diff] [review]
rearrange null checks and don't try to send a dom error for OOM - include a definition of OOM...

mozilla/dom/src/base/nsJSEnvironment.cpp	1.250