"Assertion failure: (obj)->compartment()->isGCMarking(),"

RESOLVED FIXED in Firefox 20

Status

()

defect
--
critical
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: gkw, Assigned: jonco)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla20
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox19 unaffected, firefox20 fixed, firefox21 fixed, firefox-esr17 unaffected, b2g18 unaffected)

Details

(Whiteboard: [jsbugmon:])

Attachments

(2 attachments)

Posted file stack
try {
    a = []
    r = /x/
    gczeal(10, 2)()
} catch (e) {}
try {
    (function() {
        r(function() {
            eval()
        })
    })()
} catch (e) {}
try {
    s
} catch (e) {}
a.every()

asserts js debug shell on m-c changeset f2a500997116 with -a at Assertion failure: (obj)->compartment()->isGCMarking(),

s-s because GC seems involved. Assuming sec-critical unless otherwise shown.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   116562:54696b3f852b
user:        Jon Coppeard
date:        Tue Dec 18 13:27:28 2012 +0000
summary:     Bug 820186 - Various crashes/assertions with gczeal(10) and random recursion. r=billm

Tested with --enable-more-deterministic, but I'm not sure if it's needed.
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Assignee: general → jcoppeard
Posted patch Potential fixSplinter Review
(In reply to Jon Coppeard (:jonco) from comment #2)
> Created attachment 697065 [details] [diff] [review]
> Potential fix

Is the testcase from bug 820186 (the one with the Mersenne Twister) intended to be landed in this patch as well?
(In reply to Gary Kwong [:gkw] from comment #3)
> (In reply to Jon Coppeard (:jonco) from comment #2)
> > Created attachment 697065 [details] [diff] [review]
> > Potential fix
> 
> Is the testcase from bug 820186 (the one with the Mersenne Twister) intended
> to be landed in this patch as well?

Well, it should have been in the fix for bug 820186.  But this bug was caused by the fix for that one so I included it here.
(In reply to Jon Coppeard (:jonco) from comment #2)
> Created attachment 697065 [details] [diff] [review]
> Potential fix

I verify that this patch does fix the bug. :)
> I verify that this patch does fix the bug. :)

In addition, this bug seems to only occur on Mac, it did not reproduce when I tested on 32-bit Linux js shell.
Attachment #697065 - Attachment is patch: true
Comment on attachment 697065 [details] [diff] [review]
Potential fix

It sounded like this was ready for review, so I'll mark it thus so it doesn't get lost.
Attachment #697065 - Flags: review?(wmccloskey)
Attachment #697065 - Flags: review?(wmccloskey) → review?(terrence)
Comment on attachment 697065 [details] [diff] [review]
Potential fix

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

::: js/src/jsgc.cpp
@@ +3823,5 @@
>        case SWEEP:
>          for (CompartmentsIter c(rt); !c.done(); c.next()) {
>              c->scheduledForDestruction = false;
>  
> +            if (c->isGCMarking() && c->activeAnalysis && !c->gcTypesMarked) {

Oh, wow. I'm wondering how this worked at all: do we just hit a reset during sweeping in a compartmental GC incredibly infrequently?
Attachment #697065 - Flags: review?(terrence) → review+
(In reply to Terrence Cole [:terrence] from comment #8)

Well, this code only got introduced recently.  And we hit the situation where analysis becomes active during GC pretty infrequently anyway, and on top of that it has to be a compartmental GC as well. 

Cheers for the review.
Backed out and relanded with correct bug number in commit message:

https://hg.mozilla.org/integration/mozilla-inbound/rev/05895b39ed9e
https://hg.mozilla.org/mozilla-central/rev/05895b39ed9e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
A testcase for this bug was automatically identified at js/src/jit-test/tests/gc/bug-825326.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.