Closed Bug 626526 Opened 9 years ago Closed 9 years ago

OOM error during JS_NewContext (jscntxt.cpp:1110)

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: paul.biggar, Assigned: paul.biggar)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files)

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
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.
Assignee: general → pbiggar
Status: NEW → ASSIGNED
Attachment #505656 - Flags: review?
Attachment #505656 - Flags: review? → review?(lw)
Attached patch un-hackingSplinter Review
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?
Attachment #505685 - Flags: review?(pbiggar)
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.
Attachment #505685 - Flags: review?(pbiggar) → review+
Regression from patch for bug 515812, right?

/be
Blocks: 515812
Yes.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #505656 - Flags: review?(lw)
You need to log in before you can comment on or make changes to this bug.