Stack Overflow Crash [@ js::gc::Cell::arenaHeader() | js::gc::Cell::address()] | ASSERTION: JSEventListener has wrong script context?: 'stack && NS_SUCCEEDED(stack- >Peek(&cx)) && cx && GetScriptContextFromJSContext(cx) == mContext'

VERIFIED FIXED in Firefox 6

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: bc, Assigned: billm)

Tracking

(Blocks: 1 bug, {assertion, crash})

Trunk
mozilla7
assertion, crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox6- fixed, firefox7+ fixed)

Details

(Whiteboard: fixed-in-tracemonkey, crash signature, URL)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
1. http://www.justin.tv/steven_bonnell_ii/b/286904322
2. Stack Overflow Crash WinXP debug nightly

Associated Socorro Signature UnmarkGrayChildren

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=UnmarkGrayChildren&reason_type=contains&date=05%2F31%2F2011%2003%3A38%3A02&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=UnmarkGrayChildren

which shows mostly windows but one mac.

in vs2005 debugger:

inline uintptr_t
Cell::address() const
{
    uintptr_t addr = uintptr_t(this);
    JS_ASSERT(addr % Cell::CellSize == 0);
=>    JS_ASSERT(Chunk::withinArenasRange(addr));
    return addr;
}

		addr	118147992	unsigned int
		this	0x070acb98	const js::gc::Cell * const

>	mozjs.dll!js::gc::Cell::address()  Line 442 + 0x4 bytes	C++
 	mozjs.dll!js::gc::Cell::arenaHeader()  Line 449 + 0x8 bytes	C++
 	mozjs.dll!js::gc::AssertValidColor(const void * thing=0x070acb98, unsigned int color=1)  Line 508 + 0x8 bytes	C++
 	mozjs.dll!js_GCThingIsMarked(void * thing=0x070acb98, unsigned int color=1)  Line 565 + 0xd bytes	C++
 	xul.dll!xpc_IsGrayGCThing(void * thing=0x070acb98)  Line 155 + 0xc bytes	C++
 	xul.dll!UnmarkGrayChildren(JSTracer * trc=0x0012b564, void * thing=0x070acb98, unsigned int kind=0)  Line 583 + 0x15 bytes	C++
 	mozjs.dll!js::gc::Mark<JSObject>(JSTracer * trc=0x0012b564, JSObject * thing=0x070acb98)  Line 124 + 0x1d bytes	C++
 	mozjs.dll!js::gc::MarkKind(JSTracer * trc=0x0012b564, void * thing=0x070acb98, unsigned int kind=0)  Line 317 + 0xd bytes	C++
 	mozjs.dll!js::gc::MarkValueRaw(JSTracer * trc=0x0012b564, const js::Value & v={...})  Line 341 + 0x1b bytes	C++
 	mozjs.dll!js::gc::MarkValueRange(JSTracer * trc=0x0012b564, js::Value * beg=0x050a26f8, js::Value * end=0x050a2700, const char * name=0x00a843ec)  Line 357 + 0xd bytes	C++
 	mozjs.dll!js::gc::MarkValueRange(JSTracer * trc=0x0012b564, unsigned int len=1, js::Value * vec=0x050a26f8, const char * name=0x00a843ec)  Line 364 + 0x1b bytes	C++
 	mozjs.dll!fun_trace(JSTracer * trc=0x0012b564, JSObject * obj=0x070acc40)  Line 1974 + 0x2d bytes	C++
 	mozjs.dll!js::gc::MarkChildren(JSTracer * trc=0x0012b564, JSObject * obj=0x070acc40)  Line 665 + 0x10 bytes	C++
 	mozjs.dll!JS_TraceChildren(JSTracer * trc=0x0012b564, void * thing=0x070acc40, unsigned int kind=0)  Line 753 + 0xd bytes	C++
 	xul.dll!UnmarkGrayChildren(JSTracer * trc=0x0012b564, void * thing=0x070acc40, unsigned int kind=0)  Line 590 + 0x12 bytes	C++
...


Note that automation shows this same signature in aurora WinXP/Win7 for other justin.tv urls though not this one. 

For aurora, try http://www.justin.tv/the_jesus_chatline/b/286335617 and maybe reload to trigger the crash


Note we also get

###!!! ASSERTION: JSEventListener has wrong script context?: 'stack && NS_SUCCEEDED(stack-
>Peek(&cx)) && cx && GetScriptContextFromJSContext(cx) == mContext', file c:/work/mozilla/
builds/nightly/mozilla/dom/src/events/nsJSEventListener.cpp, line 220

###!!! ASSERTION: JSEventListener has wrong script context?: 'stack && NS_SUCCEEDED(stack-
>Peek(&cx)) && cx && GetScriptContextFromJSContext(cx) == mContext', file c:/work/mozilla/
builds/aurora/mozilla/dom/src/events/nsJSEventListener.cpp, line 220

See also bug 650519
(Reporter)

Comment 1

6 years ago
Created attachment 536267 [details]
crash report xp
Seems like custom JSTracers have problems not with over-recursion.
(Assignee)

Comment 3

6 years ago
Created attachment 536674 [details] [diff] [review]
patch

I haven't been able to reproduce the problem on this site, but I think this patch should fix the problem. It just checks for a stack overflow in UnmarkGray, and if it runs out of stack, it requires a GC to run before the next CC.
Assignee: general → wmccloskey
Status: NEW → ASSIGNED
Attachment #536674 - Flags: review?(gal)

