diff --git a/js/src/jsdbg.cpp b/js/src/jsdbg.cpp index 4f84d4bc4c5..46b9e07516b 100644 --- a/js/src/jsdbg.cpp +++ b/js/src/jsdbg.cpp @@ -528,16 +528,17 @@ Debug::mark(GCMarker *trc, JSCompartment *comp, JSGCInvocationKind gckind) // repeatedly until it returns false. bool markedAny = false; - // Search for Debug objects in the given compartment. We do this by - // searching all the compartments being debugged. + // If |comp| is non-null, we're collecting that compartment only; if |comp| is null, + // we're collecting all compartments. + // + // Search the debuggers list of every global in every compartment for Debug objects in + // the compartment(s) being collected. JSRuntime *rt = trc->context->runtime; for (JSCompartment **c = rt->compartments.begin(); c != rt->compartments.end(); c++) { JSCompartment *dc = *c; - // If comp is non-null, this is a per-compartment GC and we - // search every dc, except for comp (since no compartment can - // debug itself). If comp is null, this is a global GC and we - // search every dc that is live. + // If this is a single-compartment GC, no compartment can debug itself, so skip + // |comp|. If it's a global GC, then search every live compartment. if (comp ? dc != comp : !dc->isAboutToBeCollected(gckind)) { const GlobalObjectSet &debuggees = dc->getDebuggees(); for (GlobalObjectSet::Range r = debuggees.all(); !r.empty(); r.popFront()) { @@ -546,6 +547,7 @@ Debug::mark(GCMarker *trc, JSCompartment *comp, JSGCInvocationKind gckind) // Every debuggee has at least one debugger, so in this case // getDebuggers can't return NULL. const GlobalObject::DebugVector *debuggers = global->getDebuggers(); + JS_ASSERT(debuggers); for (Debug **p = debuggers->begin(); p != debuggers->end(); p++) { Debug *dbg = *p; JSObject *obj = dbg->toJSObject(); @@ -583,8 +585,8 @@ Debug::trace(JSTracer *trc) // Mark Debug.Frame objects that are reachable from JS if we look them up // again (because the corresponding StackFrame is still on the stack). - for (FrameMap::Enum e(frames); !e.empty(); e.popFront()) { - JSObject *frameobj = e.front().value; + for (FrameMap::Range r = frames.all(); !r.empty(); r.popFront()) { + JSObject *frameobj = r.front().value; JS_ASSERT(frameobj->getPrivate()); MarkObject(trc, *frameobj, "live Debug.Frame"); } diff --git a/js/src/jsdbg.h b/js/src/jsdbg.h index 12fe8c641cf..11116608331 100644 --- a/js/src/jsdbg.h +++ b/js/src/jsdbg.h @@ -82,19 +82,27 @@ class Debug { public: explicit ObjectMapMarkPolicy(JSTracer *tracer) : Base(tracer) { } - // The unwrap() call has the following effect: we mark the Debug.Object if the - // *referent* is alive, even if the CCW of the referent seems unreachable. Since - // the value always refers to the CCW, marking the value marks the CCW, so we - // needn't worry that the CCW will go dead. - bool keyMarked(JSObject *k) { return k->unwrap()->isMarked(); } + // The unwrap() call here means that we use the *referent's* mark, not that of the + // CCW itself, to decide whether the table entry is live. This seems weird: if the + // CCW is not marked, and the referent is, won't we end up keeping the table entry + // but GC'ing its key? But it's okay: the Debug.Object always refers to the CCW, + // so marking the value marks the CCW. + bool keyMarked(JSObject *k) { + JS_ASSERT(k->isCrossCompartmentWrapper()); + return k->unwrap()->isMarked(); + } void markKey(JSObject *k, const char *description) { js::gc::MarkObject(tracer, *k->unwrap(), description); } }; - // Keys are referents, values are Debug.Object objects. The combination of - // the a key being live and this Debug being live keeps the corresponding - // Debug.Object alive. + // The map from debuggee objects to their Debug.Object instances. However, to avoid + // holding cross-compartment references directly, the keys in this map are the + // referents' CCWs, not the referents themselves. Thus, to find the Debug.Object for a + // debuggee object, you must first find its CCW, and then look that up here. + // + // Using CCWs for keys when it's really their referents' liveness that determines the + // table entry's liveness is delicate; see comments on ObjectMapMarkPolicy. typedef WeakMap, ObjectMapMarkPolicy> ObjectWeakMap; ObjectWeakMap objects;