Closed
Bug 1239369
Opened 9 years ago
Closed 9 years ago
Restore oomTest's check that an is exception thrown on failure
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
Details
Attachments
(2 files)
(deleted),
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
Bug 1219905 removed oomTest's check that an exception is always thrown on failure because there are some places where this doesn't happen. This is unfortunate because it means we no longer check that ReportOutOfMemory is called correctly.
Instead we can make this an option to oomTest where the default behaviour is to include the check and turn this off in the cases where it doesn't apply. We will also have to disable it when --fuzzing-safe is passed.
Assignee | ||
Comment 1•9 years ago
|
||
As above. Add an argument to oomTest() to indicate whether it should expects an exception to be thrown if execution fails, defaulting to true. Pass false in the one case where this does not happen.
Attachment #8707881 -
Flags: review?(terrence)
Assignee | ||
Comment 2•9 years ago
|
||
Add missing calls to ReportOutOfMemory to make the tests pass again.
Attachment #8707882 -
Flags: review?(terrence)
Updated•9 years ago
|
Attachment #8707882 -
Flags: review?(terrence) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8707881 [details] [diff] [review]
bug1239369-make-oomTest-check-for-exception
Review of attachment 8707881 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/builtin/TestingFunctions.cpp
@@ +3285,5 @@
> " repeatedly executing it and simulating allocation failure at successive\n"
> +" allocations until the function completes without seeing a failure.\n"
> +" By default this tests that an exception is raised if execution fails, but\n"
> +" this can be disabled by passing false as the optional second parameter."
> +),
Probably also worth mentioning that the check is disabled in --fuzzing-safe: it's usually included in the fuzzer's STR and that's when we're going to be most confused when toggling this arg does nothing.
Attachment #8707881 -
Flags: review?(terrence) → review+
Comment 5•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a83f3effa278
https://hg.mozilla.org/mozilla-central/rev/df444117c7be
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment 6•6 years ago
|
||
Steve: It looks like this bug caused a regression in bug 1491326. (This is the only bug in the regression range). Jon is out on PTO till Oct. Can you investigate in the mean time?
Depends on: 1491326
Flags: needinfo?(sdetar)
Comment 7•6 years ago
|
||
I am going to let Jon investigate this when he gets back from PTO in a couple of days.
Updated•6 years ago
|
Flags: needinfo?(sdetar)
You need to log in
before you can comment on or make changes to this bug.
Description
•