"Assertion failure: gc::IsMarked(x), at ../jsweakmap.h:140" with CountHeap

RESOLVED FIXED in Firefox 20

Status

()

defect
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: decoder, Assigned: billm)

Tracking

(Blocks 1 bug, {assertion, testcase})

Trunk
mozilla20
x86
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17 unaffected, firefox18 unaffected, firefox19 unaffected, firefox20 fixed, firefox-esr10 unaffected, firefox-esr17 unaffected, b2g18 unaffected)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
The following testcase asserts on mozilla-central revision fd919eb97465 (run with --ion-eager):


var g = newGlobal('new-compartment');
var dbg = Debugger(g);
function test(code, expected) {
    var actual = '';
    g.h = function () { actual += dbg.getNewestFrame().environment.type; }
    g.eval(code);
}
test("h();", 'object');
gczeal(4);
var count2 = countHeap();
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update,bisect]
Reproduced.
Assignee: general → jorendorff
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect,bisectfix]
(Reporter)

Comment 2

6 years ago
Updated wrong bug. nevermind.
Whiteboard: [jsbugmon:update,bisect,bisectfix] → [jsbugmon:update,bisect]
This is a known problem where we try to mark stuff during a non-marking trace of weakmaps (from CountHeap in this case).
Assignee: jorendorff → wmccloskey
Let me know if I do anything to help, Bill.
Summary: [jsdbg2] Assertion failure: gc::IsMarked(x), at ../jsweakmap.h:140 → "Assertion failure: gc::IsMarked(x), at ../jsweakmap.h:140" with CountHeap
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
(Reporter)

Comment 5

6 years ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   114221:2fd089d5fff4
user:        Jon Coppeard
date:        Fri Oct 12 15:26:07 2012 +0100
summary:     Bug 790338 - Fix issues with gray marking r=billm

This iteration took 55.588 seconds to run.
Posted patch patchSplinter Review
For posterity, here's why this happened. The test has a live weakmap that maps a dead key to a dead value. At the end, we check that the dead value is collected. Before every CC, we're doing ForgetSkippable and it's calling UnmarkGray on a bunch of stuff that eventually leads to the live weakmap. Then it calls UnmarkGray on the dead value, causing it not to be collected.

One fix would be to only UnmarkGray the value if the key is black. However, we're already planning to do a weakmap fixup pass in the cycle collector. It's a lot easier to rely on that and never trace through the weakmap values during UnmarkGray.
Attachment #693187 - Flags: review?(continuation)
Comment on attachment 693187 [details] [diff] [review]
patch

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

Thanks for investigating and fixing this!

::: js/src/gc/Marking.cpp
@@ +1506,5 @@
> +     * We set eagerlyTraceWeakMaps to false because the cycle collector will fix
> +     * up any color mismatches involving weakmaps when it runs.
> +     */
> +    UnmarkGrayTracer(JSRuntime *rt)
> +      : tracingShape(false), previousShape(NULL)

nit: your indentation is inconsistent with the other constructor. I don't know which is right.
Attachment #693187 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/95566cb18e2e

Should this have a test?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Bug 790338 (the suggested regressor) only landed in Firefox 20, marking as such.
You need to log in before you can comment on or make changes to this bug.