diff --git a/js/public/GCAPI.h b/js/public/GCAPI.h index 9abc1f0cab7..be58d349d56 100644 --- a/js/public/GCAPI.h +++ b/js/public/GCAPI.h @@ -45,7 +45,7 @@ namespace JS { D(TOO_MUCH_MALLOC) \ D(ALLOC_TRIGGER) \ D(DEBUG_GC) \ - D(TRANSPLANT) \ + D(COMPARTMENT_REVIVED) \ D(RESET) \ D(OUT_OF_NURSERY) \ D(EVICT_NURSERY) \ diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index e8118faf4dc..e2fb60a5524 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -188,9 +188,6 @@ CheckMarkedThing(JSTracer *trc, T **thingp) DebugOnly rt = trc->runtime(); - JS_ASSERT_IF(IS_GC_MARKING_TRACER(trc) && rt->gc.manipulatingDeadZones, - !thing->zone()->scheduledForDestruction); - JS_ASSERT(CurrentThreadCanAccessRuntime(rt)); JS_ASSERT_IF(thing->zone()->requireGCTracer(), @@ -221,6 +218,33 @@ CheckMarkedThing(JSTracer *trc, T **thingp) } +/* + * We only set the maybeAlive flag for objects and scripts. It's assumed that, + * if a compartment is alive, then it will have at least some live object or + * script it in. Even if we get this wrong, the worst that will happen is that + * scheduledForDestruction will be set on the compartment, which will cause some + * extra GC activity to try to free the compartment. + */ +template +static inline void +SetMaybeAliveFlag(T *thing) +{ +} + +template<> +void +SetMaybeAliveFlag(JSObject *thing) +{ + thing->compartment()->maybeAlive = true; +} + +template<> +void +SetMaybeAliveFlag(JSScript *thing) +{ + thing->compartment()->maybeAlive = true; +} + template static void MarkInternal(JSTracer *trc, T **thingp) @@ -254,7 +278,7 @@ MarkInternal(JSTracer *trc, T **thingp) return; PushMarkStack(AsGCMarker(trc), thing); - thing->zone()->maybeAlive = true; + SetMaybeAliveFlag(thing); } else { trc->callback(trc, (void **)thingp, MapTypeToTraceKind::kind); trc->unsetTracingLocation(); diff --git a/js/src/gc/Tracer.cpp b/js/src/gc/Tracer.cpp index 18692ba175a..e029182af69 100644 --- a/js/src/gc/Tracer.cpp +++ b/js/src/gc/Tracer.cpp @@ -638,7 +638,20 @@ GCMarker::appendGrayRoot(void *thing, JSGCTraceKind kind) Zone *zone = static_cast(thing)->tenuredZone(); if (zone->isCollecting()) { - zone->maybeAlive = true; + // See the comment on SetMaybeAliveFlag to see why we only do this for + // objects and scripts. We rely on gray root buffering for this to work, + // but we only need to worry about uncollected dead compartments during + // incremental GCs (when we do gray root buffering). + switch (kind) { + case JSTRACE_OBJECT: + static_cast(thing)->compartment()->maybeAlive = true; + break; + case JSTRACE_SCRIPT: + static_cast(thing)->compartment()->maybeAlive = true; + break; + default: + break; + } if (!zone->gcGrayRoots.append(root)) { resetBufferedGrayRoots(); grayBufferState = GRAY_BUFFER_FAILED; diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp index 8432dedc1a1..163db7d0900 100644 --- a/js/src/gc/Zone.cpp +++ b/js/src/gc/Zone.cpp @@ -35,8 +35,6 @@ JS::Zone::Zone(JSRuntime *rt) data(nullptr), isSystem(false), usedByExclusiveThread(false), - scheduledForDestruction(false), - maybeAlive(true), active(false), jitZone_(nullptr), gcState_(NoGC), diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h index 431fd0afccc..a4f32593ed9 100644 --- a/js/src/gc/Zone.h +++ b/js/src/gc/Zone.h @@ -266,11 +266,6 @@ struct Zone : public JS::shadow::Zone, bool usedByExclusiveThread; - // These flags help us to discover if a compartment that shouldn't be alive - // manages to outlive a GC. - bool scheduledForDestruction; - bool maybeAlive; - // True when there are active frames. bool active; diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 691fadd736d..3420e809b89 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -1090,7 +1090,6 @@ JS_TransplantObject(JSContext *cx, HandleObject origobj, HandleObject target) JS_ASSERT(!origobj->is()); JS_ASSERT(!target->is()); - AutoMaybeTouchDeadZones agc(cx); AutoDisableProxyCheck adpc(cx->runtime()); JSCompartment *destination = target->compartment(); diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 99c0f4ce5b2..55e8c0b08ba 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -65,7 +65,9 @@ JSCompartment::JSCompartment(Zone *zone, const JS::CompartmentOptions &options = debugScriptMap(nullptr), debugScopes(nullptr), enumerators(nullptr), - compartmentStats(nullptr) + compartmentStats(nullptr), + scheduledForDestruction(false), + maybeAlive(true) #ifdef JS_ION , jitCompartment_(nullptr) #endif diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index 65d68457bdc..925968878d1 100644 --- a/js/src/jscompartment.h +++ b/js/src/jscompartment.h @@ -447,6 +447,11 @@ struct JSCompartment /* Used by memory reporters and invalid otherwise. */ void *compartmentStats; + // These flags help us to discover if a compartment that shouldn't be alive + // manages to outlive a GC. + bool scheduledForDestruction; + bool maybeAlive; + #ifdef JS_ION private: js::jit::JitCompartment *jitCompartment_; diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 32b45db2331..20bd4e1660c 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -969,8 +969,6 @@ JS::IncrementalObjectBarrier(JSObject *obj) JS_ASSERT(!obj->zone()->runtimeFromMainThread()->isHeapMajorCollecting()); - AutoMarkInDeadZone amn(obj->zone()); - JSObject::writeBarrierPre(obj); } @@ -987,8 +985,6 @@ JS::IncrementalReferenceBarrier(void *ptr, JSGCTraceKind kind) JS_ASSERT(!zone->runtimeFromMainThread()->isHeapMajorCollecting()); - AutoMarkInDeadZone amn(zone); - if (kind == JSTRACE_OBJECT) JSObject::writeBarrierPre(static_cast(cell)); else if (kind == JSTRACE_STRING) diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index ba0bca9f52d..e9b9e2cc92c 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -2897,14 +2897,14 @@ GCRuntime::beginMarkPhase() isFull = false; } - zone->scheduledForDestruction = false; - zone->maybeAlive = false; zone->setPreservingCode(false); } for (CompartmentsIter c(rt, WithAtoms); !c.done(); c.next()) { JS_ASSERT(c->gcLiveArrayBuffers.empty()); c->marked = false; + c->scheduledForDestruction = false; + c->maybeAlive = false; if (shouldPreserveJITCode(c, currentTime)) c->zone()->setPreservingCode(true); } @@ -3005,40 +3005,47 @@ GCRuntime::beginMarkPhase() bufferGrayRoots(); /* - * This code ensures that if a zone is "dead", then it will be - * collected in this GC. A zone is considered dead if its maybeAlive + * This code ensures that if a compartment is "dead", then it will be + * collected in this GC. A compartment is considered dead if its maybeAlive * flag is false. The maybeAlive flag is set if: - * (1) the zone has incoming cross-compartment edges, or - * (2) an object in the zone was marked during root marking, either + * (1) the compartment has incoming cross-compartment edges, or + * (2) an object in the compartment was marked during root marking, either * as a black root or a gray root. * If the maybeAlive is false, then we set the scheduledForDestruction flag. - * At any time later in the GC, if we try to mark an object whose - * zone is scheduled for destruction, we will assert. - * NOTE: Due to bug 811587, we only assert if gcManipulatingDeadCompartments - * is true (e.g., if we're doing a brain transplant). + * At the end of the GC, we look for compartments where + * scheduledForDestruction is true. These are compartments that were somehow + * "revived" during the incremental GC. If any are found, we do a special, + * non-incremental GC of those compartments to try to collect them. * - * The purpose of this check is to ensure that a zone that we would - * normally destroy is not resurrected by a read barrier or an - * allocation. This might happen during a function like JS_TransplantObject, - * which iterates over all compartments, live or dead, and operates on their - * objects. See bug 803376 for details on this problem. To avoid the - * problem, we are very careful to avoid allocation and read barriers during - * JS_TransplantObject and the like. The code here ensures that we don't - * regress. + * Compartments can be revived for a variety of reasons. On reason is bug + * 811587, where a reflector that was dead can be revived by DOM code that + * still refers to the underlying DOM node. * - * Note that there are certain cases where allocations or read barriers in - * dead zone are difficult to avoid. We detect such cases (via the - * gcObjectsMarkedInDeadCompartment counter) and redo any ongoing GCs after - * the JS_TransplantObject function has finished. This ensures that the dead - * zones will be cleaned up. See AutoMarkInDeadZone and - * AutoMaybeTouchDeadZones for details. + * Read barriers and allocations can also cause revival. This might happen + * during a function like JS_TransplantObject, which iterates over all + * compartments, live or dead, and operates on their objects. See bug 803376 + * for details on this problem. To avoid the problem, we try to avoid + * allocation and read barriers during JS_TransplantObject and the like. */ /* Set the maybeAlive flag based on cross-compartment edges. */ for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) { for (JSCompartment::WrapperEnum e(c); !e.empty(); e.popFront()) { - Cell *dst = e.front().key().wrapped; - dst->tenuredZone()->maybeAlive = true; + const CrossCompartmentKey &key = e.front().key(); + JSCompartment *dest = nullptr; + switch (key.kind) { + case CrossCompartmentKey::ObjectWrapper: + case CrossCompartmentKey::DebuggerObject: + case CrossCompartmentKey::DebuggerSource: + case CrossCompartmentKey::DebuggerEnvironment: + dest = static_cast(key.wrapped)->compartment(); + break; + case CrossCompartmentKey::DebuggerScript: + dest = static_cast(key.wrapped)->compartment(); + break; + } + if (dest) + dest->maybeAlive = true; } } @@ -3047,9 +3054,9 @@ GCRuntime::beginMarkPhase() * during MarkRuntime. */ - for (GCZonesIter zone(rt); !zone.done(); zone.next()) { - if (!zone->maybeAlive && !rt->isAtomsZone(zone)) - zone->scheduledForDestruction = true; + for (GCCompartmentsIter c(rt); !c.done(); c.next()) { + if (!c->maybeAlive && !rt->isAtomsCompartment(c)) + c->scheduledForDestruction = true; } foundBlackGrayEdges = false; @@ -4407,8 +4414,8 @@ GCRuntime::resetIncrementalGC(const char *reason) case SWEEP: marker.reset(); - for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) - zone->scheduledForDestruction = false; + for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) + c->scheduledForDestruction = false; /* Finish sweeping the current zone group, then abort. */ abortSweepAfterCurrentGroup = true; @@ -4886,13 +4893,30 @@ GCRuntime::collect(bool incremental, int64_t budget, JSGCInvocationKind gckind, if (poke && shouldCleanUpEverything) JS::PrepareForFullGC(rt); + /* + * This code makes an extra effort to collect compartments that we + * thought were dead at the start of the GC. See the large comment in + * beginMarkPhase. + */ + bool repeatForDeadZone = false; + if (incremental && incrementalState == NO_INCREMENTAL) { + for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) { + if (c->scheduledForDestruction) { + incremental = false; + repeatForDeadZone = true; + reason = JS::gcreason::COMPARTMENT_REVIVED; + c->zone()->scheduleGC(); + } + } + } + /* * If we reset an existing GC, we need to start a new one. Also, we * repeat GCs that happen during shutdown (the gcShouldCleanUpEverything * case) until we can be sure that no additional garbage is created * (which typically happens if roots are dropped during finalizers). */ - repeat = (poke && shouldCleanUpEverything) || wasReset; + repeat = (poke && shouldCleanUpEverything) || wasReset || repeatForDeadZone; } while (repeat); if (incrementalState == NO_INCREMENTAL) { @@ -5515,34 +5539,6 @@ ArenaLists::containsArena(JSRuntime *rt, ArenaHeader *needle) } -AutoMaybeTouchDeadZones::AutoMaybeTouchDeadZones(JSContext *cx) - : runtime(cx->runtime()), - markCount(runtime->gc.objectsMarkedInDeadZones), - inIncremental(JS::IsIncrementalGCInProgress(runtime)), - manipulatingDeadZones(runtime->gc.manipulatingDeadZones) -{ - runtime->gc.manipulatingDeadZones = true; -} - -AutoMaybeTouchDeadZones::AutoMaybeTouchDeadZones(JSObject *obj) - : runtime(obj->compartment()->runtimeFromMainThread()), - markCount(runtime->gc.objectsMarkedInDeadZones), - inIncremental(JS::IsIncrementalGCInProgress(runtime)), - manipulatingDeadZones(runtime->gc.manipulatingDeadZones) -{ - runtime->gc.manipulatingDeadZones = true; -} - -AutoMaybeTouchDeadZones::~AutoMaybeTouchDeadZones() -{ - runtime->gc.manipulatingDeadZones = manipulatingDeadZones; - - if (inIncremental && runtime->gc.objectsMarkedInDeadZones != markCount) { - JS::PrepareForFullGC(runtime); - js::GC(runtime, GC_NORMAL, JS::gcreason::TRANSPLANT); - } -} - AutoSuppressGC::AutoSuppressGC(ExclusiveContext *cx) : suppressGC_(cx->perThreadData->suppressGC) { diff --git a/js/src/jsgcinlines.h b/js/src/jsgcinlines.h index 5c291cc8621..b3b31326c44 100644 --- a/js/src/jsgcinlines.h +++ b/js/src/jsgcinlines.h @@ -15,33 +15,6 @@ namespace js { class Shape; -/* - * This auto class should be used around any code that might cause a mark bit to - * be set on an object in a dead zone. See AutoMaybeTouchDeadZones - * for more details. - */ -struct AutoMarkInDeadZone -{ - explicit AutoMarkInDeadZone(JS::Zone *zone) - : zone(zone), - scheduled(zone->scheduledForDestruction) - { - JSRuntime *rt = zone->runtimeFromMainThread(); - if (rt->gc.manipulatingDeadZones && zone->scheduledForDestruction) { - rt->gc.objectsMarkedInDeadZones++; - zone->scheduledForDestruction = false; - } - } - - ~AutoMarkInDeadZone() { - zone->scheduledForDestruction = scheduled; - } - - private: - JS::Zone *zone; - bool scheduled; -}; - inline Allocator * ThreadSafeContext::allocator() const { diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index da6a104f4b7..716d7686c10 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -2512,9 +2512,6 @@ JSObject::TradeGuts(JSContext *cx, JSObject *a, JSObject *b, TradeGutsReserved & bool JSObject::swap(JSContext *cx, HandleObject a, HandleObject b) { - AutoMarkInDeadZone adc1(a->zone()); - AutoMarkInDeadZone adc2(b->zone()); - // Ensure swap doesn't cause a finalizer to not be run. JS_ASSERT(IsBackgroundFinalized(a->tenuredGetAllocKind()) == IsBackgroundFinalized(b->tenuredGetAllocKind())); diff --git a/js/src/jswrapper.cpp b/js/src/jswrapper.cpp index fea1d54d1df..7afa4a4e40c 100644 --- a/js/src/jswrapper.cpp +++ b/js/src/jswrapper.cpp @@ -44,8 +44,6 @@ Wrapper::New(JSContext *cx, JSObject *obj, JSObject *parent, Wrapper *handler, { JS_ASSERT(parent); - AutoMarkInDeadZone amd(cx->zone()); - RootedValue priv(cx, ObjectValue(*obj)); mozilla::Maybe opts; if (!options) { @@ -1037,8 +1035,6 @@ JS_FRIEND_API(bool) js::RecomputeWrappers(JSContext *cx, const CompartmentFilter &sourceFilter, const CompartmentFilter &targetFilter) { - AutoMaybeTouchDeadZones agc(cx); - AutoWrapperVector toRecompute(cx); for (CompartmentsIter c(cx->runtime(), SkipAtoms); !c.done(); c.next()) { diff --git a/js/src/jswrapper.h b/js/src/jswrapper.h index ec67447f8e1..1aace7702c3 100644 --- a/js/src/jswrapper.h +++ b/js/src/jswrapper.h @@ -304,34 +304,6 @@ JS_FRIEND_API(bool) RecomputeWrappers(JSContext *cx, const CompartmentFilter &sourceFilter, const CompartmentFilter &targetFilter); -/* - * This auto class should be used around any code, such as brain transplants, - * that may touch dead zones. Brain transplants can cause problems - * because they operate on all compartments, whether live or dead. A brain - * transplant can cause a formerly dead object to be "reanimated" by causing a - * read or write barrier to be invoked on it during the transplant. In this way, - * a zone becomes a zombie, kept alive by repeatedly consuming - * (transplanted) brains. - * - * To work around this issue, we observe when mark bits are set on objects in - * dead zones. If this happens during a brain transplant, we do a full, - * non-incremental GC at the end of the brain transplant. This will clean up any - * objects that were improperly marked. - */ -struct JS_FRIEND_API(AutoMaybeTouchDeadZones) -{ - // The version that takes an object just uses it for its runtime. - explicit AutoMaybeTouchDeadZones(JSContext *cx); - explicit AutoMaybeTouchDeadZones(JSObject *obj); - ~AutoMaybeTouchDeadZones(); - - private: - JSRuntime *runtime; - unsigned markCount; - bool inIncremental; - bool manipulatingDeadZones; -}; - } /* namespace js */ #endif /* jswrapper_h */ diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 5453dc21887..6f9d58c58c2 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -2036,7 +2036,7 @@ Debugger::addAllGlobalsAsDebuggees(JSContext *cx, unsigned argc, Value *vp) for (CompartmentsInZoneIter c(zone); !c.done(); c.next()) { if (c == dbg->object->compartment() || c->options().invisibleToDebugger()) continue; - c->zone()->scheduledForDestruction = false; + c->scheduledForDestruction = false; GlobalObject *global = c->maybeGlobal(); if (global) { Rooted rg(cx, global); @@ -2833,7 +2833,7 @@ Debugger::findAllGlobals(JSContext *cx, unsigned argc, Value *vp) if (c->options().invisibleToDebugger()) continue; - c->zone()->scheduledForDestruction = false; + c->scheduledForDestruction = false; GlobalObject *global = c->maybeGlobal(); diff --git a/js/src/vm/ProxyObject.cpp b/js/src/vm/ProxyObject.cpp index e8521ee7647..e2c4e4549ac 100644 --- a/js/src/vm/ProxyObject.cpp +++ b/js/src/vm/ProxyObject.cpp @@ -80,7 +80,6 @@ NukeSlot(ProxyObject *proxy, uint32_t slot) Value old = proxy->getSlot(slot); if (old.isMarkable()) { Zone *zone = ZoneOfValue(old); - AutoMarkInDeadZone amd(zone); proxy->setReservedSlot(slot, NullValue()); } else { proxy->setReservedSlot(slot, NullValue()); diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp index f3e2238c9cf..01a147d8c18 100644 --- a/js/xpconnect/src/XPCWrappedNative.cpp +++ b/js/xpconnect/src/XPCWrappedNative.cpp @@ -356,9 +356,6 @@ XPCWrappedNative::GetNewOrUsed(xpcObjectHelper& helper, mozilla::Maybe ac; if (sciWrapper.GetFlags().WantPreCreate()) { - // PreCreate may touch dead compartments. - js::AutoMaybeTouchDeadZones agc(parent); - RootedObject plannedParent(cx, parent); nsresult rv = sciWrapper.GetCallback()->PreCreate(identity, cx, parent, parent.address()); @@ -1285,9 +1282,6 @@ RescueOrphans(HandleObject obj) return NS_OK; // Global object. We're done. parentObj = js::UncheckedUnwrap(parentObj, /* stopAtOuter = */ false); - // PreCreate may touch dead compartments. - js::AutoMaybeTouchDeadZones agc(parentObj); - // Recursively fix up orphans on the parent chain. rv = RescueOrphans(parentObj); NS_ENSURE_SUCCESS(rv, rv);