Closed
Bug 1203814
Opened 9 years ago
Closed 9 years ago
Adding a testing function causes intermittent orange in oomInFormatStackDump.js on Win32 debug SM(p).
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: arai, Assigned: jonco)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
Separated from bug 774364.
Bug 774364 adds a testing function, and it causes intermittent orange in oomInFormatStackDump.js on Win32 debug SM(p).
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb3d7a51f85e
The test files are following:
https://dxr.mozilla.org/mozilla-central/source/js/src/jit-test/tests/gc/oomInFormatStackDump.js
https://dxr.mozilla.org/mozilla-central/source/js/src/jit-test/lib/oomTest.js
putting resetOOMFailure into |try-catch| helps, so the OOM happens there.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b156811e08b9
putting resetOOMFailure into |with| makes it worse, it causes a bunch of OOMs.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=22b446a4f72c
Reporter | ||
Comment 1•9 years ago
|
||
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/js/src/builtin/TestingFunctions.cpp#1022
> static bool
> ResetOOMFailure(JSContext* cx, unsigned argc, Value* vp)
> {
> CallArgs args = CallArgsFromVp(argc, vp);
> args.rval().setBoolean(OOM_counter >= OOM_maxAllocations);
> OOM_maxAllocations = UINT32_MAX;
> return true;
> }
ResetOOMFailure itself does nothing that will throw OOM, so it could be happening before/after calling the c++ function itself.
Assignee | ||
Comment 2•9 years ago
|
||
I think the problem is that the oomTest() function can cause an out-of-memory exception to be thrown anywhere between oomAtAllocation() and resetOOMFailure(), but the latter call is outside the try {} block which is supposed to catch these exceptions.
So the fix is to move it inside the block. We still probably want to call this in the catch {} block too though because exceptions can happen for other reasons.
Assignee: nobody → jcoppeard
Attachment #8659829 -
Flags: review?(jdemooij)
Assignee | ||
Comment 3•9 years ago
|
||
Try build looking good so far:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7084d80e7e41
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8659829 [details] [diff] [review]
bug1203814-oomTest
Review of attachment 8659829 [details] [diff] [review]:
-----------------------------------------------------------------
r=bbouvier via IRC.
Attachment #8659829 -
Flags: review?(jdemooij) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 5•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b481753ea71f
It now passed try run with my patches :)
thanks!
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•