Bug 1211022 - Add a type specialization for weak C++ GC thing references; r=sfink

--HG--
extra : rebase_source : 12192c6bb506c7e97308260131b0a00ebb13a6e6

js/src/gc/Barrier.h

@@ -595,6 +595,11 @@ class ReadBarriered : public ReadBarrieredBase<T>
     }
 };
 
+// A WeakRef pointer does not hold its target live and is automatically nulled
+// out when the GC discovers that it is not reachable from any other path.
+template <typename T>
+using WeakRef = ReadBarriered<T>;
+
 // Add Value operations to all Barrier types. Note, this must be defined before
 // HeapSlot for HeapSlot's base to get these operations.
 template <>

js/src/gc/Marking.cpp

@@ -387,6 +387,8 @@ template <typename T> void DispatchToTracer(JSTracer* trc, T* thingp, const char
 template <typename T> T DoCallback(JS::CallbackTracer* trc, T* thingp, const char* name);
 template <typename T> void DoMarking(GCMarker* gcmarker, T* thing);
 template <typename T> void DoMarking(GCMarker* gcmarker, T thing);
+template <typename T> void NoteWeakEdge(GCMarker* gcmarker, T** thingp);
+template <typename T> void NoteWeakEdge(GCMarker* gcmarker, T* thingp);
 
 template <typename T>
 void
@@ -402,6 +404,18 @@ js::TraceManuallyBarrieredEdge(JSTracer* trc, T* thingp, const char* name)
     DispatchToTracer(trc, ConvertToBase(thingp), name);
 }
 
+template <typename T>
+void
+js::TraceWeakEdge(JSTracer* trc, WeakRef<T>* thingp, const char* name)
+{
+    // Non-marking tracers treat the edge strongly.
+    if (!trc->isMarkingTracer())
+        DispatchToTracer(trc, ConvertToBase(thingp->unsafeUnbarrieredForTracing()), name);
+
+    NoteWeakEdge(static_cast<GCMarker*>(trc),
+                 ConvertToBase(thingp->unsafeUnbarrieredForTracing()));
+}
+
 template <typename T>
 void
 js::TraceRoot(JSTracer* trc, T* thingp, const char* name)
@@ -448,6 +462,7 @@ js::TraceRootRange(JSTracer* trc, size_t len, T* vec, const char* name)
 #define INSTANTIATE_ALL_VALID_TRACE_FUNCTIONS(type) \
     template void js::TraceEdge<type>(JSTracer*, WriteBarrieredBase<type>*, const char*); \
     template void js::TraceManuallyBarrieredEdge<type>(JSTracer*, type*, const char*); \
+    template void js::TraceWeakEdge<type>(JSTracer*, WeakRef<type>*, const char*); \
     template void js::TraceRoot<type>(JSTracer*, type*, const char*); \
     template void js::TraceNullableRoot<type>(JSTracer*, type*, const char*); \
     template void js::TraceRange<type>(JSTracer*, size_t, WriteBarrieredBase<type>*, const char*); \
@@ -705,6 +720,47 @@ DoMarking(GCMarker* gcmarker, T thing)
     DispatchTyped(DoMarkingFunctor<T>(), thing, gcmarker);
 }
 
+template <typename T>
+void
+NoteWeakEdge(GCMarker* gcmarker, T** thingp)
+{
+    // Do per-type marking precondition checks.
+    if (MustSkipMarking(*thingp))
+        return;
+
+    CheckTracedThing(gcmarker, *thingp);
+
+    // If the target is already marked, there's no need to store the edge.
+    if (IsMarkedUnbarriered(thingp))
+        return;
+
+    gcmarker->noteWeakEdge(thingp);
+}
+
+template <typename T>
+void
+NoteWeakEdge(GCMarker* gcmarker, T* thingp)
+{
+    MOZ_CRASH("the gc does not support tagged pointers as weak edges");
+}
+
+template <typename T>
+void
+js::GCMarker::noteWeakEdge(T* edge)
+{
+    static_assert(IsBaseOf<Cell, typename mozilla::RemovePointer<T>::Type>::value,
+                  "edge must point to a GC pointer");
+    MOZ_ASSERT((*edge)->isTenured());
+
+    // Note: we really want the *source* Zone here. The edge may start in a
+    // non-gc heap location, however, so we use the fact that cross-zone weak
+    // references are not allowed and use the *target's* zone.
+    JS::Zone::WeakEdges &weakRefs = (*edge)->asTenured().zone()->gcWeakRefs;
+    AutoEnterOOMUnsafeRegion oomUnsafe;
+    if (!weakRefs.append(reinterpret_cast<TenuredCell**>(edge)))
+        oomUnsafe.crash("Failed to record a weak edge for sweeping.");
+}
+
 // The simplest traversal calls out to the fully generic traceChildren function
 // to visit the child edges. In the absence of other traversal mechanisms, this
 // function will rapidly grow the stack past its bounds and crash the process.
