215798 - Running Venkman crashes Mozilla [@ js_CloseTokenStream]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Vedran Miletic]

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.5b) Gecko/20030810
Venkman 0.9.77

Running Venkman will crash Mozilla every time on a fresh profile.
Talkback IDs: TB22649216K, TB22649166E

Comment 1

[Adam Hauner]

WFM 2003081004/W2K

Comment 2

[Vedran Miletic]

This is how I reproduced it:
1. Uninstall Mozilla, install
http://ftp.mozilla.org/pub/mozilla/nightly/latest/mozilla-win32-installer-sea.exe
2. Install http://www.hacksrus.com/~ginda/venkman/venkman-0.9.77.xpi (I
installed ChatZilla 0.9.15 too, dunno if that could be relevant).
3. Exit Mozilla.
4. Create a fresh profile (this is important step, it works for me with old
profile - but, afaik, fresh profile shouldn't affect installed Venkman because
this extension comes integrated and gets updated on installing 0.9.77 xpi and it
is profile independant).
5. Run Mozilla.
6. Run Venkman.
It will crash Mozilla.

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Incident ID 22649216
Stack Signature 	JS_GetPrivate c2f0748f
Email Address 	
Product ID 	MozillaTrunk
Build ID 	2003081004
Trigger Time 	2003-08-11 06:01:24
Platform 	Win32
Operating System 	Windows NT 5.2 build 3790
Module 	js3250.dll
URL visited 	
User Comments 	
Trigger Reason 	Access violation
Source File Name 	c:/builds/seamonkey/mozilla/js/src/jsapi.c
Trigger Line No. 	1941
Stack Trace 	
JS_GetPrivate [c:/builds/seamonkey/mozilla/js/src/jsapi.c, line 1941]
js_PCToLineNumber [c:/builds/seamonkey/mozilla/js/src/jsscript.c, line 1163]
jsd_GetClosestLine [c:/builds/seamonkey/mozilla/js/jsd/jsd_scpt.c, line 477]
jsdScript::IsLineExecutable [c:/builds/seamonkey/mozilla/js/jsd/jsd_xpc.cpp,
line 1377]
XPTC_InvokeByIndex
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2019]
XPC_WN_CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1270]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 845]
js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2861]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861]
nsXPCWrappedJSClass::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp, line 1333]
nsXPCWrappedJS::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp, line 429]
PrepareAndDispatch
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp,
line 119]
SharedStub
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp,
line 147]
jsdService::EnumerateScripts [c:/builds/seamonkey/mozilla/js/jsd/jsd_xpc.cpp,
line 2644]
XPTC_InvokeByIndex
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2019]
XPC_WN_CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1270]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 845]
js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2861]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861]
js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2861]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861]
js_InternalInvoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 936]
JS_CallFunctionValue [c:/builds/seamonkey/mozilla/js/src/jsapi.c, line 3540]
nsJSContext::CallEventHandler
[c:/builds/seamonkey/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1220]
nsJSEventListener::HandleEvent
[c:/builds/seamonkey/mozilla/dom/src/events/nsJSEventListener.cpp, line 183]
nsEventListenerManager::HandleEventSubType
[c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp, line
1195]
nsEventListenerManager::HandleEvent
[c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp, line
1879]
GlobalWindowImpl::HandleDOMEvent
[c:/builds/seamonkey/mozilla/dom/src/base/nsGlobalWindow.cpp, line 852]
DocumentViewerImpl::LoadComplete
[c:/builds/seamonkey/mozilla/content/base/src/nsDocumentViewer.cpp, line 952]
nsDocShell::EndPageLoad
[c:/builds/seamonkey/mozilla/docshell/base/nsDocShell.cpp, line 4327]
nsWebShell::EndPageLoad
[c:/builds/seamonkey/mozilla/docshell/base/nsWebShell.cpp, line 800]
nsDocShell::OnStateChange
[c:/builds/seamonkey/mozilla/docshell/base/nsDocShell.cpp, line 4261]
nsDocLoaderImpl::FireOnStateChange
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 1215]
nsDocLoaderImpl::doStopDocumentLoad
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 870]
nsDocLoaderImpl::DocLoaderIsEmpty
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 768]
nsDocLoaderImpl::OnStopRequest
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 698]
nsLoadGroup::RemoveRequest
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsLoadGroup.cpp, line 704]
nsCachedChromeChannel::HandleStopLoadEvent
[c:/builds/seamonkey/mozilla/rdf/chrome/src/nsChromeProtocolHandler.cpp, line 485]
PL_HandleEvent [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c, line 672]
PL_ProcessPendingEvents [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c,
line 610]
_md_EventReceiverProc [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c, line
1413]
USER32.dll + 0x35142 (0x77d35142)
USER32.dll + 0x35142 (0x77d35142)
_setargv()
kernel32.dll + 0xf38c (0x77e4f38c)
kernel32.dll + 0x30abc (0x77e70abc) 

