326223 - JS_PARANOID_REQUEST dies in spidermonkey

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[timeless]

JS_NewContext itself isn't JS_PARANOID_REQUEST/JS_CHECK_REQUEST safe.
This is before a caller can possible JS_BeginRequest(cx) because the cx hasn't
yet been given back to any caller.

00 ntdll!DbgBreakPoint (FPO: [0,0,0])
01 js3250!JS_Assert(char * s = 0x100efd3c "cx->requestDepth", char * file = 0x100efd20 "r:/mozilla/js/src/jsapi.c", int ln = 4454)+0x2c (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\src\jsutil.c @ 62]
02 js3250!JS_MakeStringImmutable(struct JSContext * cx = 0x00bc7c38, struct JSString * str = 0x00bdf608)+0x26 (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\src\jsapi.c @ 4454]
03 js3250!js_AtomizeString(struct JSContext * cx = 0x00bc7c38, struct JSString * str = 0x00bdf608, unsigned int flags = 1)+0x132 (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\src\jsatom.c @ 671]
04 js3250!js_InitRuntimeStringState(struct JSContext * cx = 0x00bc7c38)+0x68 (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\src\jsstr.c @ 2478]
05 js3250!js_NewContext(struct JSRuntime * rt = 0x00bdbf78, unsigned int stackChunkSize = 0x100)+0x24e (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\src\jscntxt.c @ 174]
06 js3250!JS_NewContext(struct JSRuntime * rt = 0x00bdbf78, unsigned int stackChunkSize = 0x100)+0x10 (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\src\jsapi.c @ 938]
07 jsd3250!_newJSDContext(struct JSRuntime * jsrt = 0x00bdbf78, struct JSD_UserCallbacks * callbacks = 0x00000000, void * user = 0x00000000)+0x1e3 (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\jsd\jsd_high.c @ 154]
08 jsd3250!jsd_DebuggerOnForUser(struct JSRuntime * jsrt = 0x00bdbf78, struct JSD_UserCallbacks * callbacks = 0x00000000, void * user = 0x00000000)+0x1e (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\jsd\jsd_high.c @ 221]
09 jsd3250!JSD_DebuggerOnForUser(struct JSRuntime * jsrt = 0x00bdbf78, struct JSD_UserCallbacks * callbacks = 0x00000000, void * user = 0x00000000)+0x14 (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\jsd\jsdebug.c @ 52]
0a jsd3250!jsdService::OnForRuntime(struct JSRuntime * rt = 0x00bdbf78)+0x5f (FPO: [Non-Fpo]) (CONV: stdcall) [r:\mozilla\js\jsd\jsd_xpc.cpp @ 2613]
0b jsd3250!jsdASObserver::Observe(class nsISupports * aSubject = 0x00000000, char * aTopic = 0x004fb54c "start", unsigned short * aData = 0x005070e4 "")+0x10b (FPO: [Non-Fpo]) (CONV: stdcall) [r:\mozilla\js\jsd\jsd_xpc.cpp @ 3842]
0c xpcom_core!NS_CreateServicesFromCategory(char * category = 0x004fb554 "xpcom-autoregistration", class nsISupports * origin = 0x00000000, char * observerTopic = 0x004fb54c "start")+0x35a (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\xpcom\components\nscategorymanager.cpp @ 896]
0d xpcom_core!nsComponentManagerImpl::AutoRegister(class nsIFile * aSpec = 0x00000000)+0x6d (FPO: [Non-Fpo]) (CONV: stdcall) [r:\mozilla\xpcom\components\nscomponentmanager.cpp @ 3279]
0e xpcshell!main(int argc = 1, char ** argv = 0x003772e8, char ** envp = 0x00372fb8)+0x108 (FPO: [Non-Fpo]) (CONV: cdecl) [r:\mozilla\js\src\xpconnect\shell\xpcshell.cpp @ 1514]
0f xpcshell!mainCRTStartup(void)+0x12c (FPO: [Non-Fpo]) (CONV: cdecl) [f:\vs70builds\3077\vc\crtbld\crt\src\crtexe.c @ 398]
10 kernel32!BaseProcessStart+0x23 (FPO: [Non-Fpo])

Comment 1

[timeless]

Attachment: change JS_NewContext not to kill unfortunate embedders

i'm slowly trying to flush patches from my tree...

Comment 2

[Brendan Eich [:brendan]]

Didn't Michael Daumling just file this?

/be

Comment 3

[Bob Clary [:bc] (inactive)]

bug 325525

Comment 4

[Brendan Eich [:brendan]]

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

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 210996 [details] [diff] [review]
change JS_NewContext not to kill unfortunate embedders

>@@ -173,6 +176,9 @@ js_NewContext(JSRuntime *rt, size_t stac
>             js_DestroyContext(cx, JS_NO_GC);
>             return NULL;
>         }
>+#ifdef JS_THREADSAFE
>+        JS_EndRequest(cx);
>+#endif

Nit: move this up above the early-returning if-then shown in context?  It's slightly tidier for the reader not to have to worry about whether js_DestroyContext implicitly ends any requests on cx (it does, but still).

/be

Comment 6

[timeless]

Comment on attachment 210996 [details] [diff] [review]
change JS_NewContext not to kill unfortunate embedders

mozilla/js/src/jscntxt.c 	3.73
mozilla/js/src/jscntxt.c 	3.74

sorry, i'm so used to knowing that it does, and all of my patches expect that behavior. it's hard not to rely on it, as you will probably see in the much larger patch now on your plate.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 210996 [details] [diff] [review]
change JS_NewContext not to kill unfortunate embedders

Should this go on the branch?

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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

Comment 9

[Brendan Eich [:brendan]]

There's no point in going through all the JS bugs that are fixed in JS1.7 and seeking individual approval, unless you believe there's a fix that's too risky for 1.8.1.  I know of no such fix.

/be

Comment 10

[Mike Schroepfer]

Comment on attachment 210996 [details] [diff] [review]
change JS_NewContext not to kill unfortunate embedders

Minusing given comment 9