77807 - more gc problems running JS from the new-script hook.

Status: RESOLVED FIXED

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

Description

[Robert Ginda]

I survive hundreds (literally) of new-script hooks called when opening a new
Navigator window, only to crash as the window is about to appear.  The crash
itself is leaves the following skidmarks:

-*- venkman: script created (true):  (anonymous) 2...81
-*- venkman: script created (true):  (anonymous) 2...15
-*- venkman: script created (true):  (anonymous) 19...25
-*- venkman: script created (true):  (anonymous) 47...24
-*- venkman: script created (true):  (anonymous) 73...4
-*- venkman: script created (true):  (anonymous) 79...10
-*- venkman: script created (true):  (anonymous) 93...35
-*- venkman: script created (true):  (anonymous) 131...3
Program received signal SIGSEGV, Segmentation fault.
0x401f1ac7 in JS_HashTableRawLookup (ht=0x8b36e30, keyHash=5902, key=0x8b38478)
    at jshash.c:178
178         while ((he = *hep) != NULL) {
(gdb) where
#0  0x401f1ac7 in JS_HashTableRawLookup (ht=0x8b36e30, keyHash=5902, 
    key=0x8b38478) at jshash.c:178
#1  0x401ce5c4 in js_IndexAtom (cx=0x84eb5e0, atom=0x8b38478, al=0xbfffc154)
    at jsatom.c:732
#2  0x401e18f4 in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b30368)
    at jsemit.c:947
#3  0x401e7335 in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b6caa8)
    at jsemit.c:2631
#4  0x401e756c in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b6ca78)
    at jsemit.c:2676
#5  0x401e5908 in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b36968)
    at jsemit.c:2107
#6  0x40222ef9 in Statements (cx=0x84eb5e0, ts=0x8b6c780, tc=0xbfffc100)
    at jsparse.c:909
#7  0x40221839 in js_CompileTokenStream (cx=0x84eb5e0, chain=0x8a12a28, 
    ts=0x8b6c780, cg=0xbfffc100) at jsparse.c:391
#8  0x401c7599 in CompileTokenStream (cx=0x84eb5e0, obj=0x8a12a28, 
    ts=0x8b6c780, tempMark=0x84eb660, eofp=0x0) at jsapi.c:2798
#9  0x401c780a in JS_CompileUCScriptForPrincipals (cx=0x84eb5e0, 
    obj=0x8a12a28, principals=0x81b812c, chars=0x8bc2308, length=9179, 
    filename=0x0, lineno=0) at jsapi.c:2877
#10 0x401c8490 in JS_EvaluateUCScriptForPrincipals (cx=0x84eb5e0, 
    obj=0x8a12a28, principals=0x81b812c, chars=0x8bc2308, length=9179, 
    filename=0x0, lineno=0, rval=0xbfffc2a0) at jsapi.c:3283
#11 0x4065cf8b in nsJSContext::EvaluateStringWithValue (this=0x850eab8, 
    aScript=@0xbfffc600, aScopeObject=0x8a12a28, aPrincipal=0x0, aURL=0x0, 
    aLineNo=0, aVersion=0x0, aRetValue=0xbfffc97c, aIsUndefined=0xbfffc53c)
    at nsJSEnvironment.cpp:494
#12 0x413f6a90 in nsXBLBinding::InstallProperties (this=0x8b6da60)
    at nsXBLBinding.cpp:1323

The -*- venkman lines are printed by JS called via XPConnect when a new script
is created.  In this case, the final 8 scripts have no file/ function names
(nice), and so I'm guessing nsXBLBinding::InstallProperties is responsible for
all of them.  Setting a breakpoint in js_GC just before the crash, I hit the
following stack:

#0  js_GC (cx=0x84eb5e0, gcflags=0) at jsgc.c:976
#1  0x401f0aeb in js_ForceGC (cx=0x84eb5e0) at jsgc.c:943
#2  0x401c3a3e in JS_GC (cx=0x84eb5e0) at jsapi.c:1633
#3  0x401c3aab in JS_MaybeGC (cx=0x84eb5e0) at jsapi.c:1652
#4  0x4065f4c2 in nsJSContext::ScriptEvaluated (this=0x850eab8, aTerminated=0)
    at nsJSEnvironment.cpp:1345
#5  0x40bc51b0 in DoPostScriptEvaluated (cx=0x84eb5e0)
    at xpcwrappedjsclass.cpp:69
#6  0x40bc4c25 in nsXPCWrappedJSClass::CallMethod (this=0x853e238, 
    wrapper=0x853e268, methodIndex=3, info=0x8535928, nativeParams=0xbfffa518)
    at xpcwrappedjsclass.cpp:1283
#7  0x40bc14b8 in nsXPCWrappedJS::CallMethod (this=0x853e268, methodIndex=3, 
    info=0x8535928, params=0xbfffa518) at xpcwrappedjs.cpp:442
#8  0x40149b6f in PrepareAndDispatch (self=0x853e268, methodIndex=3, 
    args=0xbfffa5e4) at xptcstubs_unixish_x86.cpp:80
#9  0x40149c0a in nsXPTCStubBase::Stub3 (this=0x853e268)
    at ../../../../../../dist/include/xptcstubsdef.inc:5
#10 0x42226983 in jsds_ScriptHookProc (jsdc=0x84febb8, jsdscript=0x8b37be8, 
    creating=1, callerdata=0x853e268) at jsd_xpc.cpp:148
