11819 - Multiple Load of Attached XUL Crashes in Listener Cleanup

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[rhp (gone)]

I will attach a XUL file to this bug that is a sample of what mail headers will
start looking like in Messenger. When I turn on this feature and load a few
messages into the same webshell, I crash in:

                 nsEventListenerManager::ReleaseListeners()

To recreate, just bring up apprunner (or viewer), load the attached XUL file
and then force a reload. You will see the crash.


Here is the Win32 stack trace:

nsEventListenerManager::ReleaseListeners(nsVoidArray * * 0x0af4d5c4) line 148 +
21 bytes
nsEventListenerManager::~nsEventListenerManager() line 78
nsEventListenerManager::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsEventListenerManager::Release(nsEventListenerManager * const 0x0af4d5a0) line
85 + 96 bytes
RDFElementImpl::~RDFElementImpl() line 451 + 27 bytes
RDFElementImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
RDFElementImpl::Release(RDFElementImpl * const 0x0af4c420) line 544 + 102 bytes
nsSupportsArray::Clear(nsSupportsArray * const 0x0af4cae0) line 313 + 36 bytes
nsSupportsArray::DeleteArray() line 58
nsSupportsArray::~nsSupportsArray() line 35
nsSupportsArray::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsSupportsArray::Release(nsSupportsArray * const 0x0af4cae0) line 53 + 95 bytes
RDFElementImpl::~RDFElementImpl() line 465 + 18 bytes
RDFElementImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
RDFElementImpl::Release(RDFElementImpl * const 0x0af48480) line 544 + 102 bytes
nsSupportsArray::Clear(nsSupportsArray * const 0x0af428d0) line 313 + 36 bytes
nsSupportsArray::DeleteArray() line 58
nsSupportsArray::~nsSupportsArray() line 35
nsSupportsArray::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsSupportsArray::Release(nsSupportsArray * const 0x0af428d0) line 53 + 95 bytes
RDFElementImpl::~RDFElementImpl() line 465 + 18 bytes
RDFElementImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
RDFElementImpl::Release(RDFElementImpl * const 0x0af38f90) line 544 + 102 bytes
nsCOMPtr<nsIContent>::~nsCOMPtr<nsIContent>() line 474
XULDocumentImpl::~XULDocumentImpl() line 941 + 327 bytes
XULDocumentImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
XULDocumentImpl::Release(XULDocumentImpl * const 0x0b202560) line 1036 + 102
bytes
nsCOMPtr<nsIDocument>::~nsCOMPtr<nsIDocument>() line 474
DocumentViewerImpl::~DocumentViewerImpl() line 295 + 66 bytes
DocumentViewerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
DocumentViewerImpl::Release(DocumentViewerImpl * const 0x0b2022a0) line 237 +
99 bytes
nsWebShell::Embed(nsWebShell * const 0x0a9c0500, nsIContentViewer * 0x0b219420,
const char * 0x0b21b530, nsISupports * 0x00000000) line 927 + 27 bytes
nsDocumentBindInfo::OnStartRequest(nsDocumentBindInfo * const 0x0b21cf10,
nsIChannel * 0x0b21ea60, nsISupports * 0x00000000) line 1953 + 36 bytes
nsOnStartRequestEvent::HandleEvent(nsOnStartRequestEvent * const 0x0b21a0a0)
line 212
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x0b21a0a4) line 149 + 12 bytes
PL_HandleEvent(PLEvent * 0x0b21a0a4) line 509 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00aefe90) line 470 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x0dfa0420, unsigned int 49324, unsigned int 0,
long 11468432) line 932 + 9 bytes
USER32! 77e71268()
00aefe90()

Comment 1

[rhp (gone)]

Attachment: Multiple loads of this XUL file will crash apprunner/viewer

Comment 2

[Peter Trudelle]

assigning to hyatt for triage, cc waterson cuz there's a lot of RDF stuff on the
stack.  Chris, could you please glance at this?

Comment 3

[David Hyatt]

Reassigning it to waterson.

Comment 4

[Chris Waterson]

Pink, I took a stab at fixing this bug, but it'll end up needing quite a bit o'
work on your part I think. Here's the deal.

You've registered a frame (the toolbox frame) as a drag listener. This is kind
of bad, because a frame is not _really_ an nsISupports; the event listener
manager addrefs the frame, and then when the document is torn down, it tries to
release it. Problem is, the frame's lifetime is controlled by the frame system,
not by XPCOM addref/release.

What you need to do (IMO) is create a small delegate object that implements
nsIDOMDragListener. The toolbox frame can hold a pointer reference to that
object, and the object can hold a weak reference back to the toolbox frame.
When the toolbox frame goes away, it can release the object and null the back
pointer. The delegate object will forward nsIDOMDragListener on to the toolbox
frame iff its back pointer != 0.

That will allow the "frame world" of lifetimes to co-exist with the XPCOM
refcounted world of lifetimes so that nobody stomps on anyone else.

Comment 5

[Chris Waterson]

Attachment: the fix

Comment 6

[Chris Waterson]

Okay Pink, I drank one too many cups o' coffee tonight. I went ahead and
implemented the fix. Please see the above set of diffs to nsToolboxFrame.h and
nsToolboxFrame.cpp. If they look ok, I'll check 'em in.

Comment 7

[Mike Pinkerton (not reading bugmail)]

Yikes. this is the first i've even heard of this bug (since rods is all over
cvsblame on the lines that do the bad thing). Um, you seem to understand it, but
I really don't even know why the toolbox frame is a nsIDOMDragListener. That
seems very very very bad.

You have my permission to do the right thing to fix this bug. I'll go back in
later and rework things when I get to toolbox d&d. I sure hope he didn't do the
same thing on toolbars. Sigh.

Reassigning to waterson.

Comment 8

[Chris Waterson]

rods rocks! ok, i'll check in tomorrow after i verify on mac & linux.

Comment 9

[rhp (gone)]

Just an FYI...looks good on Win32. Let me know when you check this in so I can
turn on the new header display.

- rhp

Comment 10

[Chris Waterson]

Fix checked in. Update mozilla/layout/xul/base/src.

Comment 11

[Paul MacQuiddy]

Sorry for spam, re-assigning phillip's QA contact XPToolkit/XPWidget bugs to
claudius due to restructure

Comment 12

[Paul MacQuiddy]

verified

Comment 13

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/1f943a02006a
[fenix] For https://github.com/mozilla-mobile/fenix/issues/11819 - Show the mic in widget only if setting is enabled