Closed Bug 626547 Opened 9 years ago Closed 9 years ago

Ensure FinishJIT is called if InitJIT fails

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 628332

People

(Reporter: njn, Assigned: njn)

References

Details

InitJIT is called from JSCompartment::init().  It can fail, in which case FinishJIT should be called to ensure any memory allocated is freed.

For the init() call in jsgc.cpp this will happen because if init() fails the JSCompartment is deleted immediately.

For the init() call in jsapi.cpp this won't happen.  Either the compartment should be js_delete'd if init() fails, or we should change InitJIT() to call FinishJIT() itself on failure.
pbiggar, I bet we have a bunch of leaks after OOMs like this one.  Bug 642094 combined with Valgrind should help identify them!
(In reply to comment #0)
> For the init() call in jsapi.cpp this won't happen.  Either the compartment
> should be js_delete'd if init() fails, or we should change InitJIT() to call
> FinishJIT() itself on failure.

The latter. The original C code used these names and rules:

New : Destroy :: Init : Finish

if during a New, something went wrong, the calloc'ed or memset(0)'ed data structure was Destroyed before null was returned to signal the error. Destroy was infallible.

Some data structures had exposed Init and Finish functions as well as New and Destroy, to support both heap and embedded-in-larger-allocation-or-on-stack storage. The rule was Init was called exactly once before the struct was made available to clients, and if it failed, it Finish'ed. Again default-zero-init helped. Finish is idempotent (js_FinishAtomState is an example from the old C codebase).

/be
(In reply to comment #1)
> pbiggar, I bet we have a bunch of leaks after OOMs like this one.  Bug 642094
> combined with Valgrind should help identify them!

njn means bug 624094. Yes, I use valgrind extensively in that bug now (though I perhaps haven't updated the patch).
Depends on: 628332
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 628332
(In reply to comment #3)
>
> njn means bug 624094.

Sigh, I'm getting dyslexic in my old age.
You need to log in before you can comment on or make changes to this bug.