Fix "Assertion failure: gGotError, at js/src/shell/js.cpp:448" after OOM in the shell

RESOLVED FIXED in mozilla31

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
It is surprisingly hard to get this to trip using oomAfterAllocations, because that usually crashes. But for example:

$OBJDIR/dist/bin/js  -e 'const libdir = "js/src/jit-test/lib/";' -f js/src/jit-test/lib/prolog.js -e 'oomAfterAllocations(5301);' -f js/src/jit-test/tests/ion/lookupswitch.js

You may have to try several different numbers in place of 5301.
(Assignee)

Comment 1

3 years ago
Created attachment 8400336 [details] [diff] [review]
Part 1 - Call js_ReportOutOfMemory when deliberately triggering an OOM for testing

I have no idea what the performance consequences of this will be. If the compiler is smart, none.
Assignee: general → jorendorff
Attachment #8400336 - Flags: review?(terrence)
(Assignee)

Comment 2

3 years ago
Created attachment 8400337 [details] [diff] [review]
Part 2 - Add an oomCallback to the shell to make the assertion correct
Attachment #8400337 - Flags: review?(terrence)
Comment on attachment 8400336 [details] [diff] [review]
Part 1 - Call js_ReportOutOfMemory when deliberately triggering an OOM for testing

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

r=me If you get regression warnings after pushing it we can revisit.
Attachment #8400336 - Flags: review?(terrence) → review+
Comment on attachment 8400337 [details] [diff] [review]
Part 2 - Add an oomCallback to the shell to make the assertion correct

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

Great! r=me
Attachment #8400337 - Flags: review?(terrence) → review+
(Assignee)

Updated

3 years ago
Blocks: 912928, 988953
https://hg.mozilla.org/mozilla-central/rev/601c1ba8c437
https://hg.mozilla.org/mozilla-central/rev/05f9f8a561bc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
I can still get this assertion failure rather quickly, even with this patch applied. I assume there are other places where we hit this? See bug 727333 comment 9. I don't know if you want to open a new bug for this or fix it in this bug?
Flags: needinfo?(jorendorff)
(Assignee)

Comment 7

3 years ago
New bug, assigned to me, please.
Flags: needinfo?(jorendorff)
Depends on: 1108155
You need to log in before you can comment on or make changes to this bug.