171949 - Crash selecting themes [@ JS_GetScriptFilename] [@ JS_HashString] Trunk

Status: RESOLVED FIXED

(Core :: XBL, defect, P1)

Description

[greer]

The first time that Talkback shows this signature is in the 20020909 Trunk
build. I don't see a specific checkin related to this part of the code. 
dougt did a lot of checkins on the 7th (most unrelated), cc'ing him on the off
chance that he is aware of a change that might have exposed this.

The comments point to changing themes so over to themes for first look, though
the stack looks more like it's xpconnect.

Stack Trace: 

	 JS_GetScriptFilename	[c:/builds/seamonkey/mozilla/js/src/jsdbgapi.c  line 767]
	 _createJSDObject	[c:/builds/seamonkey/mozilla/js/jsd/jsd_obj.c  line 142]
	 jsd_ObjectHook	[c:/builds/seamonkey/mozilla/js/jsd/jsd_obj.c  line 171]
	 js_NewObject	[c:/builds/seamonkey/mozilla/js/src/jsobj.c  line 1725]
	 JS_NewObject	[c:/builds/seamonkey/mozilla/js/src/jsapi.c  line 2019]
	 XPCWrappedNative::Init
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp  line 706]
	 XPCWrappedNative::GetNewOrUsed
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp  line 354]
	 XPCConvert::NativeInterface2JSObject
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcconvert.cpp  line 1061]
	 nsXPConnect::WrapNative
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/nsXPConnect.cpp  line 563]
	 xpc_NewIDObject	[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcjsid.cpp
 line 975]
	 nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp  line 259]
	 nsXPCWrappedJSClass::GetRootJSObject
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp  line 597]
	 nsXPCWrappedJS::GetNewOrUsed
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp  line 219]
	 XPCConvert::JSObject2NativeInterface
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcconvert.cpp  line 1135]
	 nsXPConnect::WrapJS
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/nsXPConnect.cpp  line 587]
	 nsJSUtils::ConvertJSValToXPCObject
[c:/builds/seamonkey/mozilla/dom/src/base/nsJSUtils.cpp  line 125]
	 GlobalWindowImpl::GetObjectProperty
[c:/builds/seamonkey/mozilla/dom/src/base/nsGlobalWindow.cpp  line 3869]
	 nsContentTreeOwner::SetStatus
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsContentTreeOwner.cpp  line 349]
	 nsWebShell::OnOverLink
[c:/builds/seamonkey/mozilla/docshell/base/nsWebShell.cpp  line 692]
	 nsGenericElement::TriggerLink
[c:/builds/seamonkey/mozilla/content/base/src/nsGenericElement.cpp  line 3103]
	 nsGenericHTMLElement::HandleDOMEventForAnchors
[c:/builds/seamonkey/mozilla/content/html/content/src/nsGenericHTMLElement.cpp 
line 1544]
	 nsHTMLLinkElement::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/html/content/src/nsHTMLLinkElement.cpp 
line 364]
	 nsGenericDOMDataNode::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/base/src/nsGenericDOMDataNode.cpp  line 829]
	 nsEventStateManager::GenerateMouseEnterExit
[c:/builds/seamonkey/mozilla/content/events/src/nsEventStateManager.cpp  line 2476]
	 nsEventStateManager::PreHandleEvent
[c:/builds/seamonkey/mozilla/content/events/src/nsEventStateManager.cpp  line 396]
	 PresShell::HandleEventInternal
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp  line 6215]
	 PresShell::HandleEvent
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp  line 6143]
	 nsViewManager::HandleEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp  line 2121]
	 nsView::HandleEvent	[c:/builds/seamonkey/mozilla/view/src/nsView.cpp  line 301]
	 nsViewManager::DispatchEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp  line 1932]
	 HandleEvent	[c:/builds/seamonkey/mozilla/view/src/nsView.cpp  line 83]
	 nsWindow::DispatchEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 1042]
	 nsWindow::DispatchWindowEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 1059]
	 nsWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 5220]
	 ChildWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 5475]
	 nsWindow::ProcessMessage
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 3921]
	 nsWindow::WindowProc
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 1308]
	 USER32.dll + 0x3a5f (0x77d13a5f)
	 USER32.dll + 0x3b2e (0x77d13b2e)
	 USER32.dll + 0x3d6a (0x77d13d6a)
	 USER32.dll + 0x3dd0 (0x77d13dd0)
	 nsAppShellService::Run
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp  line 472]
	 main1	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1538]
	 main	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1886]
	 WinMain	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1906]
	 WinMainCRTStartup()
	 kernel32.dll + 0x1eb69 (0x77e5eb69)
 
 	Source File : c:/builds/seamonkey/mozilla/js/src/jsdbgapi.c line : 767

     (11733783)	Comments: accessing the view themes menu
     (11594805)	Comments: Pressed the left button at 'Apply theme'
     (11594721)	Comments: I only opened Navigator and tryed to select the theme
