From 240896825f83f4d1530636d355ebb7b4de12d492 Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Fri, 5 Aug 2016 14:13:35 -0700 Subject: [PATCH] Bug 1290551 - Part 2: Assert that finishRoots actually unroots everything; r=jonco --HG-- extra : rebase_source : 322bbaf46bb1dc1b14bef0a939a07702f478c01c --- js/src/gc/Nursery.cpp | 4 ++++ js/src/gc/RootMarking.cpp | 42 +++++++++++++++++++++++++++++++++++++-- js/src/jit/Ion.cpp | 6 ++++++ js/src/jsatom.cpp | 4 ++++ js/src/jscompartment.cpp | 21 ++++++++++++++++++++ js/src/jscompartment.h | 4 ++++ js/src/jspubtd.h | 7 ------- js/src/vm/Runtime.cpp | 22 -------------------- js/src/vm/Runtime.h | 1 + js/src/vm/ScopeObject.cpp | 6 ++++++ js/src/vm/ScopeObject.h | 1 + 11 files changed, 87 insertions(+), 31 deletions(-) diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index f39ede44cb0d..6ceae2d94b7c 100644 --- a/js/src/gc/Nursery.cpp +++ b/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(); } diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index 6b37def196fb..0b64e6dbc645 100644 --- a/js/src/gc/RootMarking.cpp +++ b/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(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. diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index 31af50e6295f..a975688a7f3c 100644 --- a/js/src/jit/Ion.cpp +++ b/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(); !i.done(); i.next()) { JitCode* code = i; diff --git a/js/src/jsatom.cpp b/js/src/jsatom.cpp index 8f4bd9126e49..b80e5baf9839 100644 --- a/js/src/jsatom.cpp +++ b/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()) diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 67711d33b6b0..d8546fa4fee7 100644 --- a/js/src/jscompartment.cpp +++ b/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() { diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index a6d7924aadba..5e091ac848b5 100644 --- a/js/src/jscompartment.h +++ b/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 diff --git a/js/src/jspubtd.h b/js/src/jspubtd.h index 772ade2d9859..0b1ba709560a 100644 --- a/js/src/jspubtd.h +++ b/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 - static void traceAllInContext(T* cx, JSTracer* trc) { - for (AutoGCRooter* gcr = cx->roots.autoGCRooters_; gcr; gcr = gcr->down) - gcr->trace(trc); - } - protected: AutoGCRooter * const down; diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp index bb8eb7317906..307861d4e9ff 100644 --- a/js/src/vm/Runtime.cpp +++ b/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); diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index 18c2969961d1..323076902eda 100644 --- a/js/src/vm/Runtime.h +++ b/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(); diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp index 204fd2bcb14b..fbf0fbe954e2 100644 --- a/js/src/vm/ScopeObject.cpp +++ b/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) diff --git a/js/src/vm/ScopeObject.h b/js/src/vm/ScopeObject.h index 59416409bb82..826f6d6642dc 100644 --- a/js/src/vm/ScopeObject.h +++ b/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