@@ -2260,6 +2316,17 @@ IsMarkedInternal(T* thingp)
     return rv;
 }
 
+bool
+js::gc::IsAboutToBeFinalizedDuringSweep(TenuredCell& tenured)
+{
+    MOZ_ASSERT(!IsInsideNursery(&tenured));
+    MOZ_ASSERT(!tenured.runtimeFromAnyThread()->isHeapMinorCollecting());
+    MOZ_ASSERT(tenured.zoneFromAnyThread()->isGCSweeping());
+    if (tenured.arenaHeader()->allocatedDuringIncremental)
+        return false;
+    return !tenured.isMarked();
+}
+
 template <typename T>
 static bool
 IsAboutToBeFinalizedInternal(T** thingp)
@@ -2282,11 +2349,8 @@ IsAboutToBeFinalizedInternal(T** thingp)
 
     Zone* zone = thing->asTenured().zoneFromAnyThread();
     if (zone->isGCSweeping()) {
-        if (thing->asTenured().arenaHeader()->allocatedDuringIncremental)
-            return false;
-        return !thing->asTenured().isMarked();
-    }
-    else if (zone->isGCCompacting() && IsForwarded(thing)) {
+        return IsAboutToBeFinalizedDuringSweep(thing->asTenured());
+    } else if (zone->isGCCompacting() && IsForwarded(thing)) {
         *thingp = Forwarded(thing);
         return false;
     }
@@ -2328,13 +2392,6 @@ IsMarked(WriteBarrieredBase<T>* thingp)
     return IsMarkedInternal(ConvertToBase(thingp->unsafeUnbarrieredForTracing()));
 }
 
-template <typename T>
-bool
-IsMarked(ReadBarrieredBase<T>* thingp)
-{
-    return IsMarkedInternal(ConvertToBase(thingp->unsafeUnbarrieredForTracing()));
-}
-
 template <typename T>
 bool
 IsAboutToBeFinalizedUnbarriered(T* thingp)
@@ -2360,7 +2417,6 @@ IsAboutToBeFinalized(ReadBarrieredBase<T>* thingp)
 #define INSTANTIATE_ALL_VALID_TRACE_FUNCTIONS(type) \
     template bool IsMarkedUnbarriered<type>(type*); \
     template bool IsMarked<type>(WriteBarrieredBase<type>*); \
-    template bool IsMarked<type>(ReadBarrieredBase<type>*); \
     template bool IsAboutToBeFinalizedUnbarriered<type>(type*); \
     template bool IsAboutToBeFinalized<type>(WriteBarrieredBase<type>*); \
     template bool IsAboutToBeFinalized<type>(ReadBarrieredBase<type>*);

js/src/gc/Marking.h

@@ -186,6 +186,9 @@ class GCMarker : public JSTracer
     template <typename S, typename T> void traverseEdge(S source, T* target);
     template <typename S, typename T> void traverseEdge(S source, T target);
 
