Closed
Bug 1398507
Opened 7 years ago
Closed 7 years ago
Assertion failure: fallibleScope_ ([OOM] Cannot allocate a new chunk in an infallible scope.), at js/src/ds/LifoAlloc.cpp:105
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: bc, Assigned: nbp)
References
()
Details
(Keywords: assertion)
Attachments
(2 files)
179.66 KB,
text/plain
|
Details | |
1.18 KB,
patch
|
tcampbell
:
review+
lizzard
:
approval-mozilla-release-
|
Details | Diff | Splinter Review |
1. http://iogames.space/snowfight-io/ wait for the game to complete loading or reload. 2. Assertion failure: fallibleScope_ ([OOM] Cannot allocate a new chunk in an infallible scope.), at /mozilla/builds/nightly/mozilla/js/src/ds/LifoAlloc.cpp:105 Beta 56, Nightly 57 on Fedora 26 with 16G. I didn't get close to OOM. The attached log is from a run on 2017-08-22. Bughunter didn't reproduce this today so I don't have a current crash report. Testing locally didn't give me a stack in the log.
Assignee | ||
Comment 1•7 years ago
|
||
I will investigate, but I suspect this might be a duplicate of Bug 1398105. This assertion exists on debug builds and is not really an OOM, it corresponds to an allocation site which relies too much on the infallible allocator. On release builds, if the allocation site is not checked, then it will likely fail later with a nullptr SEGV.
Assignee | ||
Comment 2•7 years ago
|
||
Based on the crash report, this is the only allocation in this loop which can cause such issue. Strangely enough, we would expect the vectors to reserve enough ballast space for this one to never trigger, but it might if we have purely math-oriented code with tons of loops.
Attachment #8906595 -
Flags: review?(tcampbell)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Comment 3•7 years ago
|
||
Comment on attachment 8906595 [details] [diff] [review] Use a fallible allocator for LoopAliasInfo allocations. Review of attachment 8906595 [details] [diff] [review]: ----------------------------------------------------------------- It also surprises me that this fails, but it is an allocation in a loop under user-controlled bounds so should be fixed anyways.
Attachment #8906595 -
Flags: review?(tcampbell) → review+
Pushed by npierron@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/061de9b749ef Use a fallible allocator for LoopAliasInfo allocations. r=tcampbell
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/061de9b749ef
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 6•7 years ago
|
||
Do you think this is worth considering for uplift to 56 since it's such a small patch and affects real-world websites? Or should it just ride the 57 train?
status-firefox55:
--- → wontfix
status-firefox56:
--- → affected
status-firefox-esr52:
--- → wontfix
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6) > Do you think this is worth considering for uplift to 56 since it's such a > small patch and affects real-world websites? Or should it just ride the 57 > train? As this assertion triggered on a real website, I will request an up-lift. Usually these assertions are found by fuzzers and never reported on any website, and we do not consider them for up-lift. Thanks for raising the question.
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8906595 [details] [diff] [review] Use a fallible allocator for LoopAliasInfo allocations. Approval Request Comment [Feature/Bug causing the regression]: Bug 937540 (?) [User impact if declined]: Tiny crashing rate. [Is this code covered by automated tests?]: No, because it is hard to reproduce. [Has the fix been verified in Nightly?]: No, only guesses based on the stack trace that this would be the only crash reason possible. Note, all crashes with this assertion have a common pattern, i-e an unbounded loop with an infallible allocation. Which is exactly what is fixed by this patch. [Needs manual test from QE? If yes, steps to reproduce]: No, no step to reproduce the issue. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Replace an infallible allocation (crashing on allocation failures), by a fallible allocation which report the allocation failure (as already done by other branches in the same function). [String changes made/needed]: None.
Attachment #8906595 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 9•7 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #1) > This assertion exists on debug builds and is not really an OOM, it > corresponds to an allocation site which relies too much on the infallible > allocator. On release builds, if the allocation site is not checked, then > it will likely fail later with a nullptr SEGV. Correcting my-self, on release build a crash would only happen on OOM. The OOM is controlled by AutoEnterOOMUnsafeRegion [1] class, which will cause a crash to be reported [2]. This is no longer a nullptr SEGV as mentioned before. [1] http://searchfox.org/mozilla-central/rev/e76c0fee79a34a3612740b33508276f4b37c3ee8/js/src/ds/LifoAlloc.h#293 [2] no results reported with this crash reason over the last 6 months: https://crash-stats.mozilla.com/search/?moz_crash_reason=%3D%5Bunhandlable%20oom%5D%20LifoAlloc%3A%3AallocInfallible&date=%3E%3D2017-03-14T10%3A15%3A43.000Z&date=%3C2017-09-14T10%3A15%3A43.000Z&page=1&_sort=-date&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform
Comment 10•7 years ago
|
||
Comment on attachment 8906595 [details] [diff] [review] Use a fallible allocator for LoopAliasInfo allocations. 56 is on m-r now.
Attachment #8906595 -
Flags: approval-mozilla-beta? → approval-mozilla-release?
Comment 11•7 years ago
|
||
If this isn't a high crash rate, I'd like this to wait and ride with 57 since we're building the 56 release candidate now. If the point is that this crash might never get reported correctly and might be widespread please n-i to me to explain or find me on irc and we can reconsider. Thanks
Flags: needinfo?(nicolas.b.pierron)
Updated•7 years ago
|
Attachment #8906595 -
Flags: approval-mozilla-release? → approval-mozilla-release-
Assignee | ||
Comment 12•7 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #11) > If this isn't a high crash rate, I'd like this to wait and ride with 57 > since we're building the 56 release candidate now. This is fine for me. > If the point is that this crash might never get reported correctly and might > be widespread please n-i to me to explain or find me on irc and we can > reconsider. Thanks Crash-stat should report these OOM correctly. There is just no volume in the last 6 months. Our debug builds are deliberately changed to be picky about these assertions to ensure the correctness of the allocator usage.
Flags: needinfo?(nicolas.b.pierron)
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•