307560 - WAY_TOO_MUCH_GC JS eng assert under SetNewDocument

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Boris Zbarsky [:bzbarsky]]

STEPS TO REPRODUCE:

1)  Apply all patches to blockers of bug 307312 that have lower bug numbers than
this bug.
2)  Start mozilla.

Assertion failure: flags != GCF_FINAL, at ../../../mozilla/js/src/jsgc.c:1040
#3  0xb7fd0137 in JS_Assert (s=0xb7fef403 "flags != GCF_FINAL", 
    file=0xb7feeff0 "../../../mozilla/js/src/jsgc.c", ln=1040)
    at ../../../mozilla/js/src/jsutil.c:63
#4  0xb7f6f0f3 in UnmarkedGCThingFlags (thing=0x83928e8, arg=0xbfffc494)
    at ../../../mozilla/js/src/jsgc.c:1040
#5  0xb7f6fde3 in js_MarkGCThing (cx=0x8408278, thing=0x83928e8, arg=0xbfffc494)
    at ../../../mozilla/js/src/jsgc.c:1443
#6  0xb7f38dce in JS_MarkGCThing (cx=0x8408278, thing=0x83928e8, 
    name=0xbfffc4c4 "<local root 2>", arg=0x0) at
../../../mozilla/js/src/jsapi.c:1837
#7  0xb7f484ad in js_MarkLocalRoots (cx=0x8408278, lrs=0x8417bf0)
    at ../../../mozilla/js/src/jscntxt.c:660
#8  0xb7f70d9e in js_GC (cx=0x8408278, gcflags=5) at
../../../mozilla/js/src/jsgc.c:1793
#9  0xb7f6e4c7 in js_NewGCThing (cx=0x8408278, flags=0, nbytes=8)
    at ../../../mozilla/js/src/jsgc.c:571
#10 0xb7f97b3b in js_NewObject (cx=0x8408278, clasp=0x812896c, proto=0x8322af0, 
    parent=0x83229f8) at ../../../mozilla/js/src/jsobj.c:1885
#11 0xb7f3a906 in JS_NewObject (cx=0x8408278, clasp=0x812896c, proto=0x8322af0, 
    parent=0x83229f8) at ../../../mozilla/js/src/jsapi.c:2258
#12 0xb7999911 in XPCWrappedNative::Init (this=0x8416ce8, ccx=@0xbfffc954, 
    parent=0x83229f8, sci=0xbfffc7f4)
    at ../../../../../mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:764
#13 0xb7997d4a in XPCWrappedNative::GetNewOrUsed (ccx=@0xbfffc954,
Object=0x8416ca8, 
    Scope=0x8413620, Interface=0x81288a0, resultWrapper=0xbfffc8e4)
    at ../../../../../mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:391
#14 0xb796d50f in nsXPCComponents::AttachNewComponentsObject (ccx=@0xbfffc954, 
    aScope=0x8413620, aGlobal=0x83229f8)
    at ../../../../../mozilla/js/src/xpconnect/src/xpccomponents.cpp:2713
#15 0xb7955f81 in nsXPConnect::InitClasses (this=0x8122830, aJSContext=0x8408278, 
    aGlobalJSObj=0x83229f8)
    at ../../../../../mozilla/js/src/xpconnect/src/nsXPConnect.cpp:443
#16 0xb795620d in nsXPConnect::InitClassesWithNewWrappedGlobal (this=0x8122830, 
    aJSContext=0x8408278, aCOMObj=0x8419ca8, aIID=@0xb5b909ec, aFlags=2, 
    _retval=0x8408128) at
../../../../../mozilla/js/src/xpconnect/src/nsXPConnect.cpp:506
#17 0xb59ae555 in nsGlobalWindow::SetNewDocument (this=0x8408050,
aDocument=0x8418088, 
    aState=0x0, aRemoveEventListeners=1, aClearScopeHint=1, aIsInternalCall=0)
    at ../../../../mozilla/dom/src/base/nsGlobalWindow.cpp:1035

Trying for XPConnect first.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I get a slightly different stack for the assertion, but it's happening because xpc_CloneJSFunction doesn't preserve the object returned by JS_CloneFunctionObject on xpcwrappednativeinfo.cpp:56 across the call to JS_SetReservedSlot at xpcwrappednativeinfo.cpp:78 :

