626526 - OOM error during JS_NewContext (jscntxt.cpp:1110)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Paul Biggar]

An allocation failure at allocation 28/211 in ../jit-test/tests/arguments/args-createontrace.js causes problems (detected using bug 624094)

Command (from obj directory, using patch from bug 624094):
    shell/js -A 28 -m -j -p -e "const platform='darwin'; const libdir='../jit-test/lib/';" -f ../jit-test/lib/prolog.js -f ../jit-test/tests/arguments/args-createontrace.js

stdout, stderr, exitcode: ('', 'Assertion failure: !cx->thread, at ../jscntxt.cpp:1110\n', -11)

Diagnosis: 
  - segfault (probably due to lack of OOM checking)

Stack trace (from valgrind):
 Invalid write of size 4
at: JS_Assert (jsutil.cpp:87)
by: FreeContext(JSContext*) (jscntxt.cpp:1110)
by: js_NewContext(JSRuntime*, unsigned long) (jscntxt.cpp:846)
by: JS_NewContext (jsapi.cpp:983)
by: NewContext(JSRuntime*) (js.cpp:5309)
by: main (js.cpp:5559)
  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Comment 1

[Paul Biggar]

Attachment: Deallocate thread after OOM

If cx->busyArrays.init() OOMs, FreeContext expects that cx->thread is clear. It looks to me that this is the way to free it, though it looks a bit ugly.

Comment 2

[Luke Wagner [:luke]]

Attachment: un-hacking

Hey Paul, thanks for finding this and writing a patch.  I wrote this back when I was more superstitious concerning alloc policies.  Now that I see this code again, I think the whole "letting the cx get constructed enough to use the ContextAllocPolicy" hack can be removed by just using the SystemAllocPolicy.  Sound good to you?

Comment 3

[Paul Biggar]

Comment on attachment 505685 [details] [diff] [review]
un-hacking

I've confirmed this removed the error and leaks less memory than my patch. Cleaner too.

Comment 4

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/9de332a8e330

Comment 5

[Brendan Eich [:brendan]]

Regression from patch for bug 515812, right?

/be

Comment 6

[Luke Wagner [:luke]]

Yes.

Comment 7

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/9de332a8e330