Closed
Bug 660778
Opened 14 years ago
Closed 14 years ago
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'
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla7
People
(Reporter: bc, Assigned: billm)
References
()
Details
(Keywords: assertion, crash, Whiteboard: fixed-in-tracemonkey)
Crash Data
Attachments
(2 files)
233.34 KB,
text/plain
|
Details | |
10.10 KB,
patch
|
gal
:
review+
asa
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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•14 years ago
|
||
Comment 2•14 years ago
|
||
Seems like custom JSTracers have problems not with over-recursion.
Assignee | ||
Comment 3•14 years ago
|
||
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.
Comment 4•14 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+
Comment 5•14 years ago
|
||
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•14 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•14 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Updated•14 years ago
|
Crash Signature: [@ js::gc::Cell::arenaHeader() | js::gc::Cell::address()]
Comment 9•14 years ago
|
||
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•14 years ago
|
||
This is fixed on mozilla-central right? Merged on June 6 by cdleary ?
http://hg.mozilla.org/mozilla-central/rev/85083c2d1f32
Comment 11•14 years ago
|
||
(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).
Comment 12•14 years ago
|
||
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
Closed: 14 years ago
OS: Windows XP → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Assignee | ||
Comment 13•14 years ago
|
||
Assignee | ||
Comment 16•14 years ago
|
||
Should we try to get this into 6?
Comment 17•14 years ago
|
||
We don't need to track this but request approval for 6 and it will get triaged.
Assignee | ||
Updated•14 years ago
|
Attachment #536674 -
Flags: approval-mozilla-beta?
Updated•14 years ago
|
Attachment #536674 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 18•14 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 19•14 years ago
|
||
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+
Comment 20•14 years ago
|
||
Comment 21•14 years ago
|
||
(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
Comment 22•14 years ago
|
||
I thought I was supposed to wait until the landing actually stuck, but thanks, I'll set them immediately in the future.
Comment 23•14 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•14 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•14 years ago
|
||
> Which browser.startup.homepage_override.buildID? Firefox 6.0b1 or b2?
Build 2
Comment 26•14 years ago
|
||
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:
--- → +
Comment 27•14 years ago
|
||
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.
Comment 28•14 years ago
|
||
"I think it is did. " This should be "I think it is."
Comment 29•14 years ago
|
||
mNeedGCBeforeCC is on mozilla-beta, marking this as fixed
status-firefox7:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•