Closed
Bug 1390087
Opened 7 years ago
Closed 7 years ago
Assertion failure: zone->isGCScheduled(), at js/src/jsgc.cpp:6785
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | wontfix |
firefox56 | --- | fixed |
firefox57 | --- | fixed |
People
(Reporter: gkw, Assigned: jonco)
References
Details
(Keywords: assertion, bugmon, testcase, Whiteboard: [jsbugmon:update])
Attachments
(2 files)
25.86 KB,
text/plain
|
Details | |
2.00 KB,
patch
|
sfink
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 3bfcbdf5c6c3 (build with --enable-debug, run with --fuzzing-safe --ion-offthread-compile=off --no-baseline --no-ion): // jsfunfuzz-generated setGCCallback({ action: "majorGC" }); // Adapted from randomly chosen test: js/src/jit-test/tests/gc/bug-1374797.js gcparam("allocationThreshold", 1); offThreadCompileScript(""); for (let i = 0; i < 99999; i++) { Symbol.for(i); } eval(0); Backtrace: #0 js::gc::GCRuntime::budgetIncrementalGC (this=0x7f8ca4b4d670, nonincrementalByAPI=<optimized out>, reason=<optimized out>, budget=..., lock=...) at js/src/jsgc.cpp:6791 #1 0x00000000009f626e in js::gc::GCRuntime::gcCycle (this=this@entry=0x7f8ca4b4d670, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::ALLOC_TRIGGER) at js/src/jsgc.cpp:6901 #2 0x00000000009f6a85 in js::gc::GCRuntime::collect (this=this@entry=0x7f8ca4b4d670, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::ALLOC_TRIGGER) at js/src/jsgc.cpp:7060 #3 0x00000000009f6ef2 in js::gc::GCRuntime::gcSlice (this=0x7f8ca4b4d670, reason=JS::gcreason::ALLOC_TRIGGER, millis=0) at js/src/jsgc.cpp:7146 #4 0x00000000009f70f1 in js::gc::GCRuntime::gcIfRequested (this=0x7f8ca4b4d670) at js/src/jsgc.cpp:7348 /snip For detailed crash information, see attachment. Setting s-s as a start because this seems to involve GC.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/e33e38d61829 user: Jon Coppeard date: Tue Jul 25 11:28:41 2017 +0100 summary: Bug 1374797 - Fix logic around triggering atoms GCs r=sfink Jon, is bug 1374797 a likely regressor?
Blocks: 1374797
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 3•7 years ago
|
||
The problem here is that when we're scheduling zones for GC and checking the GC budget we look at all zones, even those used by helper threads that we can't collect anyway. So let's ignore these.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Attachment #8896961 -
Flags: review?(sphink)
Updated•7 years ago
|
status-firefox55:
--- → wontfix
status-firefox56:
--- → affected
status-firefox-esr52:
--- → affected
Updated•7 years ago
|
Updated•7 years ago
|
Attachment #8896961 -
Flags: review?(sphink) → review+
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2e5d28a1a277 Ignore zones that we can't collect when scheduling zones for GC r=sfink
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2e5d28a1a277
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 7•7 years ago
|
||
Please request Beta approval on this when you get a chance.
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8896961 [details] [diff] [review] bug1390087-zone-scheduling Approval Request Comment [Feature/Bug causing the regression]: This has been like this since forever basically. [User impact if declined]: We could occasionally end up doing GCs non-incrementally for no good reason. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: It's a very simple change that ignores zones that we aren't going to collect anyway when budgeting GC. [String changes made/needed]: None.
Flags: needinfo?(jcoppeard)
Attachment #8896961 -
Flags: approval-mozilla-beta?
Comment 9•7 years ago
|
||
Comment on attachment 8896961 [details] [diff] [review] bug1390087-zone-scheduling Fix an assertion. Looks low risk. Beta56+.
Attachment #8896961 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 10•7 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/594c90fb0183968b0d4ffe3ea2a5a5a0578be3bb
Updated•7 years ago
|
Comment 11•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #8) > [Is this code covered by automated tests?]: Yes. > [Has the fix been verified in Nightly?]: Yes. > [Needs manual test from QE? If yes, steps to reproduce]: No. Setting qe-verify- based on Jon's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•