476050 - TM: js_GC tests for JS_ON_TRACE appear insufficient

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

I haven't tried to get this to crash, but I see 2 possible bugs in this code:

void
js_GC(JSContext *cx, JSGCInvocationKind gckind)
{
    ...
    if (JS_ON_TRACE(cx))
        goto out;
    ...
restart:
    ...
    ... (actually do GC here)
    ...
out:
    ...
    if (rt->gcLevel > 1 || rt->gcPoke) {
        rt->gcLevel = 1;
        rt->gcPoke = JS_FALSE;
        JS_UNLOCK_GC(rt);
        goto restart;
    }
    ...
}

1.  There's a path that puts us at `restart` even though we're on trace.[*]

2.  If we're on trace in some context other context than cx, and that context is on the current thread, we could GC.  (I don't know how applications use multiple contexts on a thread in practice, so I really don't know how significant this is.)

[*] You can thank treehydra for that observation.

Comment 1

[Jason Orendorff [:jorendorff]]

Part 1 is fixed in rev 932126be5356 (bug 476177).

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Obsolete with the removal of tracejit.