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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: gwagner)

Tracking

({crash, testcase})

Trunk
x86_64
Mac OS X
crash, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey, crash signature)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 526693 [details]
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

6 years ago
Interesting. Gregor?
(Assignee)

Comment 2

6 years ago
Created attachment 526790 [details] [diff] [review]
patch

null is a gcThing for historically reasons according to luke.
isMarkable is what we want.
Assignee: general → anygregor
(Assignee)

Updated

6 years ago
Attachment #526790 - Flags: review?(gal)

Comment 3

6 years ago
Comment on attachment 526790 [details] [diff] [review]
patch

Doh. Thanks.
Attachment #526790 - Flags: review?(gal) → review+
(Assignee)

Comment 4

6 years ago
http://hg.mozilla.org/tracemonkey/rev/59325b2ca38b
Whiteboard: fixed-in-tracemonkey

Comment 5

6 years ago
jorendorff, this is probably the crash we had during the aurora landing
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.
(Reporter)

Comment 7

6 years ago
> 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
(Reporter)

Comment 8

6 years ago
Why does this bug only happen with a forced GC, and not with normal GCs or shutdown GC?
(Assignee)

Comment 9

6 years ago
(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.
(Reporter)

Comment 10

6 years ago
Interesting. So we might want to check in several testcases, with various combinations of keeping the key and the map alive.
Pushed: http://hg.mozilla.org/tracemonkey/rev/332284d2b284

Jesse: <3
http://hg.mozilla.org/mozilla-central/rev/332284d2b284
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Crash Signature: [@ js::gc::Cell::compartment]
A testcase for this bug was automatically identified at js/src/tests/js1_8_5/extensions/regress-650753.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.