Closed Bug 1384550 Opened 5 years ago Closed 5 years ago

Intermittent dom/html/test/test_bug1264157.html | application crashed [@ js::gc::GCRuntime::prepareZonesForCollection] after Assertion failure: atomsZone->isGCMarking(), at /home/worker/workspace/build/src/js/src/jsgc.cpp:3962


(Core :: JavaScript Engine, defect, P5)




Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed


(Reporter: intermittent-bug-filer, Assigned: jonco)



(Keywords: crash, intermittent-failure)

Crash Data


(1 file)

The is the assertion added in bug 1374797:

     * Check that we do collect the atoms zone if we triggered a GC for that
     * purpose.
    MOZ_ASSERT_IF(reason == JS::gcreason::DELAYED_ATOMS_GC, atomsZone->isGCMarking());
Assignee: nobody → jcoppeard
Blocks: 1374797
I think this is happening because when we trigger a GC it doesn't happen immediately, and when it does happen the current state may have changed and we're now unable to collect the atoms zone.

Since this doesn't happen very often I added a check in gcIfRequested() and made it skip the collection.  In practice I expect the atoms GC will get triggered the next time an atom is allocated.  The patch clears majorGCTriggerReason so that it doesn't prevent another GC of any kind from being triggered.
Attachment #8890468 - Flags: review?(sphink)
Comment on attachment 8890468 [details] [diff] [review]

Review of attachment 8890468 [details] [diff] [review]:

I was happy to see that assert go in, because it seemed like very nonlocal state that would be nice to depend upon but the proof of why it should hold was non-obvious. So I think this is a good result -- without it, our telemetry ends up with GCs triggered by atoms that don't collect atoms, not to mention that's probably not very useful behavior.
Attachment #8890468 - Flags: review?(sphink) → review+
Pushed by
Skip atoms collection if it's no longer possible r=sfink
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Duplicate of this bug: 1385246
You need to log in before you can comment on or make changes to this bug.