Closed Bug 353704 Opened 14 years ago Closed 14 years ago

crash (not sure what I did, could have been on blogger.com) [@ js_SetProtoOrParent] [@ xpsp2res.dll] [@ 0x20202020]

Categories

(Core :: XPConnect, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: moco, Assigned: dbaron)

References

Details

(4 keywords, Whiteboard: [patch])

Crash Data

Attachments

(5 files)

crash (not sure what I did, could have been on blogger.com) [@js_SetProtoOrParent]

this was on my mac, trunk, debug build from 20060921

I'll attach the crash report
I think I had blogger.com going in a tab, and I was publishing a post, and then I clicked on "view blog" before it was done publishing.
Assignee: nobody → general
Severity: normal → critical
Component: General → JavaScript Engine
Keywords: crash
Product: Firefox → Core
QA Contact: general → general
Summary: crash (not sure what I did, could have been on blogger.com) [@js_SetProtoOrParent] → crash (not sure what I did, could have been on blogger.com) [@ js_SetProtoOrParent]
Crashes in and at random addresses (primarily 0x20202020, which for many users shows up as xpsp2res.dll) within js_SetProtoOrParent are by far the #1 topcrash for trunk builds starting with 2006-09-20-04-trunk.  Many of the stacks are:

0x20202020
js_GetSlotThreadSafe (sometimes)
js_SetProtoOrParent
JS_SetPrototype
xpc_CloneJSFunction
Keywords: topcrash
Summary: crash (not sure what I did, could have been on blogger.com) [@ js_SetProtoOrParent] → crash (not sure what I did, could have been on blogger.com) [@ js_SetProtoOrParent] [@ xpsp2res.dll] [@ 0x20202020]
My steps to repro are:
 1. load bug 353596
 2. middle click on the attachment to load it in a new tab
 3. click the [X] in the corner of the dialog immediately after it pops up

Using these steps, I found that the XPCWrappedNativeScope::mPrototypeJSFunction that we're trying to access in xpc_CloneJSFunction was allocated here:

NS_LogCtor_P (/home/dbaron/builds/trunk/mozilla/xpcom/base/nsTraceRefcntImpl.cpp:1111)
js_NewGCThing (/home/dbaron/builds/trunk/mozilla/js/src/jsgc.c:1448)
js_NewObject (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:2342)
js_NewFunction (/home/dbaron/builds/trunk/mozilla/js/src/jsfun.c:2073)
js_DefineFunction (/home/dbaron/builds/trunk/mozilla/js/src/jsfun.c:2142)
JS_DefineFunctions (/home/dbaron/builds/trunk/mozilla/js/src/jsapi.c:3792)
JS_InitClass (/home/dbaron/builds/trunk/mozilla/js/src/jsapi.c:2290)
js_InitObjectClass (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:2159)
js_InitFunctionAndObjectClasses (/home/dbaron/builds/trunk/mozilla/js/src/jsapi.c:1186)
JS_ResolveStandardClass (/home/dbaron/builds/trunk/mozilla/js/src/jsapi.c:1485)
TempGlobalResolve (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:443)
js_LookupPropertyWithFlags (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:3228)
js_GetProperty (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:3370)
XPCWrappedNativeScope::SetGlobal(XPCCallContext&, JSObject*) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp:212)
XPCWrappedNativeScope (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp:159)
XPCWrappedNativeScope::GetNewOrUsed(XPCCallContext&, JSObject*) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp:120)
nsXPConnect::InitClasses(JSContext*, JSObject*) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:419)
nsXPConnect::InitClassesWithNewWrappedGlobal(JSContext*, nsISupports*, nsID const&, unsigned int, nsIXPConnectJSObjectHolder**) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:488)
nsJSContext::InitContext(nsIScriptGlobalObject*) (/home/dbaron/builds/trunk/mozilla/dom/src/base/nsJSEnvironment.cpp:2171)
...

and garbage collected very soon after:

