Bug 1290551 - Part 2: Assert that finishRoots actually unroots everything; r=jonco

--HG--
extra : rebase_source : 322bbaf46bb1dc1b14bef0a939a07702f478c01c

js/src/gc/Nursery.cpp

@@ -785,6 +785,10 @@ js::Nursery::freeMallocedBuffers()
 void
 js::Nursery::waitBackgroundFreeEnd()
 {
+    // We may finishRoots before nursery init if runtime init fails.
+    if (!isEnabled())
+        return;
+
     MOZ_ASSERT(freeMallocedBuffersTask);
     freeMallocedBuffersTask->join();
 }

js/src/gc/RootMarking.cpp

@@ -223,7 +223,8 @@ AutoGCRooter::trace(JSTracer* trc)
 /* static */ void
 AutoGCRooter::traceAll(JSTracer* trc)
 {
-    traceAllInContext(trc->runtime()->contextFromMainThread(), trc);
+    for (AutoGCRooter* gcr = trc->runtime()->contextFromMainThread()->roots.autoGCRooters_; gcr; gcr = gcr->down)
+        gcr->trace(trc);
 }
 
 /* static */ void
@@ -389,13 +390,50 @@ js::gc::GCRuntime::traceRuntimeCommon(JSTracer* trc, TraceOrMarkRuntime traceOrM
     }
 }
 
+#ifdef DEBUG
+class AssertNoRootsTracer : public JS::CallbackTracer
+{
+    void onChild(const JS::GCCellPtr& thing) override {
+        MOZ_CRASH("There should not be any roots after finishRoots");
+    }
+
+  public:
+    AssertNoRootsTracer(JSRuntime* rt, WeakMapTraceKind weakTraceKind)
+      : JS::CallbackTracer(rt, weakTraceKind)
+    {}
+};
+#endif // DEBUG
+
 void
 js::gc::GCRuntime::finishRoots()
 {
+    rt->finishAtoms();
+
     if (rootsHash.initialized())
         rootsHash.clear();
 
-    rt->mainThread.roots.finishPersistentRoots();
+    rt->contextFromMainThread()->roots.finishPersistentRoots();
+
+    rt->finishSelfHosting();
+
+    for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next())
+        c->finishRoots();
+
+#ifdef DEBUG
+    // The nsWrapperCache may not be empty before our shutdown GC, so we have
+    // to skip that table when verifying that we are fully unrooted.
+    auto prior = grayRootTracer;
+    grayRootTracer = Callback<JSTraceDataOp>(nullptr, nullptr);
+
+    AssertNoRootsTracer trc(rt, TraceWeakMapKeysValues);
+    AutoPrepareForTracing prep(rt->contextFromMainThread(), WithAtoms);
+    gcstats::AutoPhase ap(rt->gc.stats, gcstats::PHASE_TRACE_HEAP);
+    traceRuntime(&trc, prep.session().lock);
+
+    // Restore the wrapper tracing so that we leak instead of leaving dangling
+    // pointers.
+    grayRootTracer = prior;
+#endif // DEBUG
 }
 
 // Append traced things to a buffer on the zone for use later in the GC.

js/src/jit/Ion.cpp

