981809 - Crash in mochitest-other during WeakMap markIteratively

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Steve Fink [:sfink] [:s:]]

On the Maple branch, we have a reliable crash at some point during M-oth. See eg https://tbpl.mozilla.org/?tree=Maple&rev=b129030101a5

It happens on all platforms. It happens for all debug builds, but some of the opt builds escape it.

The test it happens in varies, but the top of the stack always looks like:

libxul.so!js::WeakMap<js::EncapsulatedPtr<JSObject, unsigned int>, js::EncapsulatedPtr<JSObject, unsigned int>, js::DefaultHasher<js::EncapsulatedPtr<JSObject, unsigned int> > >::markIteratively(JSTracer*) [jsweakmap.h:b129030101a5 : 165 + 0x2]
libxul.so!js::WeakMapBase::markCompartmentIteratively(JSCompartment*, JSTracer*) [jsweakmap.cpp:b129030101a5 : 42 + 0xb]
libxul.so!MarkWeakReferences<js::CompartmentsIterT<js::gc::GCZoneGroupIter> > [jsgc.cpp:b129030101a5 : 3164 + 0x7]
libxul.so!MarkGrayReferences<js::gc::GCZoneGroupIter, js::CompartmentsIterT<js::gc::GCZoneGroupIter> > [jsgc.cpp:b129030101a5 : 3205 + 0xc]
libxul.so!BeginSweepPhase [jsgc.cpp:b129030101a5 : 3215 + 0x6]
libxul.so!IncrementalCollectSlice [jsgc.cpp:b129030101a5 : 4652 + 0x4]
libxul.so!GCCycle [jsgc.cpp:b129030101a5 : 4791 + 0x21]
libxul.so!Collect [jsgc.cpp:b129030101a5 : 4929 + 0x1e]
libxul.so!js::GC(JSRuntime*, js::JSGCInvocationKind, JS::gcreason::Reason) [jsgc.cpp:b129030101a5 : 4961 + 0x2f]
libxul.so!JS::GCForReason(JSRuntime*, JS::gcreason::Reason) [jsfriendapi.cpp:b129030101a5 : 200 + 0x17]
libxul.so!nsJSContext::GarbageCollectNow(JS::gcreason::Reason, nsJSContext::IsIncremental, nsJSContext::IsCompartment, nsJSContext::IsShrinking, long long) [nsJSEnvironment.cpp:b129030101a5 : 1866 + 0xf]
libxul.so!nsDOMWindowUtils::GarbageCollect(nsICycleCollectorListener*, int) [nsDOMWindowUtils.cpp:b129030101a5 : 1402 + 0x12]

Comment 1

[Steve Fink [:sfink] [:s:]]

The crash happens because a major GC tries to mark a JSObject* whose contents have been moved (swept by the nursery; the object is filled with 0x2b).

I walked through the rest of the WeakMap. It contains 6 keys:

 - [2] XMLHttpRequestPrototype
 - [9] HTMLDocument
 - [11] HTMLParagraphElement
 - [22] Window
 - [27] CanvasRenderingContext2D
 - [28] <swept nursery>

I also added an assertion to catch insertions to the WeakMap and verify that the object isn't already bogus. It did not fire.

Comment 2

[Steve Fink [:sfink] [:s:]]

bholley, I don't suppose you would recognize what WeakMap that is offhand from the keys in comment 1?

Comment 3

[Steve Fink [:sfink] [:s:]]

Oops, I seem to be putting the real info in this bug, so I probably shouldn't invalidate it.

Comment 4

[Bobby Holley (:bholley)]

possibly an xray expando map via JS::WeakMapPtr?

Comment 5

[Bobby Holley (:bholley)]

(because those are special, and because all the keys are native xrayable objects)

Comment 6

[Steve Fink [:sfink] [:s:]]

(In reply to Bobby Holley (:bholley) from comment #4)
> possibly an xray expando map via JS::WeakMapPtr?
>
> (because those are special, and because all the keys are native xrayable
> objects)

Hm, that's the one I saw too. In fact, it looks like XPCWrappedNativeScope::mXrayExpandos is not barriered and contains nursery-allocated keys. But I fixed that, and yet my crash persists. Maybe I did it wrong; I'll be checking that next.

Comment 7

[Steve Fink [:sfink] [:s:]]

Attachment: Postbarrier WeakMapPtr keys

Try pushes for this are:

  https://tbpl.mozilla.org/?tree=Try&rev=dd62797e9472
  https://tbpl.mozilla.org/?tree=Try&rev=2b8da1b2db05

which it's a little hard to be happy about, but at least this particular crash
is gone.

Comment 8

[Terrence Cole [:terrence]]

Comment on attachment 8390890 [details] [diff] [review]
Postbarrier WeakMapPtr keys

Review of attachment 8390890 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

When I reviewed the addition of WeakMapPtr, I assumed the barriering would be done externally and that Bobby would somehow magically know this needed doing. Just making the barriering internal is a much better solution in general; I should have seen the problem and insisted on this as a solution when I did the initial review.

Comment 9

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/b239389ec1e6

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b239389ec1e6