Comment 4

[Brendan Eich [:brendan]]

Can someone reproduce this with a debug build?

Why does the new inside-the-AOL-firewall talkback server's "Code around the PC"
tab no longer disassemble code around the PC?

/be

Comment 5

[Brendan Eich [:brendan]]

jrgm, pls. see last comment about talkback server regression.

/be

Comment 6

[John Morrison]

At the risk of being sent out for skooling on my x86, EIP = 0x0 in the registers
in that incident, so we don't really have a good place to go in the modules. 

It does, I suppose, beg the question of whether EIP was really zero at the time 
of the crash or if we somehow botched the handling of the data while we wrote out 
the blackbox (or later, from the blackbox on the way into the database). However, 
I've looked at other incidents that were submitted and processed at the same 
time, and most have a plausible value for the PC. Not sure what to say...

Comment 7

[John Morrison]

I found another crash with the stack function list, and which had 
(supposedly) valid register info...

Trigger Type:  Program Crash
Trigger Reason:  Access violation
Thread ID: 
Call Stack:    (Signature = JS_GetPrivate 5060302b)

JS_GetPrivate 	[c:/builds/seamonkey/mozilla/js/src/jsapi.c, line 1941]
js_PCToLineNumber 	[c:/builds/seamonkey/mozilla/js/src/jsscript.c, line 1163]
jsd_GetClosestLine 	[c:/builds/seamonkey/mozilla/js/jsd/jsd_scpt.c, line 477]
jsdScript::IsLineExecutable 	[c:/builds/seamonkey/mozilla/js/jsd/jsd_xpc.cpp,
line 1377]
XPTC_InvokeByIndex 
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2019]
XPC_WN_CallMethod 
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1270]
js_Invoke 	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 845]
js_Interpret 	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2861]
js_Invoke 	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861]
nsXPCWrappedJSClass::CallMethod 
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp, line 1333]
nsXPCWrappedJS::CallMethod 
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp, line 429]
PrepareAndDispatch 
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp,
line 119]
SharedStub 
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp,
line 147]
jsds_ScriptHookProc 	[c:/builds/seamonkey/mozilla/js/jsd/jsd_xpc.cpp, line 723]
jsd_NewScriptHookProc 	[c:/builds/seamonkey/mozilla/js/jsd/jsd_scpt.c, line 563]
js_CallNewScriptHook 	[c:/builds/seamonkey/mozilla/js/src/jsscript.c, line 1089]
js_NewScriptFromCG 	[c:/builds/seamonkey/mozilla/js/src/jsscript.c, line 1055]
CompileTokenStream 	[c:/builds/seamonkey/mozilla/js/src/jsapi.c, line 2963]
JS_CompileUCScriptForPrincipals 	[c:/builds/seamonkey/mozilla/js/src/jsapi.c,
line 3038]
JS_EvaluateUCScriptForPrincipals 	[c:/builds/seamonkey/mozilla/js/src/jsapi.c,
line 3487]
nsJSContext::EvaluateString 
[c:/builds/seamonkey/mozilla/dom/src/base/nsJSEnvironment.cpp, line 876]
nsScriptLoader::EvaluateScript 
[c:/builds/seamonkey/mozilla/content/base/src/nsScriptLoader.cpp, line 632]
nsScriptLoader::ProcessRequest 
[c:/builds/seamonkey/mozilla/content/base/src/nsScriptLoader.cpp, line 546]
nsScriptLoader::OnStreamComplete 
[c:/builds/seamonkey/mozilla/content/base/src/nsScriptLoader.cpp, line 888]
nsStreamLoader::OnStopRequest 
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsStreamLoader.cpp, line 144]
nsStreamListenerTee::OnStopRequest 
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsStreamListenerTee.cpp, line 66]
nsCOMPtr_base::assign_with_AddRef 
[c:/builds/seamonkey/mozilla/xpcom/glue/nsCOMPtr.cpp, line 71]
nsHttpChannel::OnStopRequest 
[c:/builds/seamonkey/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line 3326]
nsInputStreamPump::OnStateStop 
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsInputStreamPump.cpp, line 484]
nsInputStreamPump::OnInputStreamReady 
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsInputStreamPump.cpp, line 325]
nsInputStreamReadyEvent::EventHandler 
[c:/builds/seamonkey/mozilla/xpcom/io/nsStreamUtils.cpp, line 117]
PL_HandleEvent 	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c, line 672]
PL_ProcessPendingEvents 	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c,
line 610]
_md_EventReceiverProc 	[c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c,
line 1413]
nsAppShellService::Run 
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 484]
main1 	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1306]
main 	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1672]
WinMain 	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1694]
WinMainCRTStartup() 	 
KERNEL32.DLL + 0x87f5 (0x7c4e87f5) 	 


