Bug 803376 - Add a new form of TRANSPLANT GC, with assertions for leaks (r=luke)

--HG--
extra : rebase_source : 5cd365e2b3917ad3b649a39613ab9dae47d27320

js/src/gc/Marking.cpp

@@ -89,6 +89,7 @@ CheckMarkedThing(JSTracer *trc, T *thing)
     JS_ASSERT(thing);
     JS_ASSERT(thing->compartment());
     JS_ASSERT(thing->compartment()->rt == trc->runtime);
+    JS_ASSERT_IF(IS_GC_MARKING_TRACER(trc), !thing->compartment()->scheduledForDestruction);
     JS_ASSERT(trc->debugPrinter || trc->debugPrintArg);
 
     DebugOnly<JSRuntime *> rt = trc->runtime;
@@ -116,8 +117,10 @@ MarkInternal(JSTracer *trc, T **thingp)
      * GC.
      */
     if (!trc->callback) {
-        if (thing->compartment()->isCollecting())
+        if (thing->compartment()->isCollecting()) {
             PushMarkStack(static_cast<GCMarker *>(trc), thing);
+            thing->compartment()->maybeAlive = true;
+        }
     } else {
         trc->callback(trc, (void **)thingp, GetGCThingTraceKind(thing));
         JS_UNSET_TRACING_LOCATION(trc);

js/src/jsapi.cpp

@@ -803,6 +803,8 @@ JSRuntime::JSRuntime(JSUseHelperThreads useHelperThreads)
     gcSliceBudget(SliceBudget::Unlimited),
     gcIncrementalEnabled(true),
     gcExactScanningEnabled(true),
+    gcInTransplant(false),
+    gcObjectsMarkedInDeadCompartments(0),
     gcPoke(false),
     heapState(Idle),
 #ifdef JS_GC_ZEAL
@@ -1555,6 +1557,8 @@ JS_TransplantObject(JSContext *cx, JSObject *origobjArg, JSObject *targetArg)
     JS_ASSERT(!IsCrossCompartmentWrapper(origobj));
     JS_ASSERT(!IsCrossCompartmentWrapper(target));
 
+    AutoTransplantGC agc(cx);
+
     JSCompartment *destination = target->compartment();
     WrapperMap &map = destination->crossCompartmentWrappers;
     Value origv = ObjectValue(*origobj);
@@ -1627,6 +1631,8 @@ js_TransplantObjectWithWrapper(JSContext *cx,
     RootedObject targetobj(cx, targetobjArg);
     RootedObject targetwrapper(cx, targetwrapperArg);
 
+    AutoTransplantGC agc(cx);
+
     AssertHeapIsIdle(cx);
     JS_ASSERT(!IsCrossCompartmentWrapper(origobj));
     JS_ASSERT(!IsCrossCompartmentWrapper(origwrapper));

js/src/jscntxt.h

@@ -716,6 +716,23 @@ struct JSRuntime : js::RuntimeFriendFields
      */
     bool                gcExactScanningEnabled;
 
+
+    /*
+     * This is true if we are in the middle of a brain transplant (e.g.,
+     * JS_TransplantObject).
+     */
+    bool                gcInTransplant;
+
+    /*
+     * This field is incremented each time we mark an object inside a
+     * compartment with no incoming cross-compartment pointers. Typically if
+     * this happens it signals that an incremental GC is marking too much
+     * stuff. At various times we check this counter and, if it has changed, we
+     * run an immediate, non-incremental GC to clean up the dead
+     * compartments. This should happen very rarely.
+     */
+    unsigned            gcObjectsMarkedInDeadCompartments;
+
     bool                gcPoke;
 
     enum HeapState {

js/src/jscompartment.cpp

@@ -66,6 +66,8 @@ JSCompartment::JSCompartment(JSRuntime *rt)
     typeLifoAlloc(LIFO_ALLOC_PRIMARY_CHUNK_SIZE),
     data(NULL),
     active(false),
+    scheduledForDestruction(false),
+    maybeAlive(true),
     lastAnimationTime(0),
     regExps(rt),
     propertyTree(thisForCtor()),
@@ -307,9 +309,9 @@ JSCompartment::wrap(JSContext *cx, Value *vp, JSObject *existing)
 
     RootedObject obj(cx, &vp->toObject());
 
-    /* See if we can reuse |existing| as the wrapper for |obj|. */
     JSObject *proto = Proxy::LazyProto;
     if (existing) {
+        /* Is it possible to reuse |existing|? */
         if (!existing->getTaggedProto().isLazy() ||
             existing->getClass() != &ObjectProxyClass ||
             existing->getParent() != global ||

js/src/jscompartment.h

@@ -9,6 +9,7 @@
 #define jscompartment_h___
 
 #include "mozilla/Attributes.h"
+#include "mozilla/Util.h"
 
 #include "jscntxt.h"
 #include "jsfun.h"
@@ -279,6 +280,13 @@ struct JSCompartment
     bool                         active;  // GC flag, whether there are active frames
     js::WrapperMap               crossCompartmentWrappers;
 
+    /*
+     * These flags help us to discover if a compartment that shouldn't be alive
+     * manages to outlive a GC.
+     */
+    bool                         scheduledForDestruction;
+    bool                         maybeAlive;
+
     /* Last time at which an animation was played for a global in this compartment. */
     int64_t                      lastAnimationTime;
 

js/src/jsfriendapi.cpp

@@ -901,7 +901,12 @@ IncrementalReferenceBarrier(void *ptr)
 {
     if (!ptr)
         return;
-    JS_ASSERT(!static_cast<gc::Cell *>(ptr)->compartment()->rt->isHeapBusy());
+
+    gc::Cell *cell = static_cast<gc::Cell *>(ptr);
+    JS_ASSERT(!cell->compartment()->rt->isHeapBusy());
+
+    AutoMarkInDeadCompartment amn(cell->compartment());
+
     uint32_t kind = gc::GetGCThingTraceKind(ptr);
     if (kind == JSTRACE_OBJECT)
         JSObject::writeBarrierPre((JSObject *) ptr);

js/src/jsgc.cpp

@@ -1467,6 +1467,8 @@ ArenaLists::allocateFromArena(JSCompartment *comp, AllocKind thingKind)
     ArenaList *al = &arenaLists[thingKind];
     AutoLockGC maybeLock;
 
+    JS_ASSERT(!comp->scheduledForDestruction);
+
 #ifdef JS_THREADSAFE
     volatile uintptr_t *bfs = &backgroundFinalizeState[thingKind];
     if (*bfs != BFS_DONE) {
@@ -2117,6 +2119,15 @@ GCMarker::markBufferedGrayRoots()
     grayRoots.clearAndFree();
 }
 
+void
+GCMarker::markBufferedGrayRootCompartmentsAlive()
+{
+    for (GrayRoot *elem = grayRoots.begin(); elem != grayRoots.end(); elem++) {
+        Cell *thing = static_cast<Cell *>(elem->thing);
+        thing->compartment()->maybeAlive = true;
+    }
+}
+
 void
 GCMarker::appendGrayRoot(void *thing, JSGCTraceKind kind)
 {
@@ -3315,6 +3326,9 @@ BeginMarkPhase(JSRuntime *rt)
         }
 
         c->setPreservingCode(ShouldPreserveJITCode(c, currentTime));
+
+        c->scheduledForDestruction = false;
+        c->maybeAlive = false;
     }
 
     /* Check that at least one compartment is scheduled for collection. */
@@ -3385,6 +3399,58 @@ BeginMarkPhase(JSRuntime *rt)
         c->arenas.unmarkAll();
 
     MarkRuntime(gcmarker);
+
+    /*
+     * This code ensures that if a compartment is "dead", then it will be
+     * collected in this GC. A compartment is considered dead if its maybeAlive
+     * flag is false. The maybeAlive flag is set if:
+     *   (1) the compartment has incoming cross-compartment edges, or
+     *   (2) an object in the compartment was marked during root marking, either
+     *       as a black root or a gray root.
+     * If the maybeAlive is false, then we set the scheduledForDestruction flag.
+     * At any time later in the GC, if we try to mark an object whose
+     * compartment is scheduled for destruction, we will assert.
+     *
+     * The purpose of this check is to ensure that a compartment that we would
+     * normally destroy is not resurrected by a read barrier or an
+     * allocation. This might happen during a function like JS_TransplantObject,
+     * which iterates over all compartments, live or dead, and operates on their
+     * objects. See bug 803376 for details on this problem. To avoid the
+     * problem, we are very careful to avoid allocation and read barriers during
+     * JS_TransplantObject and the like. The code here ensures that we don't
+     * regress.
+     *
+     * Note that there are certain cases where allocations or read barriers in
+     * dead compartments are difficult to avoid. We detect such cases (via the
+     * gcObjectsMarkedInDeadCompartment counter) and redo any ongoing GCs after
+     * the JS_TransplantObject function has finished. This ensures that the dead
+     * compartments will be cleaned up. See AutoMarkInDeadCompartment and
+     * AutoTransplantGC for details.
+     */
+
+    /* Set the maybeAlive flag based on cross-compartment edges. */
+    for (CompartmentsIter c(rt); !c.done(); c.next()) {
+        for (WrapperMap::Enum e(c->crossCompartmentWrappers); !e.empty(); e.popFront()) {
+            Cell *dst = e.front().key.wrapped;
+            dst->compartment()->maybeAlive = true;
+        }
+
+        if (c->hold)
+            c->maybeAlive = true;
+    }
+
+    /* Set the maybeAlive flag based on gray roots. */
+    rt->gcMarker.markBufferedGrayRootCompartmentsAlive();
+
+    /*
+     * For black roots, code in gc/Marking.cpp will already have set maybeAlive
+     * during MarkRuntime.
+     */
+
+    for (GCCompartmentsIter c(rt); !c.done(); c.next()) {
+        if (!c->maybeAlive)
+            c->scheduledForDestruction = true;
+    }
 }
 
 void
@@ -5817,6 +5883,25 @@ PurgeJITCaches(JSCompartment *c)
 #endif
 }
 
+AutoTransplantGC::AutoTransplantGC(JSContext *cx)
+  : runtime(cx->runtime),
+    markCount(runtime->gcObjectsMarkedInDeadCompartments),
+    inIncremental(IsIncrementalGCInProgress(runtime)),
+    inTransplant(runtime->gcInTransplant)
+{
+    runtime->gcInTransplant = true;
+}
+
+AutoTransplantGC::~AutoTransplantGC()
+{
+    if (inIncremental && runtime->gcObjectsMarkedInDeadCompartments != markCount) {
+        PrepareForFullGC(runtime);
+        js::GC(runtime, GC_NORMAL, gcreason::TRANSPLANT);
+    }
+
+    runtime->gcInTransplant = inTransplant;
+}
+
 } /* namespace js */
 
 JS_PUBLIC_API(void)

js/src/jsgc.h

@@ -985,6 +985,7 @@ struct GCMarker : public JSTracer {
     void startBufferingGrayRoots();
     void endBufferingGrayRoots();
     void markBufferedGrayRoots();
+    void markBufferedGrayRootCompartmentsAlive();
 
     static void GrayCallback(JSTracer *trc, void **thing, JSGCTraceKind kind);
 
@@ -1172,6 +1173,30 @@ MaybeVerifyBarriers(JSContext *cx, bool always = false)
 void
 PurgeJITCaches(JSCompartment *c);
 
+/*
+ * This auto class should be used around any code that does brain
+ * transplants. Brain transplants can cause problems because they operate on all
+ * compartments, whether live or dead. A brain transplant can cause a formerly
+ * dead object to be "reanimated" by causing a read or write barrier to be
+ * invoked on it during the transplant.
+ *
+ * To work around this issue, we observe when mark bits are set on objects in
+ * dead compartments. If this happens during a brain transplant, we do a full,
+ * non-incremental GC at the end of the brain transplant. This will clean up any
+ * objects that were improperly marked.
+ */
+struct AutoTransplantGC
+{
+    AutoTransplantGC(JSContext *cx);
+    ~AutoTransplantGC();
+
+  private:
+    JSRuntime *runtime;
+    unsigned markCount;
+    bool inIncremental;
+    bool inTransplant;
+};
+
 } /* namespace js */
 
 #endif /* jsgc_h___ */

js/src/jsgcinlines.h

@@ -24,6 +24,32 @@ namespace js {
 
 struct Shape;
 
+/*
+ * This auto class should be used around any code that might cause a mark bit to
+ * be set on an object in a dead compartment. See AutoTransplantGC for more
+ * details.
+ */
+struct AutoMarkInDeadCompartment
+{
+    AutoMarkInDeadCompartment(JSCompartment *comp)
+      : compartment(comp),
+        scheduled(comp->scheduledForDestruction)
+    {
+        if (comp->rt->gcInTransplant && comp->scheduledForDestruction) {
+            comp->rt->gcObjectsMarkedInDeadCompartments++;
+            comp->scheduledForDestruction = false;
+        }
+    }
+
+    ~AutoMarkInDeadCompartment() {
+        compartment->scheduledForDestruction = scheduled;
+    }
+
+  private:
+    JSCompartment *compartment;
+    bool scheduled;
+};
+
 namespace gc {
 
 inline JSGCTraceKind

js/src/jsobj.cpp

@@ -2942,6 +2942,9 @@ JSObject::swap(JSContext *cx, JSObject *other_)
     RootedObject self(cx, this);
     RootedObject other(cx, other_);
 
+    AutoMarkInDeadCompartment adc1(self->compartment());
+    AutoMarkInDeadCompartment adc2(other->compartment());
+
     // Ensure swap doesn't cause a finalizer to not be run.
     JS_ASSERT(IsBackgroundFinalized(getAllocKind()) ==
               IsBackgroundFinalized(other->getAllocKind()));

js/src/jswrapper.cpp

@@ -45,6 +45,8 @@ Wrapper::New(JSContext *cx, JSObject *obj, JSObject *proto, JSObject *parent,
 {
     JS_ASSERT(parent);
 
+    AutoMarkInDeadCompartment amd(cx->compartment);
+
 #if JS_HAS_XML_SUPPORT
     if (obj->isXML()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
@@ -989,21 +991,34 @@ js::IsDeadProxyObject(RawObject obj)
     return IsProxy(obj) && GetProxyHandler(obj) == &DeadObjectProxy::singleton;
 }
 
+static void
+NukeSlot(JSObject *wrapper, uint32_t slot, Value v)
+{
+    Value old = wrapper->getSlot(slot);
+    if (old.isMarkable()) {
+        Cell *cell = static_cast<Cell *>(old.toGCThing());
+        AutoMarkInDeadCompartment amd(cell->compartment());
+        wrapper->setReservedSlot(slot, v);
+    } else {
+        wrapper->setReservedSlot(slot, v);
+    }
+}
+
 void
 js::NukeCrossCompartmentWrapper(JSContext *cx, JSObject *wrapper)
 {
     JS_ASSERT(IsCrossCompartmentWrapper(wrapper));
 
-    SetProxyPrivate(wrapper, NullValue());
+    NukeSlot(wrapper, JSSLOT_PROXY_PRIVATE, NullValue());
     SetProxyHandler(wrapper, &DeadObjectProxy::singleton);
 
     if (IsFunctionProxy(wrapper)) {
-        wrapper->setReservedSlot(JSSLOT_PROXY_CALL, NullValue());
-        wrapper->setReservedSlot(JSSLOT_PROXY_CONSTRUCT, NullValue());
+        NukeSlot(wrapper, JSSLOT_PROXY_CALL, NullValue());
+        NukeSlot(wrapper, JSSLOT_PROXY_CONSTRUCT, NullValue());
     }
 
-    wrapper->setReservedSlot(JSSLOT_PROXY_EXTRA + 0, NullValue());
-    wrapper->setReservedSlot(JSSLOT_PROXY_EXTRA + 1, NullValue());
+    NukeSlot(wrapper, JSSLOT_PROXY_EXTRA + 0, NullValue());
+    NukeSlot(wrapper, JSSLOT_PROXY_EXTRA + 1, NullValue());
 }
 
 /*
@@ -1149,6 +1164,8 @@ JS_FRIEND_API(bool)
 js::RecomputeWrappers(JSContext *cx, const CompartmentFilter &sourceFilter,
                       const CompartmentFilter &targetFilter)
 {
+    AutoTransplantGC agc(cx);
+
     AutoWrapperVector toRecompute(cx);
 
     for (CompartmentsIter c(cx->runtime); !c.done(); c.next()) {

js/src/vm/Debugger.cpp

@@ -2475,6 +2475,8 @@ Debugger::findAllGlobals(JSContext *cx, unsigned argc, Value *vp)
         return false;
 
     for (CompartmentsIter c(cx->runtime); !c.done(); c.next()) {
+        c->scheduledForDestruction = false;
+
         GlobalObject *global = c->maybeGlobal();
         if (global) {
             Value globalValue(ObjectValue(*global));