@@ -613,6 +613,12 @@ jit::LazyLinkTopActivation(JSContext* cx)
 JitRuntime::Mark(JSTracer* trc, AutoLockForExclusiveAccess& lock)
 {
     MOZ_ASSERT(!trc->runtime()->isHeapMinorCollecting());
+
+    // Shared stubs are allocated in the atoms compartment, so do not iterate
+    // them after the atoms heap after it has been "finished."
+    if (trc->runtime()->atomsAreFinished())
+        return;
+
     Zone* zone = trc->runtime()->atomsCompartment(lock)->zone();
     for (auto i = zone->cellIter<JitCode>(); !i.done(); i.next()) {
         JitCode* code = i;

js/src/jsatom.cpp

@@ -200,6 +200,10 @@ void
 js::MarkAtoms(JSTracer* trc, AutoLockForExclusiveAccess& lock)
 {
     JSRuntime* rt = trc->runtime();
+
+    if (rt->atomsAreFinished())
+        return;
+
     for (AtomSet::Enum e(rt->atoms(lock)); !e.empty(); e.popFront()) {
         const AtomStateEntry& entry = e.front();
         if (!entry.isPinned())

js/src/jscompartment.cpp

@@ -690,6 +690,27 @@ JSCompartment::traceRoots(JSTracer* trc, js::gc::GCRuntime::TraceOrMarkRuntime t
     wasm.trace(trc);
 }
 
+void
+JSCompartment::finishRoots()
+{
+    if (watchpointMap)
+        watchpointMap->clear();
+
+    if (debugScopes)
+        debugScopes->finish();
+
+    if (lazyArrayBuffers)
+        lazyArrayBuffers->clear();
+
+    if (objectMetadataTable)
+        objectMetadataTable->clear();
+
+    clearScriptCounts();
+
+    if (nonSyntacticLexicalScopes_)
+        nonSyntacticLexicalScopes_->clear();
+}
+
 void
 JSCompartment::sweepAfterMinorGC()
 {

js/src/jscompartment.h

@@ -604,6 +604,10 @@ struct JSCompartment
      * regardless of whether the JSCompartment itself is still live.
      */
     void traceRoots(JSTracer* trc, js::gc::GCRuntime::TraceOrMarkRuntime traceOrMark);
+    /*
+     * This method clears out tables of roots in preparation for the final GC.
+     */
+    void finishRoots();
     /*
      * These methods mark pointers that cross compartment boundaries. They are
      * called in per-zone GCs to prevent the wrappers' outgoing edges from

js/src/jspubtd.h

@@ -196,13 +196,6 @@ class JS_PUBLIC_API(AutoGCRooter)
     static void traceAll(JSTracer* trc);
     static void traceAllWrappers(JSTracer* trc);
 
-    /* T must be a context type */
-    template<typename T>
-    static void traceAllInContext(T* cx, JSTracer* trc) {
-        for (AutoGCRooter* gcr = cx->roots.autoGCRooters_; gcr; gcr = gcr->down)
-            gcr->trace(trc);
-    }
-
   protected:
     AutoGCRooter * const down;
 

js/src/vm/Runtime.cpp

@@ -385,22 +385,6 @@ JSRuntime::destroyRuntime()
             CancelOffThreadIonCompile(comp, nullptr);
         CancelOffThreadParses(this);
 
-        /* Clear debugging state to remove GC roots. */
-        for (CompartmentsIter comp(this, SkipAtoms); !comp.done(); comp.next()) {
-            if (WatchpointMap* wpmap = comp->watchpointMap)
-                wpmap->clear();
-        }
-
-        /*
-         * Clear script counts map, to remove the strong reference on the
-         * JSScript key.
-         */
-        for (CompartmentsIter comp(this, SkipAtoms); !comp.done(); comp.next())
-            comp->clearScriptCounts();
-
-        /* Clear atoms to remove GC roots and heap allocations. */
-        finishAtoms();
-
         /* Remove persistent GC roots. */
         gc.finishRoots();
 
@@ -423,12 +407,6 @@ JSRuntime::destroyRuntime()
     MOZ_ASSERT(ionLazyLinkListSize_ == 0);
     MOZ_ASSERT(ionLazyLinkList_.isEmpty());
 
-    /*
-     * Clear the self-hosted global and delete self-hosted classes *after*
-     * GC, as finalizers for objects check for clasp->finalize during GC.
-     */
-    finishSelfHosting();
-
     MOZ_ASSERT(!numExclusiveThreads);
     AutoLockForExclusiveAccess lock(this);
 

js/src/vm/Runtime.h

@@ -1048,6 +1048,7 @@ struct JSRuntime : public JS::shadow::Runtime,
   public:
     bool initializeAtoms(JSContext* cx);
     void finishAtoms();
+    bool atomsAreFinished() const { return !atoms_; }
 
     void sweepAtoms();
 

js/src/vm/ScopeObject.cpp

@@ -2552,6 +2552,12 @@ DebugScopes::sweep(JSRuntime* rt)
     liveScopes.sweep();
 }
 
+void
+DebugScopes::finish()
+{
+    proxiedScopes.clear();
+}
+
 #ifdef JSGC_HASH_TABLE_CHECKS
 void
 DebugScopes::checkHashTablesAfterMovingGC(JSRuntime* runtime)

js/src/vm/ScopeObject.h

@@ -1352,6 +1352,7 @@ class DebugScopes
   public:
     void mark(JSTracer* trc);
     void sweep(JSRuntime* rt);
+    void finish();
 #ifdef JS_GC_ZEAL
     void checkHashTablesAfterMovingGC(JSRuntime* rt);
 #endif