NS_LogDtor_P (/home/dbaron/builds/trunk/mozilla/xpcom/base/nsTraceRefcntImpl.cpp:1152)
js_GC (/home/dbaron/builds/trunk/mozilla/js/src/jsgc.c:2940)
js_NewGCThing (/home/dbaron/builds/trunk/mozilla/js/src/jsgc.c:1336)
js_NewObject (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:2342)
XPCWrappedNativeProto::Init(XPCCallContext&, int, XPCNativeScriptableCreateInfo const*) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativeproto.cpp:113)
XPCWrappedNativeProto::GetNewOrUsed(XPCCallContext&, XPCWrappedNativeScope*, nsIClassInfo*, XPCNativeScriptableCreateInfo const*, int, int) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativeproto.cpp:228)
XPCWrappedNative::GetNewOrUsed(XPCCallContext&, nsISupports*, XPCWrappedNativeScope*, XPCNativeInterface*, int, XPCWrappedNative**) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:373)
nsXPCComponents::AttachNewComponentsObject(XPCCallContext&, XPCWrappedNativeScope*, JSObject*) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpccomponents.cpp:3827)
nsXPConnect::InitClassesWithNewWrappedGlobal(JSContext*, nsISupports*, nsID const&, unsigned int, nsIXPConnectJSObjectHolder**) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:540)
nsJSContext::InitContext(nsIScriptGlobalObject*) (/home/dbaron/builds/trunk/mozilla/dom/src/base/nsJSEnvironment.cpp:2171)


That space in memory was then reused for two other objects (both allocated and then collected) before we ran into a problem.
And the XPCWrappedNativeScope in question is in gScopes (first, in fact), and its mGlobalJSObject was destroyed at the same stack as the mPrototypeJSFunction.
Actually, the third use of the pointer was for the same type of object, and I think it's that one I care about.  It was created here:

NS_LogCtor_P (/home/dbaron/builds/trunk/mozilla/xpcom/base/nsTraceRefcntImpl.cpp:1111)
js_NewGCThing (/home/dbaron/builds/trunk/mozilla/js/src/jsgc.c:1448)
js_NewObject (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:2342)
JS_InitClass (/home/dbaron/builds/trunk/mozilla/js/src/jsapi.c:2217)
js_InitFunctionClass (/home/dbaron/builds/trunk/mozilla/js/src/jsfun.c:2022)
js_InitFunctionAndObjectClasses (/home/dbaron/builds/trunk/mozilla/js/src/jsapi.c:1181)
JS_ResolveStandardClass (/home/dbaron/builds/trunk/mozilla/js/src/jsapi.c:1485)
nsWindowSH::NewResolve(nsIXPConnectWrappedNative*, JSContext*, JSObject*, long, unsigned int, JSObject**, int*) (/home/dbaron/builds/trunk/mozilla/dom/src/base/nsDOMClassInfo.cpp:6084)
XPC_WN_Helper_NewResolve (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1065)
js_LookupPropertyWithFlags (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:3181)
js_GetProperty (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:3370)
XPCWrappedNativeScope::SetGlobal(XPCCallContext&, JSObject*) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp:212)
nsXPConnect::InitClassesWithNewWrappedGlobal(JSContext*, nsISupports*, nsID const&, unsigned int, nsIXPConnectJSObjectHolder**) (/home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:532)
nsJSContext::CreateNativeGlobalForInner(nsIScriptGlobalObject*, int, void**, nsISupports**) (/home/dbaron/builds/trunk/mozilla/dom/src/base/nsJSEnvironment.cpp:2055)
nsGlobalWindow::SetNewDocument(nsIDocument*, nsISupports*, int, int) (/home/dbaron/builds/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp:1333)

and destroyed on a different event off the event loop, probably on the first GC under that event.
Attached patch patchSplinter Review
For a while I was thinking this was more complicated than it really was, even though I'd noticed the problem.  This makes mPrototypeJSFunction (which was added later) be managed more like mPrototypeJSObject -- so that it can't be dangling.

This is almost definitely a regression from my leak fix to bug 353090, although I haven't actually tested that.  What it did is it turned some places where we were already seeing the assertion:

###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property!

This is pretty much always bad. It usually means that native code is
making a callback to an interface implemented in JavaScript, but the
document where the JS object was created has already been cleared and the
global properties of that document's window are *gone*. Generally this
indicates a problem that should be addressed in the design and use of the
callback code.
: 'Error', file /home/dbaron/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 594

into crashes.


