650753 - Crash [@ js::gc::Cell::compartment] with WeakMap, forced GC

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Attachment: testcase (requires extension for GC) (crashes when loaded)

Tested version: http://hg.mozilla.org/tracemonkey/rev/63a06fbd23e0

1. Install 'DOM Fuzz Lite' from
    https://www.squarefree.com/extensions/domFuzzLite.xpi
2. Load the testcase.

Result: crash [@ js::gc::Cell::compartment]

I couldn't get it to crash without the extension, which is surprising because the forced GC is at the end of the testcase.  I'm curious what's going on here.

Comment 1

[Andreas Gal :gal]

Interesting. Gregor?

Comment 2

[Gregor Wagner [:gwagner]]

Attachment: patch

null is a gcThing for historically reasons according to luke.
isMarkable is what we want.

Comment 3

[Andreas Gal :gal]

Comment on attachment 526790 [details] [diff] [review]
patch

Doh. Thanks.

Comment 4

[Gregor Wagner [:gwagner]]

http://hg.mozilla.org/tracemonkey/rev/59325b2ca38b

Comment 5

[Andreas Gal :gal]

jorendorff, this is probably the crash we had during the aurora landing

Comment 6

[Jason Orendorff [:jorendorff]]

I'm so glad this turned up sooner rather than later.

Though, this patch absolutely should not have gotten an r+ without a js test. Don't worry about it, I'll push one.

Comment 7

[Jesse Ruderman]

> I'm so glad this turned up sooner rather than later.

I found this by adding special WeakMap stuff to my DOM fuzzer and running it on TM branch (which I don't normally do).

Just for you <3

Comment 8

[Jesse Ruderman]

Why does this bug only happen with a forced GC, and not with normal GCs or shutdown GC?

Comment 9

[Gregor Wagner [:gwagner]]

(In reply to comment #8)
> Why does this bug only happen with a forced GC, and not with normal GCs or
> shutdown GC?

This path is only executed when the key is alive. I guess the keys are already  unreachable if we wait for the normal GC.

Comment 10

[Jesse Ruderman]

Interesting. So we might want to check in several testcases, with various combinations of keeping the key and the map alive.

Comment 11

[Jason Orendorff [:jorendorff]]

Pushed: http://hg.mozilla.org/tracemonkey/rev/332284d2b284

Jesse: <3

Comment 12

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/332284d2b284

Comment 13

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/tests/js1_8_5/extensions/regress-650753.js.