js_NewGCThing (/builds/trunk/mozilla/js/src/jsgc.c:711)
js_NewObject (/builds/trunk/mozilla/js/src/jsobj.c:2008)
js_CloneFunctionObject (/builds/trunk/mozilla/js/src/jsfun.c:2070)
JS_CloneFunctionObject (/builds/trunk/mozilla/js/src/jsapi.c:3414)
xpc_CloneJSFunction(XPCCallContext&, JSObject*, JSObject*) (/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativeinfo.cpp:56)
DefinePropertyIfFound (/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:453)
XPC_WN_ModsAllowed_Proto_Resolve (/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1574)
js_LookupPropertyWithFlags (/builds/trunk/mozilla/js/src/jsobj.c:2801)
js_LookupProperty (/builds/trunk/mozilla/js/src/jsobj.c:2660)
js_DeleteProperty (/builds/trunk/mozilla/js/src/jsobj.c:3322)
JS_DeleteProperty2 (/builds/trunk/mozilla/js/src/jsapi.c:2775)
JS_DeleteProperty (/builds/trunk/mozilla/js/src/jsapi.c:2763)
nsGlobalWindow::SetNewDocument(nsIDocument*, nsISupports*, int, int) (/builds/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp:1287)


js_GC (/builds/trunk/mozilla/js/src/jsgc.c:1947)
js_NewGCThing (/builds/trunk/mozilla/js/src/jsgc.c:635)
AllocSlots (/builds/trunk/mozilla/js/src/jsobj.c:1925)
js_SetRequiredSlot (/builds/trunk/mozilla/js/src/jsobj.c:4388)
JS_SetReservedSlot (/builds/trunk/mozilla/js/src/jsapi.c:3348)
xpc_CloneJSFunction(XPCCallContext&, JSObject*, JSObject*) (/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativeinfo.cpp:78)
DefinePropertyIfFound (/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:453)
XPC_WN_ModsAllowed_Proto_Resolve (/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1574)
js_LookupPropertyWithFlags (/builds/trunk/mozilla/js/src/jsobj.c:2801)
js_LookupProperty (/builds/trunk/mozilla/js/src/jsobj.c:2660)
js_DeleteProperty (/builds/trunk/mozilla/js/src/jsobj.c:3322)
JS_DeleteProperty2 (/builds/trunk/mozilla/js/src/jsapi.c:2775)
JS_DeleteProperty (/builds/trunk/mozilla/js/src/jsapi.c:2763)
nsGlobalWindow::SetNewDocument(nsIDocument*, nsISupports*, int, int) (/builds/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp:1287)

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This patch fixes this crash.  The second part of the patch was just lying around in my tree, and I have no memory of adding it, but it seems like it's an analogous situation.  I haven't tested if it's necessary.  I probably should later on.

(I'm still not getting to the point of starting up.)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The second part of the patch is NOT needed to start up successfully with WAY_TOO_MUCH_GC, so it may or may not really be needed.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 212307 [details] [diff] [review]
patch

Good, but might be better to protect early to make this future-proof and not coupled (by comments and location of AUTO_MARK_JSVAL only) to details inside the JS engine.

/be

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Address brendan's comments.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Just looking at other callers of JS_SetReservedSlot, it looks like we may need similar protection in:

XPCDispInterface::Member::GetValue (I think we have GC bugs on this one!)
XPCNativeWrapperCtor
XPCNativeWrapper::GetNewOrUsed

Followup bug, I guess?

Comment 7

[Brendan Eich [:brendan]]

dbaron: do you know, or can you find, what killed the newborn root that should have kept the clone alive?

/be

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Filed comment 6 as bug 328008.

Comment 9

[Boris Zbarsky [:bzbarsky]]

So the reason the newborn root is cleared is that XPCWrappedNativeScope::FindInJSObjectScope (which we call after we call JS_CloneFunctionObject) ends up calling DEBUG_CheckForComponentsInScope, which does a property lookup, JSObject allocation when wrapping the Components object, etc, etc.  It'd be great if this debug-only check didn't mess with us like that, but I don't see how we can manage that.  :(

So maybe what we really want here is to reorder the scope lookup and cloning?  I suppose we can still do the manual marking to be sure...  

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 212511 [details] [diff] [review]
patch

sr=jst

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 212511 [details] [diff] [review]
patch

It's possible that this could also fix the #2 topcrash (js_SetClassPrototype) in 1.5.0.1.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to MOZILLA_1_8_BRANCH.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 212511 [details] [diff] [review]
patch

approved for 1.8.0 branch, a=dveditz for drivers

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_8_0_BRANCH.

Comment 16

[Bob Clary [:bc] (inactive)]

start up WAY_TOO_MUCH_GC 1.5.0.4 testing, MOZ_NO_REMOTE=1, NO_EM_RESTART=1

Linux opt/debug ok, Windows opt ok, debug crashes in a known location in js_HashString not related to the stack in this bug.

verified fixed 1.5.0.4 

Comment 17

[Bob Clary [:bc] (inactive)]

My linux build has started crashing on start now, but I can't get a good stack. Can someone else with linux please build 1.5.0.4 and check if this bug is really fixed? Thanks.