ForkJoinNursery crashes on OOM when it could signal the error properly

RESOLVED FIXED in mozilla33

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: lth, Assigned: lth)

Tracking

unspecified
mozilla33
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The ForkJoinNursery calls CrashAtUnhandlableOOM() in its constructor and within setCurrentChunk(), when trying to allocate memory for the the nursery during initialization or when growing the nursery.  In these cases it should return an error indicator - ie the allocations should be fallible - and the caller should propagate the error.

In practice this means also that the initialization of ForkJoinContext needs to be split in two, with an explicit initialize method that initializes the ForkJoinNursery and returns a status indicator.
(Assignee)

Comment 1

4 years ago
Created attachment 8443429 [details] [diff] [review]
Patch

Not much excitement here apart from how to handle failure to expand the nursery, which I hope I have commented adequately.
Attachment #8443429 - Flags: review?(shu)

Comment 3

4 years ago
Comment on attachment 8443429 [details] [diff] [review]
Patch

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

LGTM.

::: js/src/gc/ForkJoinNursery.cpp
@@ +767,5 @@
>          // budget is used up.  There is a guard in
>          // Chunk::allocateArena() against the latter case.
>          return tenured_->arenas.allocateFromArena(evacuationZone_, thingKind);
>      } else {
> +        return allocateInTospaceInfallible(thingSize);

Didn't see this last time. Preexisting style nit: no else-after-return.

::: js/src/gc/ForkJoinNursery.h
@@ +221,5 @@
>      // Walk the list of registered slot arrays and free them all.
>      void sweepHugeSlots();
>  
>      // Set the position/end pointers to correspond to the numbered
> +    // chunk.  Returns fals if the chunk could not be allocated, either

Typo 'fals'
Attachment #8443429 - Flags: review?(shu) → review+
https://hg.mozilla.org/mozilla-central/rev/1b9f82ca8547
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.