Closed
Bug 1144356
Opened 10 years ago
Closed 9 years ago
Crash [@ js::Debugger::fireOnGarbageCollectionHook] or Assertion failure: cx, at vm/Debugger.cpp or Assertion failure: rt->contextList.getFirst() == rt->contextList.getLast(), at jsfriendapi.cpp
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox39 | --- | affected |
People
(Reporter: gkw, Assigned: fitzgen)
References
Details
(Keywords: assertion, regression, testcase)
Crash Data
Attachments
(2 files, 1 obsolete file)
// Randomly chosen test: js/src/jit-test/tests/debug/Memory-onGarbageCollection-03.js
dbg = Debugger();
t = dbg.addDebuggee(newGlobal())
dbg.memory.onGarbageCollection = _ => x
asserts js debug shell on m-c changeset e965a1a534ec with --fuzzing-safe --no-threads --no-ion -D at Assertion failure: cx, at vm/Debugger.cpp.
Configure options:
CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-profiling --disable-threadsafe --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-profiling --enable-more-deterministic" -r e965a1a534ec
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/578ba1506156
user: Nick Fitzgerald
date: Fri Mar 13 13:03:00 2015 +0100
summary: Bug 1137844 - Part 3: Fire the Debugger.Memory.prototype.onGarbageCollection hook after GCs; r=sfink
Nick, is bug 1137844 a likely regressor?
Flags: needinfo?(nfitzgerald)
Reporter | ||
Comment 1•10 years ago
|
||
(lldb) bt 5
* thread #1: tid = 0x143be, 0x00000001001f08f1 js-dbg-64-prof-dm-darwin-e965a1a534ec`js::Debugger::fireOnGarbageCollectionHook(this=<unavailable>, rt=<unavailable>, stats=<unavailable>) + 785 at Debugger.cpp:1335, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
* frame #0: 0x00000001001f08f1 js-dbg-64-prof-dm-darwin-e965a1a534ec`js::Debugger::fireOnGarbageCollectionHook(this=<unavailable>, rt=<unavailable>, stats=<unavailable>) + 785 at Debugger.cpp:1335
frame #1: 0x00000001003c49a9 js-dbg-64-prof-dm-darwin-e965a1a534ec`js::gcstats::Statistics::endGC() [inlined] js::Debugger::onGarbageCollection(rt=<unavailable>, stats=<unavailable>) + 68 at Debugger.h:991
frame #2: 0x00000001003c4965 js-dbg-64-prof-dm-darwin-e965a1a534ec`js::gcstats::Statistics::endGC(this=0x0000000102025ed0) + 1621 at Statistics.cpp:977
frame #3: 0x00000001003c4e23 js-dbg-64-prof-dm-darwin-e965a1a534ec`js::gcstats::Statistics::endSlice(this=<unavailable>) + 195 at Statistics.cpp:1029
frame #4: 0x00000001007f6fa0 js-dbg-64-prof-dm-darwin-e965a1a534ec`js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::gcreason::Reason) [inlined] js::gcstats::AutoGCSlice::~AutoGCSlice() + 12 at Statistics.h:329
(lldb)
Assignee | ||
Comment 2•10 years ago
|
||
Yeah that's the regressor.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Flags: needinfo?(nfitzgerald)
Assignee | ||
Comment 3•10 years ago
|
||
https://dxr.mozilla.org/mozilla-central/source/js/src/vm/Debugger.cpp?from=fireOnGarbageCollectionHook#1335
So it seems that we can't get a cx from the rt? Should we just return early?
:sfink, what do you think is the correct thing to do here?
Flags: needinfo?(sphink)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8578937 -
Flags: review?(sphink)
Comment 5•10 years ago
|
||
Derp.
This all seems pretty crazy. DefaultJSContext is only meaningful with respect to a particular embedding, and I see no reason why it is a reasonable JSContext to use in the first place. If you left an exception pending on it, I have no idea what would happen. Except it looks like it handles that by calling handleUncaughtException, so that particular problem is papered over. But what if the context *already* had an exception pending? Maybe even one for which creating an Error object is what triggered the garbage collection in the first place? This just seems weird and dangerous.
On the other hand, JSContexts don't mean very much these days.
It would probably make the most sense to me to create a JSContext specifically for Debugger's use (shared across the runtime, because why not.) But I don't feel like I have a firm enough grasp on things to stand behind that recommendation, so I'm going to needinfo an order of magnitude more brain cells than I possess myself.
Flags: needinfo?(sphink)
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorendorff)
Reporter | ||
Comment 6•10 years ago
|
||
This comment 0 testcase also crashes js opt shell at js::Debugger::fireOnGarbageCollectionHook
Crash Signature: [@ js::Debugger::fireOnGarbageCollectionHook]
Summary: Assertion failure: cx, at vm/Debugger.cpp → Crash [@ js::Debugger::fireOnGarbageCollectionHook] or Assertion failure: cx, at vm/Debugger.cpp
Assignee | ||
Comment 7•10 years ago
|
||
More related issues (from bug 1137527 comment 15):
JS::ubi::RootList (used to initialize a census traversal) calls JS_TraceRuntime,
which evicts the nursery,
which calls the onGarbageCollection hook,
which runs JS that allocates new objects in the nursery,
which causes assertions in the runtime tracing code.
------------------------------------------------------------
Is there some time/place we can call this hook that avoids the above problems and has easy access to a cx and the gc stats?
It seems to me that the existing stringified JSON hook avoids these issues by posting a runnable to run later to gecko's event loop? Should the Debugger do something similar (with a lot of SM<-->Gecko plumbing)?
Flags: needinfo?(terrence)
Comment 8•10 years ago
|
||
Yes, you pretty much have to do that way, sadly. It would be really nice if we had our own event loop, but we don't yet.
Flags: needinfo?(terrence)
Assignee | ||
Updated•10 years ago
|
Attachment #8578937 -
Attachment is obsolete: true
Attachment #8578937 -
Flags: review?(sphink)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorendorff)
Reporter | ||
Comment 9•10 years ago
|
||
// Randomly chosen test: js/src/jit-test/tests/debug/Memory-onGarbageCollection-04.js
evaluate("\
g = newGlobal();\
dbg = Debugger(g);\
dbg.memory.onGarbageCollection = function(){};\
g.eval(\"gc()\");\
", {
newContext: true,
})
is a similar testcase that asserts js debug shell on m-c rev 2e2244031032 with --fuzzing-safe --no-threads --no-ion at Assertion failure: rt->contextList.getFirst() == rt->contextList.getLast(), at jsfriendapi.cpp
Summary: Crash [@ js::Debugger::fireOnGarbageCollectionHook] or Assertion failure: cx, at vm/Debugger.cpp → Crash [@ js::Debugger::fireOnGarbageCollectionHook] or Assertion failure: cx, at vm/Debugger.cpp or Assertion failure: rt->contextList.getFirst() == rt->contextList.getLast(), at jsfriendapi.cpp
Reporter | ||
Comment 10•10 years ago
|
||
(lldb) bt 5
* thread #1: tid = 0x3e1fe, 0x00000001007c09df js-dbg-64-dm-nsprBuild-darwin-2e2244031032`js::DefaultJSContext(rt=<unavailable>) + 255 at jsfriendapi.cpp:1134, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
* frame #0: 0x00000001007c09df js-dbg-64-dm-nsprBuild-darwin-2e2244031032`js::DefaultJSContext(rt=<unavailable>) + 255 at jsfriendapi.cpp:1134
frame #1: 0x00000001001ead89 js-dbg-64-dm-nsprBuild-darwin-2e2244031032`js::Debugger::fireOnGarbageCollectionHook(this=0x0000000102884800, rt=<unavailable>, stats=0x00000001028676d0) + 89 at Debugger.cpp:1334
frame #2: 0x00000001003b06f9 js-dbg-64-dm-nsprBuild-darwin-2e2244031032`js::gcstats::Statistics::endGC() [inlined] js::Debugger::onGarbageCollection(rt=<unavailable>, stats=<unavailable>) + 81 at Debugger.h:991
frame #3: 0x00000001003b06a8 js-dbg-64-dm-nsprBuild-darwin-2e2244031032`js::gcstats::Statistics::endGC(this=0x00000001028676d0) + 1400 at Statistics.cpp:978
frame #4: 0x00000001003b0b73 js-dbg-64-dm-nsprBuild-darwin-2e2244031032`js::gcstats::Statistics::endSlice(this=<unavailable>) + 195 at Statistics.cpp:1030
(lldb)
Assignee | ||
Comment 11•9 years ago
|
||
Fixed in bug 1150253
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•