1201057 - Don't simulate OOM in places where we will crash with CrashAtUnhandlableOOM()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

There are many spots in the engine where for one reason or another we crash if we fail to allocate memory by CrashAtUnhandlableOOM().

When running a test which simulates OOM, doing so in these places will cause the engine to exit straight away and abort the test, and we currently have to allow this.  This has the potential to hide errors though as later allocation points are not tested.

We already have AutoEnterOOMUnsafeRegion and we can use this to disable OOM simulation where necessary.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: move-oom-guard

Move the definition of AutoEnterOOMUnsafeRegion to Utility.h where all the rest of the OOM simulation infrastructure is.

Fix warnings when this is pulled into gecko that revealed bad implicit type conversions between 64 and 32 bits.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: use-oom-unsafe-region

Use AutoEnterOOMUnsafeRegion to disable OOM simulation where we will just crash anyway.

I tried to keep the scope of the guard as small as reasonably possible but there are a couple of places where it made more sense to have it live for a longer time, notably JSObject::swap() and LoopUnroller::go().

This removes almost all the direct calls to CrashAtUnhandlableOOM().  There are a few remaining uses which could do with further investigation, but these were more complicated than a simple OOM:
 - jit::ExceptionHandlerBailout()
 - SnapshotIterator::maybeRead()
 - BOffImm::BOffImm()

Comment 3

[Jon Coppeard (:jonco)]

Attachment: update-oom-tests

Update OOM tests to not specify --allow-unhandlable-oom now we don't expect to hit this.  Ditto for --no-ggc, which was there because GGC hits an unhandlable OOM as soon as we do a minor GC.  Add --no-threads because none of this infrastructure is thread safe yet and we want reliable test results.

Comment 4

[Terrence Cole [:terrence]]

Comment on attachment 8656054 [details] [diff] [review]
use-oom-unsafe-region

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

This is very nice! Sadly, I just checked in a couple more CrashAtUnhandlableOOM with the stable hash codes patch -- sorry about that. :-(

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d051e096106
https://hg.mozilla.org/integration/mozilla-inbound/rev/40fae3130e1a
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab53cb21bbbe

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1d051e096106
https://hg.mozilla.org/mozilla-central/rev/40fae3130e1a
https://hg.mozilla.org/mozilla-central/rev/ab53cb21bbbe