626547 - Ensure FinishJIT is called if InitJIT fails

Status: RESOLVED DUPLICATE of bug 628332

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [:njn]]

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.

Comment 1

[Nicholas Nethercote [:njn]]

pbiggar, I bet we have a bunch of leaks after OOMs like this one.  Bug 642094 combined with Valgrind should help identify them!

Comment 2

[Brendan Eich [:brendan]]

(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

Comment 3

[Paul Biggar]

(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).

Comment 5

[Nicholas Nethercote [:njn]]

(In reply to comment #3)
>
> njn means bug 624094.

Sigh, I'm getting dyslexic in my old age.