which has this info for code around the PC...


x86 Registers:
EAX: 00000000  EBX: 00f63e58  ECX: 0285ed20  EDX: 6107b608
ESI: 00000001  EDI: 00000000  ESP: 0012ed94  EBP: 0012edb8
EIP: 61032a4b  cf PF af ZF sf of IF df nt RF vm   IOPL: 0
CS: 001b DS: 0023 SS: 0023 ES: 0023 FS: 0038 GS: 0000


Code Around the PC:
61032a4b 8b0f             mov     ecx,[edi]     <-- EIP here, EDI is null
61032a4d 8b4104           mov     eax,[ecx+0x4]
61032a50 3bc2             cmp     eax,edx
61032a52 740e             jz      61032a62
61032a54 85c0             test    eax,eax
61032a56 7429             jz      61032a81
61032a58 8b30             mov     esi,[eax]
61032a5a 3b3508b60761     cmp     esi,[6107b608]
61032a60 751f             jnz     61032a81
61032a62 8b7314           mov     esi,[ebx+0x14]
61032a65 807e6100         cmp     byte ptr [esi+0x61],0x0
61032a69 7416             jz      61032a81


which looks like this (with more context from the VC++ IDE). [Ignore
the difference in offsets in the left hand column; this is from a
month old build, although the actual code in JS_GetPrivate hasn't
changed, so relative offsets should match].

1928:
1929: JS_PUBLIC_API(void *)
1930: JS_GetPrivate(JSContext *cx, JSObject *obj)
1931: {
010E2A24   push        ebx
010E2A25   push        esi
010E2A26   push        edi
1932:     jsval v;
1933:
1934:     JS_ASSERT(OBJ_GET_CLASS(cx, obj)->flags & JSCLASS_HAS_PRIVATE);
1935:     v = GC_AWARE_GET_SLOT(cx, obj, JSSLOT_PRIVATE);
010E2A27   mov         edi,dword ptr [esp+14h]             <--- sets EDI
010E2A2B   mov         ebx,dword ptr [esp+10h]
010E2A2F   mov         edx,offset js_ObjectOps (0112b608)
010E2A34   mov         ecx,dword ptr [edi]                 <--- EIP is here
010E2A36   mov         eax,dword ptr [ecx+4]
010E2A39   cmp         eax,edx
010E2A3B   je          JS_GetPrivate+27h (010e2a4b)
010E2A3D   test        eax,eax
010E2A3F   je          JS_GetPrivate+46h (010e2a6a)
010E2A41   mov         esi,dword ptr [eax]
010E2A43   cmp         esi,dword ptr [js_ObjectOps (0112b608)]
010E2A49   jne         JS_GetPrivate+46h (010e2a6a)
010E2A4B   mov         esi,dword ptr [ebx+14h]
010E2A4E   cmp         byte ptr [esi+61h],0
010E2A52   je          JS_GetPrivate+46h (010e2a6a)
010E2A54   mov         esi,dword ptr [esi+2288h]
010E2A5A   cmp         esi,dword ptr [ebx+154h]
010E2A60   jne         JS_GetPrivate+46h (010e2a6a)
010E2A62   mov         eax,dword ptr [edi+4]
010E2A65   mov         eax,dword ptr [eax+0Ch]
010E2A68   jmp         JS_GetPrivate+69h (010e2a8d)
010E2A6A   cmp         eax,edx
010E2A6C   je          JS_GetPrivate+58h (010e2a7c)
010E2A6E   test        eax,eax
010E2A70   je          JS_GetPrivate+5Dh (010e2a81)
010E2A72   mov         edx,dword ptr [js_ObjectOps (0112b608)]
010E2A78   cmp         dword ptr [eax],edx
010E2A7A   jne         JS_GetPrivate+5Dh (010e2a81)
010E2A7C   cmp         dword ptr [ecx+28h],ebx
010E2A7F   je          JS_GetPrivate+3Eh (010e2a62)
010E2A81   push        3
010E2A83   push        edi
010E2A84   push        ebx
010E2A85   call        js_GetSlotThreadSafe (011027bb)
010E2A8A   add         esp,0Ch
010E2A8D   pop         edi
010E2A8E   pop         esi
1936:     if (!JSVAL_IS_INT(v))
010E2A8F   test        al,1
010E2A91   pop         ebx
010E2A92   je          JS_GetPrivate+7Ah (010e2a9e)
010E2A94   cmp         eax,80000001h
010E2A99   je          JS_GetPrivate+7Ah (010e2a9e)
1938:     return JSVAL_TO_PRIVATE(v);
010E2A9B   and         al,0FEh
1939: }
010E2A9D   ret
1937:         return NULL;
010E2A9E   xor         eax,eax
1939: }
010E2AA0   ret
1940:
1941: JS_PUBLIC_API(JSBool)
1942: JS_SetPrivate(JSContext *cx, JSObject *obj, void *data)


