Closed
Bug 1384550
Opened 7 years ago
Closed 7 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
Categories
(Core :: JavaScript Engine, defect, P5)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: jonco)
References
Details
(Keywords: crash, intermittent-failure)
Crash Data
Attachments
(1 file)
1.74 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
Filed by: cbook [at] mozilla.com https://treeherder.mozilla.org/logviewer.html#?job_id=117957068&repo=mozilla-inbound https://queue.taskcluster.net/v1/task/I7QnQmbmTvWOu3rirx0DEA/runs/0/artifacts/public/logs/live_backing.log
Assignee | ||
Comment 1•7 years ago
|
||
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
Assignee | ||
Comment 2•7 years ago
|
||
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 hidden (offtopic) |
Comment 4•7 years ago
|
||
Comment on attachment 8890468 [details] [diff] [review] bug1384550-atoms-assertion 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 jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f13dfb12254e Skip atoms collection if it's no longer possible r=sfink
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f13dfb12254e
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
status-firefox54:
--- → unaffected
status-firefox55:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment hidden (Intermittent Failures Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•