Closed Bug 903548 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: terrence, Assigned: jonco)

References

Details

Attachments

(1 file)

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.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(continuation)
Summary: GC: What do we do for UnmarkGray on a Nursery GCThing → GC: What do we do for UnmarkGray on a Nursery GCThing?
What does the stack look like for the assertion?
#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
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.
Flags: needinfo?(wmccloskey)
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.
Flags: needinfo?(continuation)
Assignee: general → jcoppeard
Attached patch gray-markingSplinter Review
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
Attachment #789664 - Flags: review?(wmccloskey)
Attachment #789664 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/85b53f097e1f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.