'modern'

Comment 1

[greer]

Also appearing under the JS_HashString signature.

Comment 2

[greer]

Attachment: Stack for the JS_HashString signature

Comment 3

[Jan Carpenter]

Not many recent incidents of either stack signature in the Talkback database;
marking this topcrash-

Comment 4

[timeless]

*** Bug 194362 has been marked as a duplicate of this bug. ***

Comment 5

[timeless]

venkman...

Comment 6

[David Bradley]

I wonder if the script was collected. The hash crash is from a bad string passed
in, which came from querying the script object. And the JS_GetScriptFilename is
in querying the script object.

So I bet the event that is occuring is now against some dead script.

Comment 7

[Brendan Eich [:brendan]]

jsd_obj.c line 137 gets the script via JS_GetFrameScript on a frame enumerated
from a live JSContext, so the script should still be alive.  Except that there
is no interpreter activation on the stack shown in comment 0.  So what
non-native stack frame was enumerated?  Does XPConnect push a non-native frame?

/be

Comment 8

[Matthias Versen [:Matti]]

*** Bug 212767 has been marked as a duplicate of this bug. ***

Comment 9

[Matthias Versen [:Matti]]

testcase : http://bugzilla.mozilla.org/attachment.cgi?id=127812

Comment 10

[[:nickname]]

Attachment: Another JS_HashString crash

Had the same crash with the JS_HashString() stack signature, although not
emering from XPCWrappedNative::Init (see attachment Another JS_HashString crash
)

Comment 11

[[:nickname]]

I forgot to add, this was from incident TB23488409W.

Comment 12

[Brendan Eich [:brendan]]

I still don't see how _createJSDObject finds a non-native (or any) JSStackFrame
on the context's stack, but this bug seems to be in XBL. 
nsXBLPrototypeHandler::ExecuteHandler does not root its void *handler local, and
if the debugger runs a GC when called indirectly from
boundContext->BindCompiledEventHandler, JS will nuke the handler function object.

Patch in a few.

/be

Comment 13

[Brendan Eich [:brendan]]

More diagnosis: nsXBLPrototypeHandler::ExecuteHandler does connect the handler
it compiles via onEventAtom's id to the scriptObject it gets from an xpconnect
object wrapper, but it fails to protect that object from being GC'd.  The "//
root" comment before

    // root
    nsCOMPtr<nsIXPConnectJSObjectHolder> wrapper;

    // XXX: Don't use the global object!
    rv = xpc->WrapNative(cx, global, aReceiver, NS_GET_IID(nsISupports),
                         getter_AddRefs(wrapper));

around line 414 is good, but this code is in a block that ends too soon, letting
the nsCOMPtr go out of scope well before handler is compiled and bound.

This is an easy-and-safe 1.5final fix.

/be

Comment 14

[Brendan Eich [:brendan]]

Attachment: fix for crash shown in second attachment

I don't know about the first attachment, that still needs diagnosis.

/be

Comment 15

[Brendan Eich [:brendan]]

Comment on attachment 131448 [details] [diff] [review]
fix for crash shown in second attachment

Looking for r+sr=bryner for 1.5final checkin.

/be

Comment 16

[Brendan Eich [:brendan]]

Cc'ing jst in case bryner is incommunicado, en route to Austin.

/be

Comment 17

[Peter Van der Beken [:peterv]]

Comment on attachment 131448 [details] [diff] [review]
fix for crash shown in second attachment

(I hope I'm allowed to do this r+/sr+ thing too)

Comment 18

[Brian Ryner (not reading)]

Comment on attachment 131448 [details] [diff] [review]
fix for crash shown in second attachment

sr=bryner too

Comment 19

[Brendan Eich [:brendan]]

Comment on attachment 131448 [details] [diff] [review]
fix for crash shown in second attachment

Looking for 1.5 approval.

/be

Comment 20

[Brendan Eich [:brendan]]

Fixed on trunk and branch.

/be