Closed Bug 466182 Opened 16 years ago Closed 14 years ago

Crash in GC at DestroyScriptsToGC during JS_SetPrototype()

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: MikeM, Unassigned)

References

Details

(Keywords: crash)

I'm getting a crash in a call to DestroyScriptsToGC() from js_GC.cpp on line 3391.
The is occurring with multiple threads & JS_THREADSAFE.

The relavent code is below:
-----------------
while ((acx = js_ContextIterator(rt, JS_FALSE, &iter)) != NULL) {
        if (!acx->thread || acx->thread == cx->thread)
            continue;
        GSN_CACHE_CLEAR(&acx->thread->gsnCache);
        js_FlushPropertyCache(acx);
#ifdef JS_TRACER
        js_FlushJITOracle(acx);
#endif
        DestroyScriptsToGC(cx, &acx->thread->scriptsToGC);
    }
------------------

The crash seems to occur because acx->thread goes null during iteration over the contexts.  That causes DestroyScriptToGC() to die a hard death.
I suspect another thread is calling JS_ClearContextThread() on the context during this loop.

Here's top of the stack during the crash:
---------------------------------
DestroyScriptsToGC(JSContext * cx=0x07c8ae38, JSScript * * listp=0x00010064)  Line 3123 + 0x6 bytes	C++

js_GC(JSContext * cx=0x07c8ae38, JSGCInvocationKind gckind=GC_LOCK_HELD)  Line 3391 + 0x19 bytes	C++

js_SetProtoOrParent(JSContext * cx=0x07c8ae38, JSObject * obj=0x08aa25a0, unsigned long slot=0, JSObject * pobj=0x07f60440)  Line 306 + 0xb bytes	C++

JS_SetPrototype(JSContext * cx=0x07c8ae38, JSObject * obj=0x08aa25a0, JSObject * proto=0x07f60440)  Line 2946 + 0x1b bytes	C++
--------------------------

The strangest thing to me is why exactly is JS_SetPrototype() conditionally (in DEBUG mode only) calling js_SetProtoOrParent() which is doing garbage collection. See line 2946 in jsapi.cpp for the details...

Here's the comment from just before the call(which I don't really understand):
    /*
     * FIXME: bug 408416. The cycle-detection required for script-writeable
     * __proto__ lives in js_SetProtoOrParent over in jsobj.c, also known as
     * js_ObjectOps.setProto. This hook must detect cycles, to prevent scripts
     * from ilooping SpiderMonkey trivially. But the overhead of detecting
     * cycles is high enough, and the threat from JS-API-calling C++ code is
     * low enough, that it's not worth burdening the non-DEBUG callers. Same
     * goes for JS_SetParent, below.
     */

Having JS_SetPrototype() doing GC is quite unexpected (at least to me) and probably not very efficient.

Please shed some like on this "DEBUG" only call as well as how to fix the crash.
Thanks!
Mike M.
Depends on: 476934
Depends on: 477021
Mike, still see this now that bug 476934 and bug 477021 are fixed?
Severity: major → critical
Keywords: crash
Nope I believe those patches this bug...
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.