903548 - GC: What do we do for UnmarkGray on a Nursery GCThing?

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Right now we assert that any object that enters UnmarkGray is tenured. This causes the browser to crash almost immediately when GGC is enabled. I'm not at all sure what we should be doing here.

Comment 1

[Andrew McCreight [:mccr8]]

What does the stack look like for the assertion?

Comment 2

[Terrence Cole [:terrence]]

#0  js::gc::Cell::unmark (this=0x7fffde00e900, color=1) at /home/jon/work/rooting/js/src/gc/Heap.h:997
#1  0x00007ffff307b50a in UnmarkGrayGCThing (thing=0x7fffde00e900) at /home/jon/work/rooting/js/src/gc/Marking.cpp:1570
#2  0x00007ffff307b462 in JS::UnmarkGrayGCThingRecursively (thing=0x7fffde00e900, kind=JSTRACE_OBJECT) at /home/jon/work/rooting/js/src/gc/Marking.cpp:1690
#3  0x00007ffff21240fc in FixWeakMappingGrayBitsTracer::FixWeakMappingGrayBits (trc=0x7fffffff4450, m=(JSObject *) 0x7fffdac3e250 [object WeakMap], k=0x7fffdb1edd00, kkind=JSTRACE_OBJECT, v=0x7fffde00e900, vkind=JSTRACE_OBJECT)
    at /home/jon/work/rooting/xpcom/base/CycleCollectedJSRuntime.cpp:271
#4  0x00007ffff33dc3d8 in js::WeakMap<js::EncapsulatedPtr<JSObject, unsigned long>, js::RelocatableValue, js::DefaultHasher<js::EncapsulatedPtr<JSObject, unsigned long> > >::traceMappings (this=0x7fffc10f1d80, tracer=0x7fffffff4450)
    at /home/jon/work/rooting/js/src/jsweakmap.h:224
#5  0x00007ffff33d66e5 in js::WeakMapBase::traceAllMappings (tracer=0x7fffffff4450) at /home/jon/work/rooting/js/src/jsweakmap.cpp:63
#6  0x00007ffff3279af5 in js::TraceWeakMaps (trc=0x7fffffff4450) at /home/jon/work/rooting/js/src/jsfriendapi.cpp:573
#7  0x00007ffff2120e54 in FixWeakMappingGrayBitsTracer::FixAll (this=0x7fffffff4450) at /home/jon/work/rooting/xpcom/base/CycleCollectedJSRuntime.cpp:232
#8  0x00007ffff2120df9 in mozilla::CycleCollectedJSRuntime::FixWeakMappingGrayBits (this=0x7fffe5643000) at /home/jon/work/rooting/xpcom/base/CycleCollectedJSRuntime.cpp:1028
#9  0x00007ffff21289ff in nsCycleCollector::FixGrayBits (this=0x7fffe5672000, aForceGC=false) at /home/jon/work/rooting/xpcom/base/nsCycleCollector.cpp:2806
#10 0x00007ffff2128640 in nsCycleCollectorRunner::Collect (this=0x7ffff6c28740, aCCType=ScheduledCC, aResults=0x7fffffffc528, aListener=0x0) at /home/jon/work/rooting/xpcom/base/nsCycleCollector.cpp:1157
#11 0x00007ffff212cf0f in nsCycleCollector::Collect (this=0x7fffe5672000, aCCType=ScheduledCC, aResults=0x7fffffffc528, aListener=0x0) at /home/jon/work/rooting/xpcom/base/nsCycleCollector.cpp:2920
#12 0x00007ffff212edf8 in nsCycleCollector_collect (aManuallyTriggered=false, aResults=0x7fffffffc528, aListener=0x0) at /home/jon/work/rooting/xpcom/base/nsCycleCollector.cpp:3361
#13 0x00007ffff0805874 in nsJSContext::CycleCollectNow (aListener=0x0, aExtraForgetSkippableCalls=0, aManuallyTriggered=false) at /home/jon/work/rooting/dom/base/nsJSEnvironment.cpp:2413
#14 0x00007ffff080e038 in CCTimerFired (aTimer=0x7fffc0bca860, aClosure=0x0) at /home/jon/work/rooting/dom/base/nsJSEnvironment.cpp:2624

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

We should just treat everything in the nursery as black. We should be able to stick checks in UnmarkGray so that it skips nursery things (although it will still have to recurse into their children) as well as whatever code returns the color so that nursery things are always considered black.

This does mean that the cycle collector won't be able to collect cycles until all the JS objects in the cycle are tenured, but that seems fine.

Comment 4

[Andrew McCreight [:mccr8]]

That sounds reasonable to me.

Do objects remain in the nursery across GCs?  If not, they shouldn't ever hold onto anything gray.

I'll have to think about this some more.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: gray-marking

Here's a patch to address this.  It does the following:

 - JS::UnmarkGrayGCThingRecursively() does not attempt to unmark things in the nursery, but it does unmark their children, and it also returns whether it unmarked anything
 - JS::GCThingIsMarkedGray() returns false for things in the nursery
 - js::VisitGrayWrapperTargets() does not visit things in the nursery
 - Cross compartment wrapper targets are not marked if they are in the nursery

Comment 6

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85b53f097e1f

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/85b53f097e1f