Closed Bug 815999 Opened 13 years ago Closed 13 years ago

Intermittent Assertion failure: !dst->compartment()->isCollecting() in browser_dbg_chrome-debugging.js [@ EndMarkingCompartmentGroup] (now [@ ShouldMarkCrossCompartment])

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla20

People

(Reporter: peterv, Assigned: jonco)

References

Details

(Keywords: assertion, intermittent-failure)

Crash Data

Attachments

(3 files, 2 obsolete files)

Better potential fix
6.80 KB, patch
billm
: review+
Details | Diff | Splinter Review
Patch to assert if a wrapper to a gray thing is created
5.43 KB, patch
billm
: review+
Details | Diff | Splinter Review
fix for black-to-gray edge from debugger, v2
6.06 KB, patch
mccr8
: review+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=17399462&tree=Mozilla-Inbound Rev5 MacOSX Mountain Lion 10.8 mozilla-inbound debug test mochitest-browser-chrome on 2012-11-27 21:41:59 PST for push 6f0c997ba776 slave: talos-mtnlion-r5-077 TEST-START | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js ++DOCSHELL 0x158da7e60 == 13 [id = 2758] ++DOMWINDOW == 54 (0x1281cba08) [serial = 7287] [outer = 0x0] ++DOMWINDOW == 55 (0x158d872a8) [serial = 7288] [outer = 0x1281cb980] WARNING: NS_ENSURE_TRUE(mMutable) failed: file ../../../../netwerk/base/src/nsSimpleURI.cpp, line 272 WARNING: NS_ENSURE_TRUE(mMutable) failed: file ../../../../netwerk/base/src/nsSimpleURI.cpp, line 272 WARNING: NS_ENSURE_TRUE(frame) failed: file ../../../layout/base/nsPresContext.cpp, line 1179 WARNING: NS_ENSURE_TRUE(frame) failed: file ../../../layout/base/nsPresContext.cpp, line 1179 WARNING: NS_ENSURE_TRUE(frame) failed: file ../../../layout/base/nsPresContext.cpp, line 1179 ++DOMWINDOW == 56 (0x150387478) [serial = 7289] [outer = 0x1281cb980] WARNING: Unable to test style tree integrity -- no content node: file ../../../layout/base/nsCSSFrameConstructor.cpp, line 8267 TEST-PASS | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js | Found a chrome debugging actor. Assertion failure: !dst->compartment()->isCollecting(), at ../../../js/src/jsgc.cpp:4010 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js | Exited with code 1 during test run INFO | automation.py | Application ran for: 0:21:54.527508 INFO | automation.py | Reading PID log: /var/folders/7c/fqxs206n3dlcgjphjlbqjk6000000w/T/tmpnKkKJTpidlog Downloading symbols from: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-macosx64-debug/1354079312/firefox-20.0a1.en-US.mac64.crashreporter-symbols.zip PROCESS-CRASH | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js | application crashed (minidump found) Crash dump filename: /var/folders/7c/fqxs206n3dlcgjphjlbqjk6000000w/T/tmpCIIQ8L/minidumps/D79E9359-EC94-4171-8DAC-1F1899DEDFB4.dmp Operating system: Mac OS X 10.8.0 12A269 CPU: amd64 family 6 model 42 stepping 7 8 CPUs Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS Crash address: 0x0 Thread 0 (crashed) 0 XUL!EndMarkingCompartmentGroup [jsgcinlines.h : 460 + 0x0] rbx = 0x00007fff7eb98c68 r12 = 0x000000016d272800 r13 = 0x00007fff5fbf65c0 r14 = 0x0000000104c8a000 r15 = 0x00007fff5fbf65b8 rip = 0x000000010302f64e rsp = 0x00007fff5fbf6580 rbp = 0x00007fff5fbfa640 Found by: given as instruction pointer in context 1 XUL!IncrementalCollectSlice [jsgc.cpp : 4206 + 0x7] rbx = 0x00007fff5fbfa758 r12 = 0x0000000104c8a000 r13 = 0x0000000103884be0 r14 = 0x00007fff5fbfa690 r15 = 0x0000000104c8a000 rip = 0x000000010302c414 rsp = 0x00007fff5fbfa650 rbp = 0x00007fff5fbfa7d0 Found by: call frame info 2 XUL!ResetIncrementalGC [jsgc.cpp : 4464 + 0x10] rbx = 0x0000000104c8a000 r12 = 0x0000000000000000 r13 = 0x0000000000000000 r14 = 0x000000010369b4f6 r15 = 0x0000000104c8a000 rip = 0x000000010302b9a5 rsp = 0x00007fff5fbfa7e0 rbp = 0x00007fff5fbfa820 Found by: call frame info 3 XUL!GCCycle [jsgc.cpp : 4809 + 0x11] rbx = 0x0000000104c8a000 r12 = 0x0000000000000000 r13 = 0x0000000000000000 r14 = 0x000000010369b4f6 r15 = 0x0000000104c8a000 rip = 0x000000010302b68a rsp = 0x00007fff5fbfa830 rbp = 0x00007fff5fbfa870 Found by: call frame info 4 XUL!Collect [jsgc.cpp : 4931 + 0x14] rbx = 0x0000000104c8a000 r12 = 0x0000000000000000 r13 = 0x0000000104c8a3f8 r14 = 0x0000000000000008 r15 = 0x0000000000000000 rip = 0x000000010302820c rsp = 0x00007fff5fbfa880 rbp = 0x00007fff5fbfa8c0 Found by: call frame info 5 XUL!JSCompartment::addDebuggee(JSContext*, js::GlobalObject*) [jscompartment.h : 523 + 0xf] rbx = 0x000000004c320aac r12 = 0x0000000100841e00 r13 = 0x0000000000000000 r14 = 0x0000000100174f90 r15 = 0x0000000100842501 rip = 0x0000000102ff449a rsp = 0x00007fff5fbfa8d0 rbp = 0x00007fff5fbfa930 Found by: call frame info 6 XUL!js::Debugger::addDebuggeeGlobal(JSContext*, JS::Handle<js::GlobalObject*>) [Debugger.cpp : 2089 + 0xe] rbx = 0x00007fff5fbfaa40 r12 = 0x0000000100174f90 r13 = 0x00007fff5fbfaa40 r14 = 0x000000016e999360 r15 = 0x00000001503505d0 rip = 0x00000001031a3599 rsp = 0x00007fff5fbfa940 rbp = 0x00007fff5fbfaa20 Found by: call frame info 7 XUL!js::Debugger::addDebuggee(JSContext*, unsigned int, JS::Value*) [Debugger.cpp : 1878 + 0xa] rbx = 0x00000001503505d0 r12 = 0x0000000100174f90 r13 = 0x00007fff5fbfaab8 r14 = 0x000000010cc97310 r15 = 0x0000000000000000 rip = 0x00000001031a2f47 rsp = 0x00007fff5fbfaa30 rbp = 0x00007fff5fbfaa90 Found by: call frame info 8 XUL!js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [jscntxtinlines.h : 364 + 0x5] rbx = 0x000000010cc97318 r12 = 0x00007fff5fbfabd0 r13 = 0x00007fff5fbfaab8 r14 = 0x0000000100174f90 r15 = 0x00000001031a2ec0 rip = 0x000000010306ee1c rsp = 0x00007fff5fbfaaa0 rbp = 0x00007fff5fbfaaf0 Found by: call frame info
Summary: Intermittent Assertion failure: !thing->compartment()->scheduledForDestruction in browser_dbg_chrome-debugging.js [@ EndMarkingCompartmentGroup] → Intermittent Assertion failure: dst->compartment()->isCollecting() in browser_dbg_chrome-debugging.js [@ EndMarkingCompartmentGroup]
Summary: Intermittent Assertion failure: dst->compartment()->isCollecting() in browser_dbg_chrome-debugging.js [@ EndMarkingCompartmentGroup] → Intermittent Assertion failure: !dst->compartment()->isCollecting() in browser_dbg_chrome-debugging.js [@ EndMarkingCompartmentGroup]
Assignee: general → jcoppeard
Status: NEW → ASSIGNED
Attached patch Possible fix (obsolete) — — Splinter Review
Change is to always sweep debugger and debugee objects in the same group, as I think was the original intention.
The original assertion assumes that a compartment edge from A -> B means that A will not finish marking after B. However, the order is calculated at the start of the sweep phase, and new wrappers can be created after this point which break this assumption. The fix is to update the assertion to ignore wrappers created during collection. This necessitates moving the finding of black->gray into the marking code, because the areana's allocatedDuringIncremental flag is cleared after the arena has been marked. This also means that it's no longer necessary to iterate though every wrapper to check this. Also included is the previous patch that ensures debugger and debuggee objects are always finalized in the same group.
Attachment #686133 - Attachment is obsolete: true
Attachment #686562 - Flags: review?(wmccloskey)
As discussed with billm, this is a quick patch to disallow
Attachment #686652 - Flags: review?(wmccloskey)
... to disallow creation of wrappers to gray objects, to see if we can get to the root of this problem. We could probably do with factoring out all calls to JSCompartment::crossCompartmentWrappers.put() but I haven't done that yet.
Comment on attachment 686652 [details] [diff] [review] Patch to assert if a wrapper to a gray thing is created I can check this in for you later today, once the try build finishes.
Attachment #686652 - Flags: review?(wmccloskey) → review+
The try build is pretty revealing. I think that findAllGlobals and RecomputeWrappers need to call UnmarkGray. That means we'll have to move it to the JS engine. I'll work on that tomorrow. That should be enough to fix this problem, I suspect.
Comment on attachment 686562 [details] [diff] [review] Better potential fix Review of attachment 686562 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Marking.cpp @@ +603,5 @@ > + * compartment that has already been marked gray. > + */ > + if (cell->isMarked(GRAY)) { > + JS_ASSERT_IF(!src->arenaHeader()->allocatedDuringIncremental, > + !cell->compartment()->isCollecting()); Please remove the allocatedDuringIncremental condition.
Attachment #686562 - Flags: review?(wmccloskey) → review+
Whiteboard: [leave open]
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc50a2127738 Not a fix, just a couple of improvements: - sweep debugger and debugee objects in same group - improve black->gray pointer detection
This caused crashes in some of the unit tests, so I backed it out. https://hg.mozilla.org/integration/mozilla-inbound/rev/70e354775e1b
(In reply to Ehsan Akhgari [:ehsan] from comment #39) > This caused crashes in some of the unit tests, so I backed it out. Thanks. Can you point me at the tests that crashed?
I pushed this to try again and got a green build, so I'm re-landing it: https://tbpl.mozilla.org/?tree=Try&rev=0bad9e4bc457 I believe there was another GC-related patch that later got reverted that could have caused the failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/c7f71ff116c3
(In reply to comment #45) > (In reply to Ehsan Akhgari [:ehsan] from comment #39) > > This caused crashes in some of the unit tests, so I backed it out. > > Thanks. Can you point me at the tests that crashed? Sorry I didn't put the links in the bug. FWIW you can go to the TBPL page for the inbound tree, and click the green "next" button at the end of the page enough number of times to get to your push the next time.
The problem here is that Debugger::findAllGlobals plucks global objects directly out of the compartment. There's no assurance that these globals are marked black--they could be gray. Then it exposes them to JS code.
Attachment #688062 - Flags: review?(continuation)
Forgot qref.
Attachment #688062 - Attachment is obsolete: true
Attachment #688062 - Flags: review?(continuation)
Attachment #688063 - Flags: review?(continuation)
Comment on attachment 688063 [details] [diff] [review] fix for black-to-gray edge from debugger, v2 Review of attachment 688063 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/public/HeapAPI.h @@ +78,5 @@ > + * engine (by handing it to running JS code or writing it into live JS > + * data). During incremental GC, since the gray bits haven't been computed yet, > + * we conservatively mark the object black. > + */ > +static inline void This should be MOZ_ALWAYS_INLINE. @@ +79,5 @@ > + * data). During incremental GC, since the gray bits haven't been computed yet, > + * we conservatively mark the object black. > + */ > +static inline void > +ExposeGCThingToJS(void *thing, JSGCTraceKind kind) This name is slightly weird. Aren't JS objects always exposed to JS? I mean, I know what you mean, but maybe something like ExposeGCThingToLiveJS? ToActiveJS? etc. Well, "live" isn't that great either, given that gray JS is still alive... ::: js/src/jsfriendapi.cpp @@ +893,5 @@ > return IsIncrementalBarrierNeeded(cx->runtime); > } > > JS_FRIEND_API(bool) > +js::IsIncrementalBarrierNeededOnGCThing(void *thing, JSGCTraceKind kind) Why do you have this |kind| argument you never use? ::: js/src/jsfriendapi.h @@ -11,5 @@ > #include "jscpucfg.h" > #include "jspubtd.h" > #include "jsprvtd.h" > > -#include "js/HeapAPI.h" I hope you checked that this compiles. ;)
Attachment #688063 - Flags: review?(continuation) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/97bdf3a66177 I've been trying to pass around the thing kind more often just because we'll probably need it for generational collection.
Crash Signature: [@ EndMarkingCompartmentGroup] → [@ EndMarkingCompartmentGroup] [@ ShouldMarkCrossCompartment]
Summary: Intermittent Assertion failure: !dst->compartment()->isCollecting() in browser_dbg_chrome-debugging.js [@ EndMarkingCompartmentGroup] → Intermittent Assertion failure: !dst->compartment()->isCollecting() in browser_dbg_chrome-debugging.js [@ EndMarkingCompartmentGroup] (now [@ ShouldMarkCrossCompartment])
Blocks: 690970
This looks fixed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: