Bug 1343261 - dead object proxies must be swept with their former targets, r=jonco
authorSteve Fink <sfink@mozilla.com>
Mon, 06 Mar 2017 12:27:43 -0800
changeset 346338 b0e08cf37f290837bede065043b0f0523c907494
parent 346337 ba842dcad0103677568fc81ba92c8f5b3c1e0c5f
child 346340 fc50ad3760aacd2475a834bba04ff1d37a6cf62e
push id87772
push usersfink@mozilla.com
push dateTue, 07 Mar 2017 16:34:31 +0000
treeherdermozilla-inbound@b0e08cf37f29 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1343261
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1343261 - dead object proxies must be swept with their former targets, r=jonco MozReview-Commit-ID: KM6gNtGWvws
js/src/gc/GCRuntime.h
js/src/gc/Zone.cpp
js/src/gc/Zone.h
js/src/jit-test/tests/cacheir/nukedCCW.js
js/src/jscompartment.h
js/src/jsgc.cpp
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -974,17 +974,17 @@ class GCRuntime
     template <class ZoneIterT, class CompartmentIterT> void markGrayReferences(gcstats::Phase phase);
     void markBufferedGrayRoots(JS::Zone* zone);
     void markGrayReferencesInCurrentGroup(gcstats::Phase phase);
     void markAllWeakReferences(gcstats::Phase phase);
     void markAllGrayReferences(gcstats::Phase phase);
 
     void beginSweepPhase(bool lastGC, AutoLockForExclusiveAccess& lock);
     void findZoneGroups(AutoLockForExclusiveAccess& lock);
