Closed
Bug 1272908
Opened 8 years ago
Closed 8 years ago
Assertion failure: !cx->isExceptionPending(), at js/src/vm/Debugger.cpp:1866
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla50
People
(Reporter: gkw, Unassigned)
References
Details
(Keywords: assertion, testcase, Whiteboard: [jsbugmon:update])
Attachments
(3 files)
The following testcase crashes on mozilla-central revision c3f5e6079284 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion): // Adapted from randomly chosen test: js/src/jit-test/tests/modules/bug-1233915.js g = newGlobal(); g.parent = this; g.eval("(" + function() { Debugger(parent).onExceptionUnwind = function(frame) frame.eval("") } + ")()"); // Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1254123.js function ERROR(msg) { throw new Error("boom"); } var dbg = new Debugger; dbg.onNewGlobalObject = ERROR; oomTest(function() { newGlobal(); }) Backtrace: 0 js-dbg-64-dm-clang-darwin-c3f5e6079284 0x000000010d4d441b js::Debugger::fireNewGlobalObject(JSContext*, JS::Handle<js::GlobalObject*>, JS::MutableHandle<JS::Value>) + 923 (Debugger.cpp:1866) 1 js-dbg-64-dm-clang-darwin-c3f5e6079284 0x000000010d4d46d1 js::Debugger::slowPathOnNewGlobalObject(JSContext*, JS::Handle<js::GlobalObject*>) + 609 (Debugger.cpp:1915) 2 js-dbg-64-dm-clang-darwin-c3f5e6079284 0x000000010d2ce561 JS_FireOnNewGlobalObject(JSContext*, JS::Handle<JSObject*>) + 97 (RootingAPI.h:667) 3 js-dbg-64-dm-clang-darwin-c3f5e6079284 0x000000010cd8eb39 NewGlobalObject(JSContext*, JS::CompartmentOptions&, JSPrincipals*) + 1065 (js.cpp:6623) 4 js-dbg-64-dm-clang-darwin-c3f5e6079284 0x000000010cd9a131 NewGlobal(JSContext*, unsigned int, JS::Value*) + 993 (RootingAPI.h:661) For detailed crash information, see attachment.
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Comment 2•8 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/9e97e2282142 parent: 295477:1d4795f0b4e2 user: Terrence Cole date: Mon Apr 25 15:42:19 2016 -0700 summary: Bug 1267412 - Use MutableHandleValue instead of pointer-to-AutoValueVector; r=sfink Terrence, is bug 1267412 a likely regressor?
Blocks: 1267412
Flags: needinfo?(terrence)
Reporter | ||
Comment 3•8 years ago
|
||
Reporter | ||
Comment 4•8 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/038ba23ecf52 user: Eric Faust date: Tue May 03 03:28:31 2016 -0700 summary: Bug 1254190 - Propagate failure of matchAllDebuggeeGlobals() in Debugger. (r=shu) A separate bisection for an earlier unreduced version of the testcase points to bug 1254190 instead. Not sure which one is correct.
Flags: needinfo?(efaustbmo)
Comment 5•8 years ago
|
||
More OOM fixes. Wheeeeeee.
Comment 6•8 years ago
|
||
Comment on attachment 8754161 [details] [diff] [review] Fix Review of attachment 8754161 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/ScopeObject.cpp @@ +2736,5 @@ > * but it simplifies later indexing logic. > */ > Rooted<GCVector<Value>> vec(cx, GCVector<Value>(cx)); > + if (!frame.copyRawFrameSlots(&vec)) { > + cx->recoverFromOutOfMemory(); Policy question. Is recoverFromOutOfMemory for things that we shouldn't care about OOMing? I can't really get in my head when this would not adversely interact with other parts of the system.
Attachment #8754161 -
Flags: review?(shu) → review+
Updated•8 years ago
|
Flags: needinfo?(jorendorff)
Pushed by efaustbmo@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/30406bf293ec Fix OOM in DebugScopes::onPopCall(). (r=shu)
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/30406bf293ec
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Comment 9•8 years ago
|
||
(In reply to Shu-yu Guo [:shu] from comment #6) > > + cx->recoverFromOutOfMemory(); > > Policy question. Is recoverFromOutOfMemory for things that we shouldn't care > about OOMing? I can't really get in my head when this would not adversely > interact with other parts of the system. That is not so far-fetched: all OOM returns are supposed to leave the universe in a good state. If you can't, you must oom-crash. So whenever you are trying something that doesn't really need to succeed, it's OK to call this method and forget that OOM happened. It so happens that the JS engine is full of "fast paths" that don't really need to succeed, because there's a corresponding slow path handy. People tend to use recoverFromOutOfMemory when hacking on a function that's currently infallible. They hack in some optimization that needs to allocate memory. If the allocation fails, then what? Maybe you should make the function fallible and change all its call sites. But you could also just recover from OOM, give up on doing that optimization, and move on. It's not so bad.
Flags: needinfo?(jorendorff)
You need to log in
before you can comment on or make changes to this bug.
Description
•