Closed
Bug 626547
Opened 10 years ago
Closed 10 years ago
Ensure FinishJIT is called if InitJIT fails
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 628332
People
(Reporter: n.nethercote, Assigned: n.nethercote)
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.
![]() |
Assignee | |
Comment 1•10 years ago
|
||
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•10 years ago
|
||
(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•10 years ago
|
||
(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
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 628332
![]() |
Assignee | |
Comment 5•10 years ago
|
||
(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.
Description
•