(I haven't yet looked into whether JS_ClearScope clears things that the page can't.  But either way I think this would probably be good to get on branches as well.)
Assignee: general → dbaron
Status: NEW → ASSIGNED
Attachment #239754 - Flags: superreview?(jst)
Attachment #239754 - Flags: review?(jst)
Component: JavaScript Engine → XPConnect
Blocks: 353090
Whiteboard: [patch]
QA Contact: general → xpconnect
OS: Mac OS X 10.4 → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 239754 [details] [diff] [review]
patch

r+sr=jst
Attachment #239754 - Flags: superreview?(jst)
Attachment #239754 - Flags: superreview+
Attachment #239754 - Flags: review?(jst)
Attachment #239754 - Flags: review+
Checked in to trunk.
Attachment #239754 - Flags: approval1.8.0.8?
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
*** Bug 354076 has been marked as a duplicate of this bug. ***
*** Bug 354291 has been marked as a duplicate of this bug. ***
*** Bug 353555 has been marked as a duplicate of this bug. ***
Comment on attachment 239754 [details] [diff] [review]
patch

This claims to be a regression from bug 353090 which is requesting 1.8.0.9 approval. Neither is requesting 1.8.1 approval which seems odd if this is an important fix.
Attachment #239754 - Flags: approval1.8.0.8? → approval1.8.0.9?
I just discussed this with brendan, and although we weren't sure if a page (rather than the C++ code I added to fix a leak) could have gotten us into a situation where we could trigger this bug and I'm at least not even sure how to figure out, this patch is quite safe and I think it's worth taking for 1.8.1.
Attachment #240249 - Flags: approval1.8.0.8?
Attachment #239754 - Flags: approval1.8.0.9?
Comment on attachment 240244 [details] [diff] [review]
fix error pointed out by brendan

Yeah, of course. Should've seen that when reviewing :(

r+sr=jst
Attachment #240244 - Flags: superreview?(jst)
Attachment #240244 - Flags: superreview+
Attachment #240244 - Flags: review?(jst)
Attachment #240244 - Flags: review+
Comment on attachment 240244 [details] [diff] [review]
fix error pointed out by brendan

Checked in to trunk.
Comment on attachment 240244 [details] [diff] [review]
fix error pointed out by brendan

>+        else {

Nit: jband-style braces go on their own lines.

/be
Brendan/DBaron - so we are not sure if this is content-induced crash and we are touching XPConnect/XPCNativeWrappers (which worries me some).  Are we sure two days before release we should take this?
No crashes in Talkback [@ xpsp2res.dll] or [@ js_SetProtoOrParent] since 2006-09-23-04. Marking as VERIFIED.
Status: RESOLVED → VERIFIED
Comment on attachment 240249 [details] [diff] [review]
combined patch for 1.8 branch

Approved for RC2.
Attachment #240249 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 240249 [details] [diff] [review]
combined patch for 1.8 branch

Disregard the approval - wrong bug.
Attachment #240249 - Flags: approval1.8.1+ → approval1.8.1?
I'm not sure if it could be content-induced.

However, the patch is quite safe.  All it's doing is:
 * marking a pointer that it wasn't marking before (to prevent it from being garbage collected and thus dangling)
 * nulling out that pointer if it actually is garbage collected so we don't have a dangling pointer

This is exactly the same thing we've been doing for a similar pointer that was there longer.
Comment on attachment 240249 [details] [diff] [review]
combined patch for 1.8 branch

Ok then.  Let's get this in for RC2.
Attachment #240249 - Flags: approval1.8.1? → approval1.8.1+
Fix checked in to MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1
Comment on attachment 240249 [details] [diff] [review]
combined patch for 1.8 branch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #240249 - Flags: approval1.8.0.8? → approval1.8.0.8+
Checked in to MOZILLA_1_8_0_BRANCH.
Keywords: fixed1.8.0.8
verified on the 1.8.0 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.8) Gecko/20061025 Firefox/1.5.0.8. I used dbaron's STR in Comment 4 and did not see the crash. Adding keyword.
Crash Signature: [@ js_SetProtoOrParent] [@ xpsp2res.dll] [@ 0x20202020]
You need to log in before you can comment on or make changes to this bug.