Bug 1296488 - Lift isBeingDestroyed above markRuntime; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Fri, 29 Jul 2016 13:22:20 -0700
changeset 354197 a39b3debfe378c1b35345ee2440d748f3513bd59
parent 354196 4b3793280f8a233f75b89593a753383acaef273e
child 354198 7fb14ae2f4d8d7e70bfd9880ee33c48a07efd055
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1296488
milestone51.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 1296488 - Lift isBeingDestroyed above markRuntime; r=jonco
js/src/gc/RootMarking.cpp
js/src/gc/Verifier.cpp
js/src/jsgc.cpp
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -269,22 +269,34 @@ PropertyDescriptor::trace(JSTracer* trc)
         TraceRoot(trc, &tmp, "Descriptor::set");
         setter = JS_DATA_TO_FUNC_PTR(JSSetterOp, tmp);
     }
 }
 
 void
 js::gc::GCRuntime::traceRuntimeForMajorGC(JSTracer* trc, AutoLockForExclusiveAccess& lock)
 {
+    // FinishRoots will have asserted that every root that we do not expect
+    // is gone, so we can simply skip traceRuntime here.
+    if (rt->isBeingDestroyed())
+        return;
+
     traceRuntimeCommon(trc, MarkRuntime, lock);
 }
 
 void
 js::gc::GCRuntime::traceRuntimeForMinorGC(JSTracer* trc, AutoLockForExclusiveAccess& lock)
 {
+    // Note that we *must* trace the runtime during the SHUTDOWN_GC's minor GC
+    // despite having called FinishRoots already. This is because FinishRoots
+    // does not clear the crossCompartmentWrapper map. It cannot do this
+    // because Proxy's trace for CrossCompartmentWrappers asserts presence in
+    // the map. And we can reach its trace function despite having finished the
+    // roots via the edges stored by the pre-barrier verifier when we finish
+    // the verifier for the last time.
     traceRuntimeCommon(trc, TraceRuntime, lock);
 }
 
 void
 js::TraceRuntime(JSTracer* trc)
 {
     MOZ_ASSERT(!trc->isMarkingTracer());
 
@@ -293,16 +305,17 @@ js::TraceRuntime(JSTracer* trc)
     AutoPrepareForTracing prep(rt->contextFromMainThread(), WithAtoms);
     gcstats::AutoPhase ap(rt->gc.stats, gcstats::PHASE_TRACE_HEAP);
     rt->gc.traceRuntime(trc, prep.session().lock);
 }
 
 void
 js::gc::GCRuntime::traceRuntime(JSTracer* trc, AutoLockForExclusiveAccess& lock)
 {
+    MOZ_ASSERT(!rt->isBeingDestroyed());
     traceRuntimeCommon(trc, TraceRuntime, lock);
 }
 
 void
 js::gc::GCRuntime::traceRuntimeCommon(JSTracer* trc, TraceOrMarkRuntime traceOrMark,
                                       AutoLockForExclusiveAccess& lock)
 {
     gcstats::AutoPhase ap(stats, gcstats::PHASE_MARK_ROOTS);
@@ -316,31 +329,29 @@ js::gc::GCRuntime::traceRuntimeCommon(JS
     }
 
     // Trace C stack roots.
     {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_MARK_ROOTERS);
 
         AutoGCRooter::traceAll(trc);
 
-        if (!rt->isBeingDestroyed()) {
-            MarkExactStackRoots(rt, trc);
-            rt->markSelfHostingGlobal(trc);
-        }
+        MarkExactStackRoots(rt, trc);
+        rt->markSelfHostingGlobal(trc);
 
         for (RootRange r = rootsHash.all(); !r.empty(); r.popFront()) {
             const RootEntry& entry = r.front();
             TraceRoot(trc, entry.key(), entry.value());
         }
 
         MarkPersistentRooted(rt, trc);
     }
 
     // Trace the atoms Compartment.
-    if (!rt->isBeingDestroyed() && !rt->isHeapMinorCollecting()) {
+    if (!rt->isHeapMinorCollecting()) {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_MARK_RUNTIME_DATA);
 
         if (traceOrMark == TraceRuntime || rt->atomsCompartment(lock)->zone()->isCollecting()) {
             MarkPermanentAtoms(trc);
             MarkAtoms(trc, lock);
             MarkWellKnownSymbols(trc);
             jit::JitRuntime::Mark(trc, lock);
         }
@@ -361,17 +372,17 @@ js::gc::GCRuntime::traceRuntimeCommon(JS
 
     // Trace JS stack roots.
     MarkInterpreterActivations(rt, trc);
     jit::MarkJitActivations(rt, trc);
 
     // Trace SPS.
     rt->spsProfiler.trace(trc);
 
-    // Trace the embeddings black and gray roots.
+    // Trace the embedding's black and gray roots.
     if (!rt->isHeapMinorCollecting()) {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_MARK_EMBEDDING);
 
         /*
          * The embedding can register additional roots here.
          *
          * We don't need to trace these in a minor GC because all pointers into
          * the nursery should be in the store buffer, and we want to avoid the
--- a/js/src/gc/Verifier.cpp
+++ b/js/src/gc/Verifier.cpp
@@ -504,17 +504,18 @@ CheckHeapTracer::onChild(const JS::GCCel
         oom = true;
 }
 
 bool
 CheckHeapTracer::check(AutoLockForExclusiveAccess& lock)
 {
     // The analysis thinks that traceRuntime might GC by calling a GC callback.
     JS::AutoSuppressGCAnalysis nogc;
-    rt->gc.traceRuntime(this, lock);
+    if (!rt->isBeingDestroyed())
+        rt->gc.traceRuntime(this, lock);
 
     while (!stack.empty()) {
         WorkItem item = stack.back();
         if (item.processed) {
             stack.popBack();
         } else {
             parentIndex = stack.length() - 1;
             TraceChildren(this, item.thing);
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2521,16 +2521,17 @@ GCRuntime::updateAllCellPointers(MovingT
  * Update pointers to relocated cells by doing a full heap traversal and sweep.
  *
  * The latter is necessary to update weak references which are not marked as
  * part of the traversal.
  */
 void
 GCRuntime::updatePointersToRelocatedCells(Zone* zone, AutoLockForExclusiveAccess& lock)
 {
+    MOZ_ASSERT(!rt->isBeingDestroyed());
     MOZ_ASSERT(zone->isGCCompacting());
 
     gcstats::AutoPhase ap(stats, gcstats::PHASE_COMPACT_UPDATE);
     MovingTracer trc(rt);
 
     zone->fixupAfterMovingGC();
 
     // Fixup compartment global pointers as these get accessed during marking.