Last Comment Bug 330526 - CRASH reloading XForms with xml event listeners
: CRASH reloading XForms with xml event listeners
Status: RESOLVED FIXED
: fixed1.8.0.5, fixed1.8.1
Product: Core
Classification: Components
Component: XForms (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: Jonas Sicking (:sicking) PTO Until July 5th
: Stephen Pride
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-14 17:41 PST by aaronr
Modified: 2006-06-22 14:53 PDT (History)
4 users (show)
dveditz: blocking1.8.0.5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (576 bytes, application/xhtml+xml)
2006-03-14 17:43 PST, aaronr
no flags Details
Patch to fix (1.43 KB, patch)
2006-03-16 15:05 PST, Jonas Sicking (:sicking) PTO Until July 5th
no flags Details | Diff | Review
Alternative patch (1.26 KB, patch)
2006-03-16 15:22 PST, Jonas Sicking (:sicking) PTO Until July 5th
aaronr: review+
jst: superreview+
dveditz: approval‑branch‑1.8.1+
dveditz: approval1.8.0.5+
Details | Diff | Review

Description aaronr 2006-03-14 17:41:42 PST
I get random crashes loading/unloading XForms that have xml event listeners (action elements) on debug builds of firefox and seamonkey.  I don't think that this is XForms related, really, since if I take a build from 2006/03/08 and update  mozilla/extensions/xforms to trunk level, I can't recreate the trap.  However, using the 2006/03/14 trunk debug build, it is easy to recreate the trap.  I have tried to recreate the trap using xhtml button with a xforms xml event listener like xf:send and it doesn't recreate.  I've used xforms controls with JS event handlers (using addEventListener) and I don't get the crash.

The problem is that in nsXMLEventsListener::Unregister(), if the refcount of the mContent of the event target is <=2, it'll crash during the nsCOMPtr::~nsCOMPtr() of the nsIDOMEventTarget.  The mContent is getting freed prematurely, causing the memory debug code in the vc8 to memset 0xddddd into the object.  Later on someone will queryinterface with it and thus the crash.

Opening this bug since I really need to follow up on this and route it to the right component.  It is hard to tell when a crash is caused by a real xforms bug anymore.
Comment 1 aaronr 2006-03-14 17:43:29 PST
Created attachment 215086 [details]
testcase

reload this testcase over and over waiting a couple of seconds or less between reloads.  Should crash within 5 or 6 reloads.
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2006-03-15 07:14:12 PST
Could this be related to Bug 330084 ? According to talkback that has been there
since 2006/03/09 and it is 4. in the crash list.
Comment 3 aaronr 2006-03-15 09:21:32 PST
(In reply to comment #2)
> Could this be related to Bug 330084 ? According to talkback that has been there
> since 2006/03/09 and it is 4. in the crash list.
> 

Stack is waaaayyy different, but it is possible.  Timing sounds right.  I've got to narrow this down to the day and then go through the checkins, I guess. 
Comment 4 aaronr 2006-03-16 00:17:12 PST
Doron verified this crashes on Linux.

Looks like this is related to the patch put in for bug 326645.  Taking a working build from 03/08 and applying this patch results in the crash.  I'll try to catch up with sicking tomorrow to talk about it.
Comment 5 aaronr 2006-03-16 12:25:39 PST
call stack:

>	xpcom_core.dll!nsQueryInterface::operator()(const nsID & aIID={...}, void * * answer=0x0012f8a4)  Line 47 + 0x15 bytes	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::assign_from_qi(nsQueryInterface qi={...}, const nsID & aIID={...})  Line 1232 + 0x10 bytes	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::nsCOMPtr<nsIContent>(nsQueryInterface qi={...})  Line 646	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::Assert_NoQueryNeeded()  Line 594	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::operator=(nsIContent * rhs=0x00000000)  Line 715	C++
 	gklayout.dll!nsDOMEventRTTearoff::LastRelease()  Line 590	C++
 	gklayout.dll!nsDOMEventRTTearoff::Release()  Line 546 + 0xbd bytes	C++
 	gklayout.dll!nsCOMPtr<nsIDOMEventTarget>::~nsCOMPtr<nsIDOMEventTarget>()  Line 584	C++
 	gklayout.dll!nsXMLEventsListener::Unregister()  Line 201 + 0x8 bytes	C++
 	gklayout.dll!EnumAndUnregisterListener(nsISupports * aContent=0x05657aa0, nsCOMPtr<nsXMLEventsListener> & aListener={...}, void * aData=0x05669d00)  Line 261	C++
 	gklayout.dll!nsBaseHashtable<nsISupportsHashKey,nsCOMPtr<nsXMLEventsListener>,nsXMLEventsListener *>::s_EnumStub(PLDHashTable * table=0x05669d0c, PLDHashEntryHdr * hdr=0x03a0086c, unsigned int number=0, void * arg=0x0012f9b0)  Line 346 + 0x1e bytes	C++
 	xpcom_core.dll!PL_DHashTableEnumerate(PLDHashTable * table=0x05669d0c, PLDHashOperator (PLDHashTable *, PLDHashEntryHdr *, unsigned int, void *)* etor=0x01ba5f20, void * arg=0x0012f9b0)  Line 621 + 0x19 bytes	C
 	gklayout.dll!nsBaseHashtable<nsISupportsHashKey,nsCOMPtr<nsXMLEventsListener>,nsXMLEventsListener *>::Enumerate(PLDHashOperator (nsISupports *, nsCOMPtr<nsXMLEventsListener> &, void *)* enumFunc=0x01ba5510, void * userArg=0x05669d00)  Line 221 + 0x13 bytes	C++
 	gklayout.dll!nsXMLEventsManager::DocumentWillBeDestroyed(nsIDocument * aDocument=0x05840668)  Line 338	C++
 	gklayout.dll!nsDocument::~nsDocument()  Line 737	C++
 	gklayout.dll!nsHTMLDocument::~nsHTMLDocument()  Line 340 + 0x95 bytes	C++
 	gklayout.dll!nsHTMLDocument::`scalar deleting destructor'()  + 0xf bytes	C++
 	gklayout.dll!nsDocument::Release()  Line 850 + 0xee bytes	C++
 	gklayout.dll!nsHTMLDocument::Release()  Line 343 + 0xd bytes	C++
 	xpcom_core.dll!nsCOMPtr_base::~nsCOMPtr_base()  Line 82	C++
 	gklayout.dll!nsCOMPtr<nsISupports>::~nsCOMPtr<nsISupports>()  + 0x10 bytes	C++
 	gklayout.dll!nsEvent::~nsEvent()  + 0x28 bytes	C++
 	gklayout.dll!nsEvent::`scalar deleting destructor'()  + 0xf bytes	C++
 	gklayout.dll!nsDOMEvent::~nsDOMEvent()  Line 139 + 0x1f bytes	C++
 	gklayout.dll!nsDOMEvent::`scalar deleting destructor'()  + 0xf bytes	C++
 	gklayout.dll!nsDOMEvent::Release()  Line 144 + 0xd9 bytes	C++
 	xpc3250.dll!XPCJSRuntime::GCCallback(JSContext * cx=0x058034f8, JSGCStatus status=JSGC_END)  Line 562 + 0x14 bytes	C++
 	gklayout.dll!DOMGCCallback(JSContext * cx=0x058034f8, JSGCStatus status=JSGC_END)  Line 2204 + 0x17 bytes	C++
 	js3250.dll!js_GC(JSContext * cx=0x058034f8, unsigned int gcflags=0)  Line 2039 + 0x11 bytes	C
 	js3250.dll!js_ForceGC(JSContext * cx=0x058034f8, unsigned int gcflags=0)  Line 1567 + 0xd bytes	C
 	js3250.dll!JS_GC(JSContext * cx=0x058034f8)  Line 1842 + 0xb bytes	C
 	gklayout.dll!nsJSContext::Notify(nsITimer * timer=0x0586bbf8)  Line 2160 + 0xd bytes	C++
 	xpcom_core.dll!nsTimerImpl::Fire()  Line 404	C++
 	xpcom_core.dll!nsTimerManager::FireNextIdleTimer()  Line 636	C++
 	gkwidget.dll!nsAppShell::Run()  Line 142	C++
 	appcomps.dll!nsAppStartup::Run()  Line 208	C++
 	seamonkey.exe!main1(int argc=1, char * * argv=0x003d2fb0, nsISupports * nativeApp=0x00ab9580)  Line 1248 + 0x22 bytes	C++
 	seamonkey.exe!main(int argc=1, char * * argv=0x003d2fb0)  Line 1750 + 0x25 bytes	C++
 	seamonkey.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	seamonkey.exe!mainCRTStartup()  Line 403	C
 	kernel32.dll!7c816d4f() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
 	kernel32.dll!7c8399f3() 	
 	gklayout.dll!nsXBLWindowKeyHandler::WalkHandlers(nsIDOMEvent * aKeyEvent=0x00000000, nsIAtom * aEventType=0x00000000)  Line 205 + 0x1a bytes	C++
 	gklayout.dll!nsXBLWindowKeyHandler::WalkHandlers(nsIDOMEvent * aKeyEvent=, nsIAtom * aEventType=)  Line 205 + 0x1a bytes	C++
Comment 6 Jonas Sicking (:sicking) PTO Until July 5th 2006-03-16 13:16:42 PST
Something very weird is going on. That stack makes no sense at all. If operator= gets a null pointer then by the time we reach Assert_NoQueryNeeded mRawPtr should be null and we should just bail. Otherwise there's a bug in nsCOMPtr which I doubt.
Comment 7 Doron Rosenberg (IBM) 2006-03-16 13:24:08 PST
gdb backtrace is pretty much the same stack as windows:

#6  0x00138b28 in nsQueryInterface::operator() (this=Variable "this" is not available.
) at nsCOMPtr.cpp:47
#7  0x073272c4 in nsCOMPtr<nsIContent>::assign_from_qi (this=0xbf80cbf8, qi={mRawPtr = 0xa5d7ac8}, aIID=@0x7891b2c) at ../../../../dist/include/xpcom/nsCOMPtr.h:1232
#8  0x07327304 in nsCOMPtr (this=0xbf80cbf8, qi={mRawPtr = 0xa5d7ac8}) at ../../../../dist/include/xpcom/nsCOMPtr.h:645
#9  0x07327346 in nsCOMPtr<nsIContent>::Assert_NoQueryNeeded (this=0xa0c3544) at ../../../../dist/include/xpcom/nsCOMPtr.h:593
#10 0x07344c31 in nsCOMPtr<nsIContent>::operator= (this=0xa0c3544, rhs=0x0) at ../../../../dist/include/xpcom/nsCOMPtr.h:714
#11 0x07557d01 in nsDOMEventRTTearoff::LastRelease (this=0xa0c3530) at /home/doron/mozbuilds/trunk/mozilla/content/base/src/nsGenericElement.cpp:585
#12 0x07557df8 in nsDOMEventRTTearoff::Release (this=0xa0c3530) at /home/doron/mozbuilds/trunk/mozilla/content/base/src/nsGenericElement.cpp:546
#13 0x073e2062 in ~nsCOMPtr (this=0xbf80cccc) at ../../dist/include/xpcom/nsCOMPtr.h:583
#14 0x075afc94 in nsXMLEventsListener::Unregister (this=0xa44ee20) at /home/doron/mozbuilds/trunk/mozilla/content/events/src/nsXMLEventsManager.cpp:200
#15 0x075afcc6 in EnumAndUnregisterListener (aContent=0xa5d8080, aListener=@0xa5d823c, aData=0xa5d8160) at /home/doron/mozbuilds/trunk/mozilla/content/events/src/nsXMLEventsManager.cpp:260
#16 0x075b130b in nsBaseHashtable<nsISupportsHashKey, nsCOMPtr<nsXMLEventsListener>, nsXMLEventsListener*>::s_EnumStub (table=0xa5d816c, hdr=0xa5d8234, number=0, arg=0xbf80cd78) at ../../../dist/include/xpcom/nsBaseHashtable.h:346
#17 0x00138996 in PL_DHashTableEnumerate (table=0xa5d816c, etor=0x75b12ec <nsBaseHashtable<nsISupportsHashKey, nsCOMPtr<nsXMLEventsListener>, nsXMLEventsListener*>::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*)>, arg=0xbf80cd78) at pldhash.c:621
#18 0x075b1022 in nsBaseHashtable<nsISupportsHashKey, nsCOMPtr<nsXMLEventsListener>, nsXMLEventsListener*>::Enumerate (this=0xa5d816c, enumFunc=0x75afc9e <EnumAndUnregisterListener>, userArg=0xa5d8160) at ../../../dist/include/xpcom/nsBaseHashtable.h:221
#19 0x075afbed in nsXMLEventsManager::DocumentWillBeDestroyed (this=0xa5d8160, aDocument=0xa540328) at /home/doron/mozbuilds/trunk/mozilla/content/events/src/nsXMLEventsManager.cpp:337
#20 0x075391cf in ~nsDocument (this=0xa540328) at /home/doron/mozbuilds/trunk/mozilla/content/base/src/nsDocument.cpp:736
#21 0x0761c9ae in ~nsHTMLDocument (this=0xa540328) at /home/doron/mozbuilds/trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:340
#22 0x07534476 in nsDocument::Release (this=0xa540328) at /home/doron/mozbuilds/trunk/mozilla/content/base/src/nsDocument.cpp:850
#23 0x0761af7e in nsHTMLDocument::Release (this=0xa540328) at /home/doron/mozbuilds/trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:343
#24 0x0077b2fd in XPCJSRuntime::GCCallback (cx=0xb3d5d188, status=JSGC_END) at /home/doron/mozbuilds/trunk/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp:562
#25 0x076db4ba in DOMGCCallback (cx=0xb3d5d188, status=JSGC_END) at /home/doron/mozbuilds/trunk/mozilla/dom/src/base/nsJSEnvironment.cpp:2204
#26 0x0021a617 in js_GC (cx=0xb3d5d188, gcflags=0) at /home/doron/mozbuilds/trunk/mozilla/js/src/jsgc.c:2039
#27 0x00219885 in js_ForceGC (cx=0xb3d5d188, gcflags=0) at /home/doron/mozbuilds/trunk/mozilla/js/src/jsgc.c:1567
#28 0x001efec2 in JS_GC (cx=0xb3d5d188) at /home/doron/mozbuilds/trunk/mozilla/js/src/jsapi.c:1842
#29 0x076db45a in nsJSContext::Notify (this=0xb3d5cad0, timer=0xa4257b0) at /home/doron/mozbuilds/trunk/mozilla/dom/src/base/nsJSEnvironment.cpp:2160
#30 0x0019dae8 in nsTimerImpl::Fire (this=0xa4257b0) at /home/doron/mozbuilds/trunk/mozilla/xpcom/threads/nsTimerImpl.cpp:403
#31 0x0019dcbd in handleTimerEvent (aEvent=0xb3d337d0) at /home/doron/mozbuilds/trunk/mozilla/xpcom/threads/nsTimerImpl.cpp:467
#32 0x001972d9 in PL_HandleEvent (self=0xb3d337d0) at /home/doron/mozbuilds/trunk/mozilla/xpcom/threads/plevent.c:688
#33 0x001971ae in PL_ProcessPendingEvents (self=0x9dff158) at /home/doron/mozbuilds/trunk/mozilla/xpcom/threads/plevent.c:623
#34 0x001997fa in nsEventQueueImpl::ProcessPendingEvents (this=0x9dff110) at /home/doron/mozbuilds/trunk/mozilla/xpcom/threads/nsEventQueue.cpp:419
#35 0x014e0d66 in event_processor_callback (source=0x9f3e1b0, condition=G_IO_IN, data=0x9dff110) at /home/doron/mozbuilds/trunk/mozilla/widget/src/gtk2/nsAppShell.cpp:67
#36 0x0061c4fc in g_vasprintf () from /usr/lib/libglib-2.0.so.0
#37 0x005f64ce in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#38 0x005f94d6 in g_main_context_check () from /usr/lib/libglib-2.0.so.0
#39 0x005f97c3 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#40 0x003cea46 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#41 0x014e15bb in nsAppShell::Run (this=0x9e71c80) at /home/doron/mozbuilds/trunk/mozilla/widget/src/gtk2/nsAppShell.cpp:139
#42 0x00ed6c8c in nsAppStartup::Run (this=0x9e6a648) at /home/doron/mozbuilds/trunk/mozilla/xpfe/components/startup/src/nsAppStartup.cpp:207
#43 0x0804d8e0 in main1 (argc=3, argv=0xbf80d594, nativeApp=Variable "nativeApp" is not available.
) at /home/doron/mozbuilds/trunk/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1248
#44 0x0804dc6f in main (argc=3, argv=0xbf80d594) at /home/doron/mozbuilds/trunk/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1750
Comment 8 aaronr 2006-03-16 14:29:34 PST
call stack of where mRawPtr is getting reassigned a ptr value all the way back to nsXMLEventsListener::Unregister().

 	gklayout.dll!nsCOMPtr<nsIContent>::assign_assuming_AddRef(nsIContent * newPtr=0x039715a0)  Line 565	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::assign_with_AddRef(nsISupports * rawPtr=0x039715a0)  Line 1225	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::operator=(nsIContent * rhs=0x039715a0)  Line 714	C++
 	gklayout.dll!nsDOMEventRTTearoff::Create(nsIContent * aContent=0x039715a0)  Line 560	C++
 	gklayout.dll!nsGenericElement::QueryInterface(const nsID & aIID={...}, void * * aInstancePtr=0x0012f768)  Line 3066 + 0x1b bytes	C++
 	gklayout.dll!nsXMLElement::QueryInterface(const nsID & aIID={...}, void * * aInstancePtr=0x0012f768)  Line 103 + 0x11 bytes	C++
 	gklayout.dll!nsXTFBindableElementWrapper::QueryInterface(const nsID & aIID={...}, void * * aInstancePtr=0x0012f768)  Line 206 + 0x11 bytes	C++
 	xpcom_core.dll!nsQueryInterface::operator()(const nsID & aIID={...}, void * * answer=0x0012f768)  Line 47 + 0x19 bytes	C++
 	xforms.dll!nsCOMPtr<nsIDOMEventTarget>::assign_from_qi(nsQueryInterface qi={...}, const nsID & aIID={...})  Line 1232 + 0x10 bytes	C++
 	xforms.dll!nsCOMPtr<nsIDOMEventTarget>::nsCOMPtr<nsIDOMEventTarget>(nsQueryInterface qi={...})  Line 646	C++
>	xforms.dll!nsXFormsControlStubBase::ResetHelpAndHint(int aInitialize=0x00000000)  Line 297	C++
 	xforms.dll!nsXFormsControlStubBase::OnDestroyed()  Line 488	C++
 	xforms.dll!nsXFormsBindableControlStub::OnDestroyed()  Line 328	C++
 	xforms.dll!nsXFormsDelegateStub::OnDestroyed()  Line 97	C++
 	gklayout.dll!nsXTFBindableElementWrapper::~nsXTFBindableElementWrapper()  Line 131	C++
 	gklayout.dll!nsXTFBindableElementWrapper::`scalar deleting destructor'()  + 0xf bytes	C++
 	gklayout.dll!nsGenericElement::Release()  Line 3075 + 0xdc bytes	C++
 	gklayout.dll!nsXMLElement::Release()  Line 132 + 0xd bytes	C++
 	gklayout.dll!nsXTFElementWrapper::Release()  Line 82 + 0xd bytes	C++
 	gklayout.dll!nsXTFBindableElementWrapper::Release()  Line 171 + 0xd bytes	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::assign_assuming_AddRef(nsIContent * newPtr=0x00000000)  Line 569	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::assign_with_AddRef(nsISupports * rawPtr=0x00000000)  Line 1225	C++
 	gklayout.dll!nsCOMPtr<nsIContent>::operator=(nsIContent * rhs=0x00000000)  Line 714	C++
 	gklayout.dll!nsDOMEventRTTearoff::LastRelease()  Line 590	C++
 	gklayout.dll!nsDOMEventRTTearoff::Release()  Line 546 + 0xbd bytes	C++
 	gklayout.dll!nsCOMPtr<nsIDOMEventTarget>::~nsCOMPtr<nsIDOMEventTarget>()  Line 584	C++
 	gklayout.dll!nsXMLEventsListener::Unregister()  Line 201 + 0x8 bytes	C++
Comment 9 Jonas Sicking (:sicking) PTO Until July 5th 2006-03-16 15:05:26 PST
Created attachment 215342 [details] [diff] [review]
Patch to fix

This should fix it. Great thanks for the excellent debugging by Aaron, this would have been really hard to track down.
Comment 10 Jonas Sicking (:sicking) PTO Until July 5th 2006-03-16 15:22:22 PST
Created attachment 215344 [details] [diff] [review]
Alternative patch

This is an alternative way to fix it. Rather then calling Release before putting the tearoff in the cache, it calls Release after all teardown is done and it's safe to reuse us from the cache.

I don't really have a strong preference either way. Jst, what do you think?
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2006-03-16 16:01:27 PST
Comment on attachment 215344 [details] [diff] [review]
Alternative patch

I don't feel strongly either way either, but I think I'd pick this one since it keeps the code that deals with this inside the block where we put things in the cache. sr=jst
Comment 12 aaronr 2006-03-16 16:36:47 PST
Comment on attachment 215344 [details] [diff] [review]
Alternative patch

fixes the testcase
Comment 13 Jonas Sicking (:sicking) PTO Until July 5th 2006-03-16 16:52:06 PST
Comment on attachment 215342 [details] [diff] [review]
Patch to fix

We desided not to go with this one.
Comment 14 Jonas Sicking (:sicking) PTO Until July 5th 2006-03-16 16:55:25 PST
For anyone looking at this bug in the future (hello future people), the problem was that when calling Release on mContent someone ended up QI-ing an element to nsIDOMEventTarget. This would grab a tearoff from the cache, in fact it would grab the very tearoff that we were in the middle of initializing so that it could be reused.

The result is that everyone got their knickers is a knot and things blew up.
Comment 15 Jonas Sicking (:sicking) PTO Until July 5th 2006-03-16 16:59:22 PST
Patch checked in
Comment 16 Allan Beaufour 2006-05-24 07:54:50 PDT
*** Bug 339104 has been marked as a duplicate of this bug. ***
Comment 17 Allan Beaufour 2006-05-24 09:08:49 PDT
Requesting blocking 1.8.0.5. We (XForms) might be able work around this (bug 339104) for 1_8_0, but a global fix would be rather nice.
Comment 18 Daniel Veditz [:dveditz] 2006-06-06 14:40:33 PDT
Comment on attachment 215344 [details] [diff] [review]
Alternative patch

approved for 1.8.0 branch, a=dveditz for drivers

Note You need to log in before you can comment on or make changes to this bug.