1203814 - Adding a testing function causes intermittent orange in oomInFormatStackDump.js on Win32 debug SM(p).

Status: RESOLVED FIXED

Description

[Tooru Fujisawa [:arai]]

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

Comment 1

[Tooru Fujisawa [:arai]]

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.

Comment 2

[Jon Coppeard (:jonco) (Away until 25th Feb)]

Created attachment 8659829 [details] [diff] [review]
bug1203814-oomTest

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.

Comment 3

[Jon Coppeard (:jonco) (Away until 25th Feb)]

Try build looking good so far:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7084d80e7e41

Comment 4

[Jon Coppeard (:jonco) (Away until 25th Feb)]

Comment on attachment 8659829 [details] [diff] [review]
bug1203814-oomTest

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

r=bbouvier via IRC.

Comment 5

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b481753ea71f
It now passed try run with my patches :)
thanks!

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7441b33a26e7

Comment 7

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/7441b33a26e7