+    // Notes a weak graph edge for later sweeping.
+    template <typename T> void noteWeakEdge(T* edge);
+
     /*
      * Care must be taken changing the mark color from gray to black. The cycle
      * collector depends on the invariant that there are no black to gray edges
@@ -390,10 +393,6 @@ template <typename T>
 bool
 IsMarked(WriteBarrieredBase<T>* thingp);
 
-template <typename T>
-bool
-IsMarked(ReadBarrieredBase<T>* thingp);
-
 template <typename T>
 bool
 IsAboutToBeFinalizedUnbarriered(T* thingp);
@@ -406,6 +405,9 @@ template <typename T>
 bool
 IsAboutToBeFinalized(ReadBarrieredBase<T>* thingp);
 
+bool
+IsAboutToBeFinalizedDuringSweep(TenuredCell& tenured);
+
 inline Cell*
 ToMarkable(const Value& v)
 {

js/src/gc/Tracer.h

@@ -76,6 +76,13 @@ template <typename T>
 void
 TraceManuallyBarrieredEdge(JSTracer* trc, T* thingp, const char* name);
 
+// Visits a WeakRef, but does not trace its referents. If *thingp is not marked
+// at the end of marking, it is replaced by nullptr. This method records
+// thingp, so the edge location must not change after this function is called.
+template <typename T>
+void
+TraceWeakEdge(JSTracer* trc, WeakRef<T>* thingp, const char* name);
+
 // Trace all edges contained in the given array.
 template <typename T>
 void

js/src/gc/Zone.h

@@ -288,10 +288,15 @@ struct Zone : public JS::shadow::Zone,
     typedef js::Vector<JSCompartment*, 1, js::SystemAllocPolicy> CompartmentVector;
     CompartmentVector compartments;
 
-    // This compartment's gray roots.
+    // This zone's gray roots.
     typedef js::Vector<js::gc::Cell*, 0, js::SystemAllocPolicy> GrayRootVector;
     GrayRootVector gcGrayRoots;
 
+    // This zone's weak edges found via graph traversal during marking,
+    // preserved for re-scanning during sweeping.
+    using WeakEdges = js::Vector<js::gc::TenuredCell**, 0, js::SystemAllocPolicy>;
+    WeakEdges gcWeakRefs;
+
     // A set of edges from this zone to other zones.
     //
     // This is used during GC while calculating zone groups to record edges that

js/src/jsapi-tests/moz.build

@@ -44,6 +44,7 @@ UNIFIED_SOURCES += [
     'testGCOutOfMemory.cpp',
     'testGCStoreBufferRemoval.cpp',
     'testGCUniqueId.cpp',
+    'testGCWeakRef.cpp',
     'testGetPropertyDescriptor.cpp',
     'testHashTable.cpp',
     'testIndexToString.cpp',

js/src/jsapi-tests/testGCWeakRef.cpp

@@ -0,0 +1,63 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+* vim: set ts=8 sts=4 et sw=4 tw=99:
+*/
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "gc/Barrier.h"
+#include "js/RootingAPI.h"
+
+struct MyHeap : JS::Traceable
+{
+    explicit MyHeap(JSObject* obj) : weak(obj) {}
+    js::WeakRef<JSObject*> weak;
+
+    static void trace(MyHeap* self, JSTracer* trc) {
+        js::TraceWeakEdge(trc, &self->weak, "weak");
+    }
+};
+
+BEGIN_TEST(testGCWeakRef)
+{
+    // Create an object and add a property to it so that we can read the
+    // property back later to verify that object internals are not garbage.
+    JS::RootedObject obj(cx, JS_NewPlainObject(cx));
+    CHECK(obj);
+    CHECK(JS_DefineProperty(cx, obj, "x", 42, 0));
+
+    // Store the object behind a weak pointer and remove other references.
+    Rooted<MyHeap> heap(cx, MyHeap(obj));
+    obj = nullptr;
+
+    rt->gc.minorGC(JS::gcreason::API);
+
+    // The minor collection should have treated the weak ref as a strong ref,
+    // so the object should still be live, despite not having any other live
+    // references.
+    CHECK(heap.get().weak.unbarrieredGet() != nullptr);
+    obj = heap.get().weak;
+    RootedValue v(cx);
+    CHECK(JS_GetProperty(cx, obj, "x", &v));
+    CHECK(v.isInt32());
+    CHECK(v.toInt32() == 42);
+
+    // A full collection with a second ref should keep the object as well.
+    CHECK(obj == heap.get().weak);
+    JS_GC(rt);
+    CHECK(obj == heap.get().weak);
+    v = UndefinedValue();
+    CHECK(JS_GetProperty(cx, obj, "x", &v));
+    CHECK(v.isInt32());
+    CHECK(v.toInt32() == 42);
+
+    // A full collection after nulling the root should collect the object, or
+    // at least null out the weak reference before returning to the mutator.
+    obj = nullptr;
+    JS_GC(rt);
+    CHECK(heap.get().weak == nullptr);
+
+    return true;
+}
+END_TEST(testGCWeakRef)
+

js/src/jsgc.cpp

@@ -4998,6 +4998,16 @@ GCRuntime::beginSweepingZoneGroup()
         }
     }
 
+    /* Clear all weakrefs that point to unmarked things. */
+    for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) {
+        for (auto edge : zone->gcWeakRefs) {
+            /* Edges may be present multiple times, so may already be nulled. */
+            if (*edge && IsAboutToBeFinalizedDuringSweep(**edge))
+                *edge = nullptr;
+        }
+        zone->gcWeakRefs.clear();
+    }
+
     FreeOp fop(rt);
     SweepAtomsTask sweepAtomsTask(rt);
     SweepInnerViewsTask sweepInnerViewsTask(rt);