... which means that |obj| is null (?).

Comment 8

[John Morrison]

> ... which means that |obj| is null (?).

or invalid.

(Still don't know why the registers are so badly trashed in the other reports,
though).

Comment 9

[Asa Dotzler [:asa]]

rginda's not going to be fixing this. if brendan's gone 'till mid next week then
it might not make beta. 

Comment 10

[Brendan Eich [:brendan]]

How about this: correlate the bug with a checkin, or week's worth of checkins,
to js/src/*.[cht]*?

/be

Comment 11

[Brendan Eich [:brendan]]

Until someone can reproduce this, here it sits.

Caillon, rginda: any hope?

/be

Comment 12

[Asa Dotzler [:asa]]

1.5b shipped. moving request forward.

Comment 13

[Robert Ginda]

this crashes for me every time I start with the -venkman command line option,
and nearly every time if I have chatzilla running when starting venkman.

Comment 14

[Asa Dotzler [:asa]]

I can't reproduce this with today's linux build launching venkman via ./mozilla
-venkman. Chofmann can't reproduce on Windows. 

Comment 15

[Asa Dotzler [:asa]]

would consider a patch if one materializes. not going to hold the release for this. 

Comment 16

[Brendan Eich [:brendan]]

Rob, isn't this worksforme?  Or can you still reproduce?  I haven't been able to
find you on IRC lately.

/be

Comment 17

[Robert Ginda]

I've been away on vacation.  Still am, kinda, but I'm closer to email now.

I don't crash on startup with -venkman anymore, but I usually crash if I open a
navigator window or chatzilla, pretty much every time.  This stack looks like:

nsProfileLock::FatalSignalHandler(int)+0x000000D0 [/home/rginda_l/src/cvs/mozill
a/dist/bin/components/libprofile.so +0x00032D1A]
UNKNOWN [/lib/i686/libpthread.so.0 +0x0000C47E]
UNKNOWN [./mozilla-bin +0x00028C48]
UNKNOWN [/home/rginda_l/src/cvs/mozilla/dist/bin/libmozjs.so +0x00018353]
JS_CompileUCScriptForPrincipals+0x00000067 [/home/rginda_l/src/cvs/mozilla/dist/
bin/libmozjs.so +0x0001853D]
JS_EvaluateUCScriptForPrincipals+0x00000030 [/home/rginda_l/src/cvs/mozilla/dist
/bin/libmozjs.so +0x000192B8]
nsJSContext::EvaluateStringWithValue(nsAString const&, void*, nsIPrincipal*, cha
r const*, unsigned, char const*, void*, int*)+0x000005A6 [/home/rginda_l/src/cvs
/mozilla/dist/bin/components/libjsdom.so +0x0007B29A]
nsXBLProtoImplField::InstallMember(nsIScriptContext*, nsIContent*, void*, void*,
 nsCString const&)+0x00000126 [/home/rginda_l/src/cvs/mozilla/dist/bin/component
s/libgklayout.so +0x008215FC]
nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding*, nsIContent*)+0x000
002B7 [/home/rginda_l/src/cvs/mozilla/dist/bin/components/libgklayout.so +0x0082
19F7]
nsXBLPrototypeBinding::InstallImplementation(nsIContent*)+0x00000030 [/home/rgin
da_l/src/cvs/mozilla/dist/bin/components/libgklayout.so +0x008128F6]
nsXBLBinding::InstallImplementation()+0x00000078 [/home/rginda_l/src/cvs/mozilla
/dist/bin/components/libgklayout.so +0x0080E31C]
nsXBLBinding::InstallImplementation()+0x00000045 [/home/rginda_l/src/cvs/mozilla
/dist/bin/components/libgklayout.so +0x0080E2E9]
nsXBLService::LoadBindings(nsIContent*, nsAString const&, int, nsIXBLBinding**, 
int*)+0x00000CB1 [/home/rginda_l/src/cvs/mozilla/dist/bin/components/libgklayout
.so +0x00830269]
nsCSSFrameConstructor::ConstructFrameInternal(nsIPresShell*, nsIPresContext*, ns
FrameConstructorState&, nsIContent*, nsIFrame*, nsIAtom*, int, nsStyleContext*, 
nsFrameItems&, int)+0x000000E3 [/home/rginda_l/src/cvs/mozilla/dist/bin/componen
ts/libgklayout.so +0x004944AD]
nsCSSFrameConstructor::ConstructFrame(nsIPresShell*, nsIPresContext*, nsFrameCon
structorState&, nsIContent*, nsIFrame*, nsFrameItems&)+0x000001F9 [/home/rginda_
l/src/cvs/mozilla/dist/bin/components/libgklayout.so +0x00494357]
nsCSSFrameConstructor::ProcessChildren(nsIPresShell*, nsIPresContext*, nsFrameCo
nstructorState&, nsIContent*, nsIFrame*, int, nsFrameItems&, int, nsTableCreator
*)+0x00000190 [/home/rginda_l/src/cvs/mozilla/dist/bin/components/libgklayout.so
 +0x004A1BB8]
nsCSSFrameConstructor::ConstructXULFrame(nsIPresShell*, nsIPresContext*, nsFrame
ConstructorState&, nsIContent*, nsIFrame*, nsIAtom*, int, nsStyleContext*, nsFra
meItems&, int, int&)+0x000012B7 [/home/rginda_l/src/cvs/mozilla/dist/bin/compone
nts/libgklayout.so +0x004915FB]
...

Comment 18

[Brendan Eich [:brendan]]

What code is UNKNOWN [/home/rginda_l/src/cvs/mozilla/dist/bin/libmozjs.so
+0x00018353]?

Line numbers, please.

/be

Comment 19

[Brendan Eich [:brendan]]

Surprise (not!).  More GC hazards in XBL: nsXBLProtoImpl::InstallImplementation
calls nsXBLProtoImpl::InitTargetObjects to get some JSObject pointers back via
out params, and then passes those in a loop to
nsXBLProtoImplField::InstallMember.  I see no GC rooting of these JSObject
pointers, and they refer to objects for XPConnect wrappers created by
nsIXPConnect::WrapNative.  What protects these from being GC'd?

Cc'ing XBL and DOM/XPC gurus not already cc'd, who should feel free to help
here.  XBL is not my baby.  I'll attempt a patch if I get time, but this should
be fixed for 1.5final.

/be

Comment 20

[Brendan Eich [:brendan]]

The second out param from nsXBLProtoImpl::InitTargetObjects, which is the final
param, aTargetClassObject, should be safe from GC, if it is a class prototype
object referenced by a <ClassConstructor>.prototype property, where
<ClassConstructor> is  a property of the global object named after the XBL class.

The first out param is the one that seems to have been GC'd in the stack in
comment 17.  That out param, aScriptObject, is set from the object of the
xpconnect wrapper created or found by InitTargetObjects.

One possible fix is to change the parameterization to have
nsXBLProtoImpl::InstallImplementation pass an nsIXPConnectJSObjectHolder out
parameter to InitTargetObjects for aScriptObject.  XPConnect and XBL people,
please advise.

/be

Comment 21

[David Bradley]

Sounds like a good option to me, that will keep it safe till it's no longer
needed or tied down elsewhere. Also can get rid of the void** casting back and
forth as well since it would be an interface pointer now.

Comment 22

[Brendan Eich [:brendan]]

Attachment: patch to fix the GC safety bug, plus a few warning fixes

This is untested.  Please help try it out.

/be

Comment 23

[Johnny Stenback (:jst)]

Comment on attachment 131582 [details] [diff] [review]
patch to fix the GC safety bug, plus a few warning fixes

I wasn't able to test this or run with it, but the change looks good. Hopefully
someone else is able to verify that this fixes the crash.

sr=jst

Comment 24

[Brendan Eich [:brendan]]

bz-vacation: maybe you can have a look to?  I'm wondering how often
InitTargetObjects will find that doc is null (near the bottom) and therefore be
unable to add a reference to the wrapper to the document?  That (!doc) seems to
be a necessary condition for the crash bug to bite in the way that this patch
addresses.  Thanks,

/be

Comment 25

[Boris Zbarsky [:bzbarsky]]

Hmm... aBoundElement is the element we're attaching the binding to, no?  In that
case, GetDocument() should never return null, if I recall correctly; if it does,
the binding will never even get attached.  Or am I misremembering what
aBoundElement is?

The patch itself looks fine.

Comment 26

[Brendan Eich [:brendan]]

Comment on attachment 131582 [details] [diff] [review]
patch to fix the GC safety bug, plus a few warning fixes

Indeed, nsXBLProtoImpl::InstallImplementation will return early with NS_OK if
GetDocument returns a null document on aBoundElement, and never reach the call
to InitTargetObjects.  So the hypothesis is false -- back to the
diagnosing-board.

/be

Comment 27

[Brendan Eich [:brendan]]

Still, since rginda can reproduce a crash, it would be good to know that the
patch I just invalidated does *not* cure that symptom.

Caillon, can you reproduce the crash?  How about in a debug build?

/be

Comment 28

[Robert Ginda]

Attachment: gdb stack from crash

That patch didn't help the crash.

I get this stack by launching with -venkman, then opening a navigator window. 
Linux, debug, with a trunk from last night.

Comment 29

[Brendan Eich [:brendan]]

Rob, caillon said he can't reproduce; my tree is still rebuilding.  Can you see
what was going wrong in the code at

#6  0x4009aeb2 in js_CloseTokenStream (cx=0x8a35518, ts=0x8ef0928) at jsscan.c:264

That looks like this line:

        JSPRINCIPALS_DROP(cx, ts->principals);

What is *ts->principals?  It ought to be a member of a larger
nsIPrincipal-implementing class that you can dump by doing some address arithmetic.

/be

Comment 30

[Brendan Eich [:brendan]]

This bug is morphing.  I now know what caused the crash in comment 17, but I do
not know what caused the earlier crashes via JS_GetPrivate.

Let's fix what can be reproduced, which is the stack at attachment 131685 [details], also
shown incompletely in comment 17.  This crash is caused by venkman calling
JS_GC(cx) while cx->tempPool is in use by CompileTokenStream, a common
subroutine in jsapi.c of JS_Compile*Script* and JS_Evaluate*Script*.

Patch in a moment.

/be

Comment 31

[Brendan Eich [:brendan]]

Attachment: proposed fix

This should do it.  Rob, please test -- you seem almost alone in being able to
reproduce the latest stack, although I did it once, today, out of >5 tries.

/be

Comment 32

[Brendan Eich [:brendan]]

Nominating for 1.5.

/be

Comment 33

[Brendan Eich [:brendan]]

Comment on attachment 131712 [details] [diff] [review]
proposed fix

Looking for either or both rginda and shaver to review.  More the merrier, for
drivers' warm fuzzies.

/be

Comment 34

[Mike Shaver (:shaver emeritus)]

Comment on attachment 131712 [details] [diff] [review]
proposed fix

sr=shaver (But why doesn't that bite us all the time?  Just lucky, and helped
by the mostly-single-threaded engine use?)

Comment 35

[Brendan Eich [:brendan]]

shaver: most call-outs from the engine to embedding code that might run a GC on
the cx do not have any arenas in use on tempPool (or codePool; stackPool has
been protected as always -- see the patch).  Only venkman, the new script hook,
might be called indirectly from CompileTokenStream.

Not sure where ginda is.  I'm going to get this into the trunk so it can bake
for the branch landing before mozilla1.5 rc2.

/be

Comment 36

[Robert Ginda]

Comment on attachment 131712 [details] [diff] [review]
proposed fix

I tried the patch against my trunk build and it works like a charm.  No more
crashes starting either chatzilla or navigator while venkman's	running. 
Thanks Brendan!

Comment 37

[Johnny Stenback (:jst)]

r=jst too, fwiw.

Comment 38

[Brendan Eich [:brendan]]

Comment on attachment 131712 [details] [diff] [review]
proposed fix

Checked into trunk.  Going for 1.5 approval for rc2.

/be

Comment 39

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 131712 [details] [diff] [review]
proposed fix

a=dbaron, since it looks perfectly safe to me.

Comment 40

[Brendan Eich [:brendan]]

This landed on the trunk and branch on Friday.

/be