Bug 1016738 - Simplify/fix "dead compartment" logic (r=luke,jonco)

This commit is contained in:
Bill McCloskey 2014-05-30 07:46:12 -07:00
parent 382a955997
commit ab59354e6b
17 changed files with 109 additions and 150 deletions

View File

@ -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) \

View File

@ -188,9 +188,6 @@ CheckMarkedThing(JSTracer *trc, T **thingp)
DebugOnly<JSRuntime *> 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<typename T>
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<typename T>
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<T>::kind);
trc->unsetTracingLocation();

View File

@ -638,7 +638,20 @@ GCMarker::appendGrayRoot(void *thing, JSGCTraceKind kind)
Zone *zone = static_cast<Cell *>(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<JSObject *>(thing)->compartment()->maybeAlive = true;
break;
case JSTRACE_SCRIPT:
static_cast<JSScript *>(thing)->compartment()->maybeAlive = true;
break;
default:
break;
}
if (!zone->gcGrayRoots.append(root)) {
resetBufferedGrayRoots();
grayBufferState = GRAY_BUFFER_FAILED;

View File

@ -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),

View File

@ -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;

View File

@ -1090,7 +1090,6 @@ JS_TransplantObject(JSContext *cx, HandleObject origobj, HandleObject target)
JS_ASSERT(!origobj->is<CrossCompartmentWrapperObject>());
JS_ASSERT(!target->is<CrossCompartmentWrapperObject>());
AutoMaybeTouchDeadZones agc(cx);
AutoDisableProxyCheck adpc(cx->runtime());
JSCompartment *destination = target->compartment();

View File

@ -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

View File

@ -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_;

View File

@ -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<JSObject*>(cell));
else if (kind == JSTRACE_STRING)

View File

@ -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<JSObject *>(key.wrapped)->compartment();
break;
case CrossCompartmentKey::DebuggerScript:
dest = static_cast<JSScript *>(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)
{

View File

@ -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
{

View File

@ -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()));

View File

@ -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<WrapperOptions> 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()) {

View File

@ -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 */

View File

@ -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<GlobalObject*> 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();

View File

@ -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());

View File

@ -356,9 +356,6 @@ XPCWrappedNative::GetNewOrUsed(xpcObjectHelper& helper,
mozilla::Maybe<JSAutoCompartment> 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);