#11 0x42221f17 in jsd_NewScriptHookProc (cx=0x84eb5e0, filename=0x0, 
    lineno=131, script=0x8b37b90, fun=0x8b30748, callerdata=0x84febb8)
    at jsd_scpt.c:420
#12 0x40239fc2 in js_NewScriptFromCG (cx=0x84eb5e0, cg=0xbfffad00, 
    fun=0x8b30748) at jsscript.c:788
#13 0x401e1499 in js_EmitFunctionBody (cx=0x84eb5e0, cg=0xbfffad00, 
    body=0x8b30398, fun=0x8b30748) at jsemit.c:855
#14 0x401e1856 in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b30368)
    at jsemit.c:930
#15 0x401e7335 in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b6caa8)
    at jsemit.c:2631
#16 0x401e756c in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b6ca78)
    at jsemit.c:2676
#17 0x401e5908 in js_EmitTree (cx=0x84eb5e0, cg=0xbfffc100, pn=0x8b36968)
    at jsemit.c:2107
#18 0x40222ef9 in Statements (cx=0x84eb5e0, ts=0x8b6c780, tc=0xbfffc100)
    at jsparse.c:909
#19 0x40221839 in js_CompileTokenStream (cx=0x84eb5e0, chain=0x8a12a28, 
    ts=0x8b6c780, cg=0xbfffc100) at jsparse.c:391
#20 0x401c7599 in CompileTokenStream (cx=0x84eb5e0, obj=0x8a12a28, 
    ts=0x8b6c780, tempMark=0x84eb660, eofp=0x0) at jsapi.c:2798
#21 0x401c780a in JS_CompileUCScriptForPrincipals (cx=0x84eb5e0, 
    obj=0x8a12a28, principals=0x81b812c, chars=0x8bc2308, length=9179, 
    filename=0x0, lineno=0) at jsapi.c:2877
#22 0x401c8490 in JS_EvaluateUCScriptForPrincipals (cx=0x84eb5e0, 
    obj=0x8a12a28, principals=0x81b812c, chars=0x8bc2308, length=9179, 
    filename=0x0, lineno=0, rval=0xbfffc2a0) at jsapi.c:3283
#23 0x4065cf8b in nsJSContext::EvaluateStringWithValue (this=0x850eab8, 
    aScript=@0xbfffc600, aScopeObject=0x8a12a28, aPrincipal=0x0, aURL=0x0, 
    aLineNo=0, aVersion=0x0, aRetValue=0xbfffc97c, aIsUndefined=0xbfffc53c)
    at nsJSEnvironment.cpp:494
#24 0x413f6a90 in nsXBLBinding::InstallProperties (this=0x8b6da60)
    at nsXBLBinding.cpp:1323
#25 0x41411e1d in nsXBLService::LoadBindings (this=0x8218298, 
    aContent=0x86d7530, aURL=@0x8b6d89c, aAugmentFlag=0, aBinding=0xbfffd0d0, 
    aResolveStyle=0xbfffd09c) at nsXBLService.cpp:714

This GC doesn't actually happen, js_GC returns at

jsgc.c:983 if (rt->gcDisabled && !(gcflags & GC_LAST_CONTEXT))
       984     return;

I'm betting that the aborted GC does *something* to precipitate this crash.
I've tried adding roots for the scriptObject and classObject JSObjects created
by nsXBLBinding::InstallProperties, but it hasn't helped.  I'm also guessing the
the GC related comments in nsJSContext::EvaluateStringWithValue are no longer
valid.  I'm going to keep poking around, but feel free to jump in with advice.

Comment 1

[Robert Ginda]

We're starting to GC even if cx->runtime->gcDisabled is set.  Here's the patch.
 r/sr= please.

Index: jsapi.c
===================================================================
RCS file: /cvsroot/mozilla/js/src/jsapi.c,v
retrieving revision 3.97
diff -u -r3.97 jsapi.c
--- jsapi.c     2001/04/11 23:07:25     3.97
+++ jsapi.c     2001/04/27 00:23:41
@@ -1626,6 +1626,9 @@
 JS_PUBLIC_API(void)
 JS_GC(JSContext *cx)
 {
+    if (cx->runtime->gcDisabled)
+        return;
+            
     if (cx->stackPool.current == &cx->stackPool.first)
         JS_FinishArenaPool(&cx->stackPool);
     JS_FinishArenaPool(&cx->codePool);

Comment 2

[Brendan Eich [:brendan]]

Those infernal arenapool finishes!  I helped rginda figure this out, and should
have known sooner (I forget that they were in JS_GC, in jsapi.c -- thought they
were in js_ForceGC in jsgc.c).  sr=[e-mail].

/be

Comment 3

[rogerl (gone)]

r=rogerl
I don't know why calling FinishArenaPool is bad, but I see there's another call 
from jsopcode.c (in js_DestroyPrinter). Does that one not need to be subverted 
likewise?

Comment 4

[Brendan Eich [:brendan]]

rogerl: no, the only question is, what else should rt->gcDisabled suppress that
it does not.  If the compiler is using an arena pool (LIFO, natch) and somehow
the debugger is invoked while compiling, and the invocation goes through a layer
such as XPConnect that may run JS_GC, then we don't want to Finish the arena
pool that's in use (cx->tempPool, cx->notePool, cx->codePool).

The Finish in jsopcode.c is for a JSPrinter's private, wholly-owned arena pool,
which can't be in use be older activations on the C stack.

/be

Comment 5

[Robert Ginda]

Fix Checked in.