1890670 - Assertion failure: markColor() >= targetColor using symbols as weakmap keys

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P3)

Description

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

I was reviewing https://phabricator.services.mozilla.com/D194973 and couldn't see where Symbols were handled. I tried this test, and it crashed with an assertion failure:

% $JS --enable-symbols-as-weakmap-keys -e 'wm = new WeakMap(); s = Symbol(); wm.set(s, new WeakMap()); ss = Symbol(); wm.get(s).set(this, ss); wm2 = new WeakMap(); wm2.set(ss, "test"); ss = null; gc()' 
Assertion failure: markColor() >= targetColor, at /home/sfink/src/mozilla4/js/src/gc/Marking.cpp:751

I don't know that this is the same thing, but it seems like at least ImplicitEdgeHolderType<Symbol> should be fixed, and processMarkStackTop seems like it should handle them too.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Also Bug 1876687 has similar assertion failure.

Comment 2

[Jon Coppeard (:jonco)]

Well this is unfortunate.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1828144

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1890670 - Trace implicit edges for symbols r?sfink

Allowing symbols as weakmap keys means that we have to trace implicit edges
generated by these. The patch adds JS::Symbol to the types we do this for.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

What's the user-facing impact of this bug?

Comment 6

[Jon Coppeard (:jonco)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)

What's the user-facing impact of this bug?

The symbols-as-weakmap-keys feature is nightly-only and behind a pref, so does not affect release builds at all. Which is a good thing given this bug.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1828144

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a1193dacf65e
Trace implicit edges for symbols r=sfink

Comment 10

[tszentpeteri]

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