Comment 4

6 years ago
Comment on attachment 536674 [details] [diff] [review]
patch

This is a great fix! I didn't think of just forcing a GC instead of dealing with a delayed marking stack.
Attachment #536674 - Flags: review?(gal) → review+
 static void
 UnmarkGrayChildren(JSTracer *trc, void *thing, uint32 kind)
 {
+    int stackDummy;
+    if (!JS_CHECK_STACK_SIZE(trc->context->stackLimit, &stackDummy)) {

Maybe a stupid question but are we sure that stackLimit corresponds to the current thread?
(Assignee)

Comment 6

6 years ago
(In reply to comment #5)
> Maybe a stupid question but are we sure that stackLimit corresponds to the
> current thread?

Good question. The context that we're using is somehow derived from some kind of per-thread data in XPConnect, so I think it should be for the current thread. Whether it's actually been set properly is another matter. I see a bunch of callers to JS_SetNativeStackQuota, so I think we're probably safe.

Unfortunately, the patch is leaking on tryserver, so I'll need to sort this out.
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/tracemonkey/rev/85083c2d1f32
Whiteboard: fixed-in-tracemonkey
(Assignee)

Updated

6 years ago
Duplicate of this bug: 637796
Crash Signature: [@ js::gc::Cell::arenaHeader() | js::gc::Cell::address()]
I believe that before my patch for Bug 663532 landed that the cycle collector never actually invoked the garbage collector, so we should make sure that that patch (and its followup that fixes a leak that my patch introduced...) ends up in the same release as this one.
Depends on: 663532
(Reporter)

Comment 10

6 years ago
This is fixed on mozilla-central right? Merged on June 6 by cdleary ?
http://hg.mozilla.org/mozilla-central/rev/85083c2d1f32
(In reply to comment #10)
> This is fixed on mozilla-central right? Merged on June 6 by cdleary ?
> http://hg.mozilla.org/mozilla-central/rev/85083c2d1f32

It looks to me like it landed and this should probably be marked fixed.

Given comment 9, is it too much for this to be pushed forward onto the Gecko 6 branch? or could it be possible? I'm asking this as it appears to be a top crasher for Thunderbird at the moment (#2).
As I say above, note that you probably need Bug 663532 and Bug 664506, too, as the cycle collector wasn't working quite right, in a way that this bug depends on, as far as I can tell.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
OS: Windows XP → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/mozilla-central/rev/85083c2d1f32
(Assignee)

Updated

6 years ago
Duplicate of this bug: 662634

Comment 15

6 years ago
It is #9 top crasher in 6.0.
tracking-firefox6: --- → ?
(Assignee)

Comment 16

6 years ago
Should we try to get this into 6?

Comment 17

6 years ago
We don't need to track this but request approval for 6 and it will get triaged.
tracking-firefox6: ? → -
(Assignee)

Updated

6 years ago
Attachment #536674 - Flags: approval-mozilla-beta?

Updated

6 years ago
Attachment #536674 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 18

6 years ago
Comment on attachment 536674 [details] [diff] [review]
patch

Please land on both Aurora and Beta and please, in the future, please include a risk assessment with approval requests. Thanks.
Attachment #536674 - Flags: approval-mozilla-aurora+
Comment on attachment 536674 [details] [diff] [review]
patch

This patch is already in Aurora, so I'm removing the approval flag.
Attachment #536674 - Flags: approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-beta/rev/61bff9064c10
(In reply to comment #20)
> http://hg.mozilla.org/releases/mozilla-beta/rev/61bff9064c10

You're supposed to update the status flags when landing stuff on branches.
status-firefox6: --- → fixed
I thought I was supposed to wait until the landing actually stuck, but thanks, I'll set them immediately in the future.

Comment 23

6 years ago
Verified on:
Mac: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:6.0) Gecko/20100101 Firefox/6.0
Windows 7: Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0
Windows XP: Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0
Linux: Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0

There are no errors/crashes while playing the videos / link provided in the description
Status: RESOLVED → VERIFIED

Comment 24

6 years ago
(In reply to comment #23)
> Verified on:
> Mac: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:6.0) Gecko/20100101
> Firefox/6.0
> Windows 7: Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0
> Windows XP: Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0
> Linux: Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0
Which browser.startup.homepage_override.buildID? Firefox 6.0b1 or b2?

Comment 25

6 years ago
> Which browser.startup.homepage_override.buildID? Firefox 6.0b1 or b2?
Build 2
It was not clear whether this landed before the last Aurora merge or not, so now we don't exactly know whether this exists on Beta or not.  Somebody from the js team should confirm.
tracking-firefox7: --- → +
I think it is did.  It is present in Aurora as of this morning.  I don't know if that is before or after the merge. http://hg.mozilla.org/releases/mozilla-aurora/file/37edafab8b33/xpcom/base/nsCycleCollector.cpp

If xpcom/base/nsCycleCollector.cpp has a call to NeedCollect(), then this patch has been applied.
"I think it is did. "   This should be "I think it is."
Blocks: 602225

Comment 29

6 years ago
mNeedGCBeforeCC is on mozilla-beta, marking this as fixed
status-firefox7: --- → fixed
Blocks: 651445
You need to log in before you can comment on or make changes to this bug.