-    MOZ_MUST_USE bool findZoneEdgesForWeakMaps();
+    MOZ_MUST_USE bool findInterZoneEdges();
     void getNextZoneGroup();
     void endMarkingZoneGroup();
     void beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock);
     bool shouldReleaseObservedTypes();
     void endSweepingZoneGroup();
     IncrementalProgress sweepPhase(SliceBudget& sliceBudget, AutoLockForExclusiveAccess& lock);
     void endSweepPhase(bool lastGC, AutoLockForExclusiveAccess& lock);
     bool allCCVisibleZonesWereCollected() const;
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -33,16 +33,17 @@ JS::Zone::Zone(JSRuntime* rt, ZoneGroup*
     types(this),
     gcWeakMapList_(group),
     compartments_(),
     gcGrayRoots_(group),
     gcWeakRefs_(group),
     weakCaches_(group),
     gcWeakKeys_(group, SystemAllocPolicy(), rt->randomHashCodeScrambler()),
     gcZoneGroupEdges_(group),
+    hasDeadProxies_(group),
     typeDescrObjects_(group, this, SystemAllocPolicy()),
     markedAtoms_(group),
     usage(&rt->gc.usage),
     threshold(),
     gcDelayBytes(0),
     propertyTree_(group, this),
     baseShapes_(group, this, BaseShapeSet()),
     initialShapes_(group, this, InitialShapeSet()),
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -366,19 +366,27 @@ struct Zone : public JS::shadow::Zone,
     js::gc::WeakKeyTable& gcWeakKeys() { return gcWeakKeys_.ref(); }
 
   private:
     // A set of edges from this zone to other zones.
     //
     // This is used during GC while calculating zone groups to record edges that
     // can't be determined by examining this zone by itself.
     js::ZoneGroupData<ZoneSet> gcZoneGroupEdges_;
+
+    // Zones with dead proxies require an extra scan through the wrapper map,
+    // so track whether any dead proxies are known to exist.
+    js::ZoneGroupData<bool> hasDeadProxies_;
+
   public:
     ZoneSet& gcZoneGroupEdges() { return gcZoneGroupEdges_.ref(); }
 
+    bool hasDeadProxies() { return hasDeadProxies_; }
+    void setHasDeadProxies(bool b) { hasDeadProxies_ = b; }
+
     // Keep track of all TypeDescr and related objects in this compartment.
     // This is used by the GC to trace them all first when compacting, since the
     // TypedObject trace hook may access these objects.
     //
     // There are no barriers here - the set contains only tenured objects so no
     // post-barrier is required, and these are weak references so no pre-barrier
     // is required.
     using TypeDescrObjectSet = js::GCHashSet<JSObject*,
--- a/js/src/jit-test/tests/cacheir/nukedCCW.js
+++ b/js/src/jit-test/tests/cacheir/nukedCCW.js
@@ -1,11 +1,11 @@
-var wrapper = evaluate("({a: 15, b: {c: 42}})", {global: newGlobal({sameZoneAs: this})});
+function testNuke() {
+    var wrapper = evaluate("({a: 15, b: {c: 42}})", {global: newGlobal({sameZoneAs: this})});
 
-function test() {
     var i, error;
     try {
         for (i = 0; i < 150; i++) {
             assertEq(wrapper.b.c, 42);
             assertEq(wrapper.a, 15);
 
             if (i == 142) {
                 // Next access to wrapper.b should throw.
@@ -15,9 +15,24 @@ function test() {
     } catch (e) {
         error = e;
     }
 
     assertEq(error.message.includes("dead object"), true);
     assertEq(i, 143);
 }
 
-test();
+function testSweep() {
+    var wrapper = evaluate("({a: 15, b: {c: 42}})", {global: newGlobal({})});
+    var error;
+    nukeCCW(wrapper);
+    gczeal(8, 1); // Sweep zones separately
+    try {
+      // Next access to wrapper.b should throw.
+      wrapper.x = 4;
+    } catch (e) {
+        error = e;
+    }
+    assertEq(error.message.includes("dead object"), true);
+}
+
+testNuke();
+testSweep();
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -740,16 +740,18 @@ struct JSCompartment
 
     // Whether the given name is in [[VarNames]].
     bool isInVarNames(JS::Handle<JSAtom*> name) {
         return varNames_.has(name);
     }
 
     void findOutgoingEdges(js::gc::ZoneComponentFinder& finder);
 
+    MOZ_MUST_USE bool findDeadProxyZoneEdges(bool* foundAny);
+
     js::DtoaCache dtoaCache;
     js::NewProxyCache newProxyCache;
 
     // Random number generator for Math.random().
     mozilla::Maybe<mozilla::non_crypto::XorShift128PlusRNG> randomNumberGenerator;
 
     // Initialize randomNumberGenerator if needed.
     void ensureRandomNumberGenerator();
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3177,17 +3177,17 @@ GCRuntime::sweepBackgroundThings(ZoneLis
                 if (arenas)
                     ArenaLists::backgroundFinalize(&fop, arenas, &emptyArenas);
             }
         }
     }
 
     AutoLockGC lock(rt);
 
-    // Release swept areans, dropping and reaquiring the lock every so often to
+    // Release swept arenas, dropping and reaquiring the lock every so often to
     // avoid blocking the active thread from allocating chunks.
     static const size_t LockReleasePeriod = 32;
     size_t releaseCount = 0;
     Arena* next;
     for (Arena* arena = emptyArenas; arena; arena = next) {
         next = arena->next;
         rt->gc.releaseArena(arena, lock);
         releaseCount++;
@@ -4395,18 +4395,18 @@ DropStringWrappers(JSRuntime* rt)
     }
 }
 
 /*
  * Group zones that must be swept at the same time.
  *
  * If compartment A has an edge to an unmarked object in compartment B, then we
  * must not sweep A in a later slice than we sweep B. That's because a write
- * barrier in A that could lead to the unmarked object in B becoming
- * marked. However, if we had already swept that object, we would be in trouble.
+ * barrier in A could lead to the unmarked object in B becoming marked.
+ * However, if we had already swept that object, we would be in trouble.
  *
  * If we consider these dependencies as a graph, then all the compartments in
  * any strongly-connected component of this graph must be swept in the same
  * slice.
  *
  * Tarjan's algorithm is used to calculate the components.
  */
 namespace {
@@ -4446,16 +4446,38 @@ JSCompartment::findOutgoingEdges(ZoneCom
         if (key.is<JSObject*>()) {
             TenuredCell& other = key.as<JSObject*>()->asTenured();
             needsEdge = !other.isMarked(BLACK) || other.isMarked(GRAY);
         }
         key.applyToWrapped(AddOutgoingEdgeFunctor(needsEdge, finder));
     }
 }
 
+bool
+JSCompartment::findDeadProxyZoneEdges(bool* foundAny)
+{
+    // As an optimization, return whether any dead proxy objects are found in
+    // this compartment so that if a zone has none, its cross compartment
+    // wrappers do not need to be scanned.
+    *foundAny = false;
+    for (js::WrapperMap::Enum e(crossCompartmentWrappers); !e.empty(); e.popFront()) {
+        Value value = e.front().value().get();
+        if (value.isObject()) {
+            if (IsDeadProxyObject(&value.toObject())) {
+                *foundAny = true;
+                CrossCompartmentKey& key = e.front().mutableKey();
+                if (!key.as<JSObject*>()->zone()->gcZoneGroupEdges().put(zone()))
+                    return false;
+            }
+        }
+    }
+
+    return true;
+}
+
 void
 Zone::findOutgoingEdges(ZoneComponentFinder& finder)
 {
     /*
      * Any compartment may have a pointer to an atom in the atoms
      * compartment, and these aren't in the cross compartment map.
      */
     JSRuntime* rt = runtimeFromActiveCooperatingThread();
@@ -4470,47 +4492,61 @@ Zone::findOutgoingEdges(ZoneComponentFin
         if (r.front()->isGCMarking())
             finder.addEdgeTo(r.front());
     }
 
     Debugger::findZoneEdges(this, finder);
 }
 
 bool
-GCRuntime::findZoneEdgesForWeakMaps()
+GCRuntime::findInterZoneEdges()
 {
     /*
      * Weakmaps which have keys with delegates in a different zone introduce the
      * need for zone edges from the delegate's zone to the weakmap zone.
      *
      * Since the edges point into and not away from the zone the weakmap is in
      * we must find these edges in advance and store them in a set on the Zone.
      * If we run out of memory, we fall back to sweeping everything in one
      * group.
      */
 
     for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
         if (!WeakMapBase::findInterZoneEdges(zone))
             return false;
     }
 
+    for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
+        if (zone->hasDeadProxies()) {
+            bool foundInZone = false;
+            for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) {
+                bool foundInCompartment = false;
+                if (!comp->findDeadProxyZoneEdges(&foundInCompartment))
+                    return false;
+                foundInZone = foundInZone || foundInCompartment;
+            }
+            if (!foundInZone)
+                zone->setHasDeadProxies(false);
+        }
+    }
+
     return true;
 }
 
 void
 GCRuntime::findZoneGroups(AutoLockForExclusiveAccess& lock)
 {
 #ifdef DEBUG
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next())
         MOZ_ASSERT(zone->gcZoneGroupEdges().empty());
 #endif
 
     JSContext* cx = TlsContext.get();
     ZoneComponentFinder finder(cx->nativeStackLimit[JS::StackForSystemCode], lock);
-    if (!isIncremental || !findZoneEdgesForWeakMaps())
+    if (!isIncremental || !findInterZoneEdges())
         finder.useOneComponent();
 
     for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
         MOZ_ASSERT(zone->isGCMarking());
         finder.addNode(zone);
     }
     zoneGroups = finder.getResultsList();
     currentZoneGroup = zoneGroups;
@@ -4775,16 +4811,18 @@ ResetGrayList(JSCompartment* comp)
 void
 js::NotifyGCNukeWrapper(JSObject* obj)
 {
     /*
      * References to target of wrapper are being removed, we no longer have to
      * remember to mark it.
      */
     RemoveFromGrayList(obj);
+
+    obj->zone()->setHasDeadProxies(true);
 }
 
 enum {
     JS_GC_SWAP_OBJECT_A_REMOVED = 1 << 0,
     JS_GC_SWAP_OBJECT_B_REMOVED = 1 << 1
 };
 
 unsigned