19302 - [DOGFOOD]Crash closing a dialog in its JS onload handler

Status: VERIFIED DUPLICATE of bug 21961

(Core :: XUL, defect, P1)

Description

[Syd Logan]

Run the above xul file. In the onload handler, a confirm prompt is displayed.
Clicking cancel will cause the onload handler to dismiss the dialog. Which leads
toa a crash.

Comment 1

[Syd Logan]

Attachment: test.xul

Comment 2

[Syd Logan]

Attachment: test.js -- place this in same directory as test.xul

Comment 3

[Syd Logan]

Place test.xul and test.js in bin/res, open with resource:/res/test.xul

Comment 4

[leger]

PDT+ AIM bug #370588 is dependant on this bug.  Putting this one on the PDT+
radar.

Comment 5

[Christopher Blizzard (:blizzard)]

This is a refcnt problem which the more I work on, the scarier it gets.  I've
got a partial fix but it ends up leaving a big gray window left behind.  This is
because the nsWindow::nsWindow class is not destroyed properly probably because
of a dangling reference somewhere.  I'm going to be rewriting the Destroy code
in the GTK code to clean this up.

Comment 6

[Syd Logan]

this is reported to be happening on Windows as well.

Comment 7

[Christopher Blizzard (:blizzard)]

Really?  I don't feel so bad now.

Comment 8

[Alex Musil]

Is Dec 2 still a reasonable date?

Comment 9

[Christopher Blizzard (:blizzard)]

Yes.  I'm still working on it.  I've got a patch that works pretty well except
for one border case.  The border case is a refcnt problem.  The problem is that
someone still has a refcnt on the toplevel nsIWidget so when you close the
window it leaves a gray box hanging in space.  Once I track down that problem I
can check it in.  If you are itching I can provide you with a pre-patch.

Comment 10

[Christopher Blizzard (:blizzard)]

Ok, I have a patch for this.  This bug kicked my butt for a couple of days.  I
got to rewrite much of the focus tracking code and destroy code for the widget,
though.  Waiting for the tree to open to check it in.

Comment 11

[Christopher Blizzard (:blizzard)]

Fix checked in.

Comment 12

[Claudius Gayle]

could someone shine a clue my way? I've downloaded the two test files and I dropped them
into the 'res' directory. 'typing resource:/res/' allows me to see the files test.xul and test.js
but clicking on test.xul with the 1999120709 Mac build crashes hard every time. With the
windows 1999120715 build doing the same loads the page test.xul and I can see 'Hello world'
but the progress bar never stops spinning and I'm never prompted with a confirm dialog.

Are the steps for reproduction no longer valid? Am I not executing them correctly? Are there
other bugs blocking reproduction/verification of this one, or is this just still badly broken? I
can't tell. I'm reopening to get this on the right radar and get sorted out.

Comment 13

[Christopher Blizzard (:blizzard)]

If I remember correctly this was a unix only bug.  I had to fix some front end
specific code to make it go.  If there's a problem with this, I would open a
different bug since it's probably not related.

Comment 14

[scalkins]

FYI: We had a depends on this bug for AIM, and when this bug was fixed, it fixed
our issue also. Now I see the problem has resurfaced in todays commercial Linux
build 1999-12-08-08 M12 and reopened our bug also. Problem did not occur on Mac
for todays builds however.

Comment 15

[Claudius Gayle]

Well, I've checked with fresh builds(1999120815) on all platforms and the
behavior is just as previously described on Win32 AND Linux, and it still
crashes hard on Mac. I'll write a new bug if I get closer to figuring out what's
wrong. This bug will depend on that new bug as I can't verify this bug until the
other is fixed.

Comment 16

[leger]

Clearing FIXED resolution due to reopen.

Comment 17

[Syd Logan]

The test XUL and JS I attached above with the original bug pass on both Windows
and Linux (I don't have a Mac box to try on).

Anyone have a test case that can be used to duplicate the problem?

Comment 18

[Syd Logan]

But the test *does* fail in the commercial tree.

Comment 19

[Christopher Blizzard (:blizzard)]

I tried this and it worked just fine in the mozilla build.

Comment 20

[Syd Logan]

Attachment: New version of test.xul (replaces one attached 11/18)

Comment 21

[scalkins]

Another testcase: In Seamonkey AIM (on Linux), remove an online buddy from your
buddy list. Have
that buddy send you an instant message. You will get the knock knock mode dialog
asking if you want to accept the IM(OK) or refuse it(Cancel). Click on Cancel
and you will crash.

Comment 22

[Syd Logan]

#0  0x40fc7d58 in nsEventListenerManager::ReleaseListeners (this=0x86bc100,
    aListeners=0x86bc10c, aScriptOnly=0) at nsEventListenerManager.cpp:166
#1  0x40fc7825 in nsEventListenerManager::~nsEventListenerManager (
    this=0x86bc100, __in_chrg=3) at nsEventListenerManager.cpp:81
#2  0x40fc79c6 in nsEventListenerManager::Release (this=0x86bc100)
    at nsEventListenerManager.cpp:95
#3  0x4120da1e in nsGenericElement::~nsGenericElement (this=0x86bb868,
    __in_chrg=2) at nsGenericElement.cpp:185
#4  0x41210e9b in nsGenericContainerElement::~nsGenericContainerElement (
    this=0x86bb868, __in_chrg=2) at nsGenericElement.cpp:1407
#5  0x411a95c1 in nsGenericXMLElement::~nsGenericXMLElement (this=0x86bb868,
    __in_chrg=2) at nsGenericXMLElement.cpp:67
#6  0x411a7a7d in nsXMLElement::~nsXMLElement (this=0x86bb850, __in_chrg=3)
    at nsXMLElement.cpp:76
#7  0x411dbba3 in AnonymousElement::~AnonymousElement (this=0x86bb850,
    __in_chrg=3) at ../../../../dist/include/nsCOMPtr.h:515
#8  0x411a7e36 in nsXMLElement::Release (this=0x86bb850) at nsXMLElement.cpp:96
#9  0x411dada6 in AnonymousElement::Release (this=0x86bb850)
    at nsScrollbarFrame.cpp:215
#10 0x40fcba19 in nsEventStateManager::~nsEventStateManager (this=0x86bacd0,
    __in_chrg=3) at nsEventStateManager.cpp:118
#11 0x40fcbca0 in nsEventStateManager::Release (this=0x86bacd0)
    at nsEventStateManager.cpp:135
#12 0x412431c4 in nsCOMPtr<nsIEventStateManager>::~nsCOMPtr (this=0x85951dc,
    __in_chrg=2) at ../../../dist/include/nsCOMPtr.h:433
#13 0x41218cb8 in nsPresContext::~nsPresContext (this=0x8595118, __in_chrg=0)
    at nsPresContext.cpp:115
#14 0x4120a9da in GalleyContext::~GalleyContext (this=0x8595118, __in_chrg=3)
    at nsGalleyContext.cpp:44
#15 0x41218e52 in nsPresContext::Release (this=0x8595118)
    at nsPresContext.cpp:119
#16 0x412473f4 in nsCOMPtr<nsIPresContext>::~nsCOMPtr (this=0x8607930,
    __in_chrg=2) at ../../../dist/include/nsCOMPtr.h:433
#17 0x411ffeb6 in DocumentViewerImpl::~DocumentViewerImpl (this=0x8607900,
    __in_chrg=3) at nsDocumentViewer.cpp:320
#18 0x411ffa95 in DocumentViewerImpl::Release (this=0x8607900)
    at nsDocumentViewer.cpp:252
#19 0x40b6a172 in ?? () from /opt/raptor/ns/dist/bin/libraptorwebwidget.so
#20 0x410fc876 in nsGfxTextControlFrame::~nsGfxTextControlFrame (
    this=0x84df540, __in_chrg=3) at nsGfxTextControlFrame.cpp:300
#21 0x40feaeb8 in nsFrame::Destroy (this=0x84df540, aPresContext=0x82796a8)
    at nsFrame.cpp:407
#22 0x41207d63 in nsFrameList::DestroyFrames (this=0x84df410,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#23 0x40fe674d in nsContainerFrame::Destroy (this=0x84df3dc,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#24 0x41207d63 in nsFrameList::DestroyFrames (this=0x84df388,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#25 0x40fe674d in nsContainerFrame::Destroy (this=0x84df354,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#26 0x41207d63 in nsFrameList::DestroyFrames (this=0x84db0dc,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#27 0x40fe674d in nsContainerFrame::Destroy (this=0x84db0a8,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#28 0x41207d63 in nsFrameList::DestroyFrames (this=0x84db090,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#29 0x40fe674d in nsContainerFrame::Destroy (this=0x84db05c,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#30 0x41207d63 in nsFrameList::DestroyFrames (this=0x84d112c,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#31 0x40fe674d in nsContainerFrame::Destroy (this=0x84d10f8,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#32 0x41207d63 in nsFrameList::DestroyFrames (this=0x84c4cc4,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#33 0x40fe674d in nsContainerFrame::Destroy (this=0x84c4c90,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#34 0x41207d63 in nsFrameList::DestroyFrames (this=0x84c4c88,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#35 0x40fe674d in nsContainerFrame::Destroy (this=0x84c4c54,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#36 0x41207d63 in nsFrameList::DestroyFrames (this=0x84c4c4c,
    aPresContext=0x82796a8) at nsFrameList.cpp:34
#37 0x40fe674d in nsContainerFrame::Destroy (this=0x84c4c18,
    aPresContext=0x82796a8) at nsContainerFrame.cpp:94
#38 0x4102dfc6 in ViewportFrame::Destroy (this=0x84c4c18,
    aPresContext=0x82796a8) at nsViewportFrame.cpp:137
#39 0x40ff22f6 in FrameManager::~FrameManager (this=0x828ef00, __in_chrg=3)
    at nsFrameManager.cpp:340
#40 0x40ff2242 in FrameManager::Release (this=0x828ef00)
    at nsFrameManager.cpp:326
#41 0x41016d25 in PresShell::~PresShell (this=0x82459b8, __in_chrg=3)
    at nsPresShell.cpp:683
#42 0x410169e4 in PresShell::Release (this=0x82459b8) at nsPresShell.cpp:614
#43 0x412434b4 in nsCOMPtr<nsIPresShell>::~nsCOMPtr (this=0x8216b94,
    __in_chrg=2) at ../../../dist/include/nsCOMPtr.h:433
#44 0x411ffea8 in DocumentViewerImpl::~DocumentViewerImpl (this=0x8216b60,
    __in_chrg=3) at nsDocumentViewer.cpp:320
#45 0x411ffa95 in DocumentViewerImpl::Release (this=0x8216b60)
    at nsDocumentViewer.cpp:252
#46 0x40b6a172 in ?? () from /opt/raptor/ns/dist/bin/libraptorwebwidget.so
#47 0x40536006 in ?? () from /opt/raptor/ns/dist/bin/libnsappshell.so
#48 0x4042b56d in ?? () from /opt/raptor/ns/dist/bin/libjsdom.so
#49 0x4042b5ff in ?? () from /opt/raptor/ns/dist/bin/libjsdom.so
#50 0x404197cb in ?? () from /opt/raptor/ns/dist/bin/libjsdom.so
#51 0x40418ead in ?? () from /opt/raptor/ns/dist/bin/libjsdom.so
#52 0x40453e19 in ?? () from /opt/raptor/ns/dist/bin/libjsdom.so
#53 0x40fc979d in nsEventListenerManager::HandleEventSubType (this=0x8514ce0,
    aListenerStruct=0x839ce78, aDOMEvent=0x82ec454, aSubType=1)
    at nsEventListenerManager.cpp:651
#54 0x40fcacba in nsEventListenerManager::HandleEvent (this=0x8514ce0,
    aPresContext=0x8612be0, aEvent=0xbffff554, aDOMEvent=0xbffff2d4, aFlags=7,
    aEventStatus=0xbffff594) at nsEventListenerManager.cpp:1197
#55 0x40430ca1 in ?? () from /opt/raptor/ns/dist/bin/libjsdom.so
#56 0x40b67859 in ?? () from /opt/raptor/ns/dist/bin/libraptorwebwidget.so
#57 0x40b5cfda in ?? () from /opt/raptor/ns/dist/bin/libraptorwebwidget.so
#58 0x40b5cc83 in ?? () from /opt/raptor/ns/dist/bin/libraptorwebwidget.so
#59 0x40b5cad0 in ?? () from /opt/raptor/ns/dist/bin/libraptorwebwidget.so
#60 0x405c12d2 in ?? () from /opt/raptor/ns/dist/bin/components/libnecko.so
#61 0x405f62e3 in ?? ()
   from /opt/raptor/ns/dist/bin/components/libnecko_file.so
#62 0x405ae10d in ?? () from /opt/raptor/ns/dist/bin/components/libnecko.so
#63 0x405ad7f7 in ?? () from /opt/raptor/ns/dist/bin/components/libnecko.so
#64 0x401b836b in ?? () from /opt/raptor/ns/dist/bin/libplds3.so
#65 0x401b827c in ?? () from /opt/raptor/ns/dist/bin/libplds3.so
#66 0x4016e999 in ?? () from /opt/raptor/ns/dist/bin/libxpcom.so
#67 0x4066216c in ?? () from /opt/raptor/ns/dist/bin/libwidget_gtk.so
#68 0x40661a7f in ?? () from /opt/raptor/ns/dist/bin/libwidget_gtk.so
#69 0x4087ad3a in ?? () from /usr/local/lib/libglib-1.2.so.0
#70 0x4087c296 in ?? () from /usr/local/lib/libglib-1.2.so.0
#71 0x4087c7d1 in ?? () from /usr/local/lib/libglib-1.2.so.0

Comment 23

[Christopher Blizzard (:blizzard)]

This looks really familiar.  Look at my comments in this bug:

http://bugzilla.mozilla.org/show_bug.cgi?id=20193

Comment 24

[Claudius Gayle]

Attachment: the Mac stack trace

Comment 25

[Claudius Gayle]

so with the 1999120910 Mac (commercial, opt) builds and the new attachement
test.xul I still crash as soon as i try to open it. I've attached my MacsBug log.

Comment 26

[Syd Logan]

Peter, I don't think Blizzard can help us because it is only reproducible in the
commercial tree. Assigning to trudelle for re-assignment to NS engineer.

Comment 27

[Alex Musil]

Clearing old date.

Comment 28

[Peter Trudelle]

Okay, reassigning to danm.

Comment 29

[Claudius Gayle]

syd, I saw where you mentioned the test fails in the comm tree. How exactly does it fail?
I'm curious because on no platform have I gotten as far as being presented with a dialog.
I need to know if my results are unique or if I'm seeing what others see.

Comment 30

[scalkins]

I saw this crash today in Comm Rel builds for Linux and Win.
To get the dialog, remove a buddy from any groups it may be in in your AIM buddy
list, then have that buddy send you an IM message. You will get an OK or Cancel
button on the Acceptance dialog box..click "Cancel". This will cause the crash.

Comment 31

[Alex Musil]

dan - if you are still having trouble repro-ing this bug, please let me know and
I can show you on my machine.

Comment 32

[Dan M]

I haven't been able to verify that this is fixed on Linux (because I don't seem to be able to launch
AIM in my Linux build today, for some inexplicable reason.) But this crash is now fixed on the Mac
and Windows. The fix is entirely XP, so I'm optimistic it's fixed for everyone.

The simpler instructions contained in the attachments don't serve to reproduce the problem because
the crash involves processing multiple load listeners after the first load listener has brought down
the window. So it's dependent on the exact XUL you're using, and could probably be reproduced
in the attachments by adding more load listeners.

Regardless, large parts of the app are just stunned when you force it to destroy a window in the
middle of the onload event. I've anesthetized it so it will survive the initial shock. But my fix
wasn't really a general solution; more a bunch of squirrely patches. I'm concerned about future
regressions. If that happens, it probably makes sense to delay window destruction until the
onload event has been completely processed. And also to patch the event handling system in
general to handle unexpected object deletions. See my notes today in bug 13860.

All that said, I should also mention that while the app will no longer crash immediately, it
will still crash when you quit the app after having seen the deadly Knock Knock dialog.
It turns out that's a different problem; the subject of Bugsplat (internal) bug 380638.
Forewarned is four-armed.

Comment 33

[Syd Logan]

I'm still crashing here with code pulled in 12/16.

Comment 34

[Dan M]

Details? Platform? Same AIM test case?

Comment 35

[Dan M]

Well, now we have problems. I've resurrected AIM on my Linux box., so now I can try
this on three platforms. I'm not seeing the crash any longer on any of my machines,
linux, mac or NT. By the way, it's kind of hard to get to the knock knock dialog on
today's linux build: I didn't see one until I tried to close the IM window. But anyway,
no crash. Suggestions? Help? Anyone else want to take a shot at fixing this?

Comment 36

[Syd Logan]

dan, what about the test.xul and test.js from above?

Comment 37

[chris hofmann]

final m12 candidates are spinnning now. moving to m13.
if we fall off track and need to respin m12 for some
yet unknown reason we can consider this if you get
a fix in hand.

Comment 38

[Christopher Blizzard (:blizzard)]

I haven't been able to reproduce this.

Comment 39

[Syd Logan]

Blizzard, it's commercial tree only.

Comment 40

[chris hofmann]

syd suggests removing pdt+

no longer required for the work he is doing, or at least not until B2.

Comment 41

[Christopher Blizzard (:blizzard)]

I've it's commercial build only there's probably no way I'm going to be able to
track it down unless you get more than a backtrace.

Comment 42

[leger]

Clearing FIXED resolution due to reopen.

Comment 43

[leger]

Putting on PDT- radar.

Comment 44

[Syd Logan]

Another test case:

remove ~/.mozilla
start mozilla (regualt, not commercial).
In the Profile manager, select "Next", then "Finish".
then, you will crash.

Currently, the profile dialog has a bug of its own that rginda and ben goodger
are working out, but, once that is fixed, this should be easy to reporduce. he
is doing all his stuff, including dismissal of the window, in the onload handler
too.

Comment 45

[Syd Logan]

The stack crawl is the same as in 21961, BTW.

Comment 46

[Dan M]

Thanks for taking this one, blizzard. For record's sake, none of the three crash scenarios
mentioned in this bug cause me any problems at all.

Comment 47

[Syd Logan]

This seems to have morphed into 21961. Marking resolved, duplicate.

*** This bug has been marked as a duplicate of 21961 ***

Comment 48

[Christopher Blizzard (:blizzard)]

Please ignore the spam.  Changing address.

Comment 49

[Christopher Blizzard (:blizzard)]

buste when I reassigned

Comment 50

[shrirang khanzode]

Sorry for the spam. changing qa contact.

Comment 51

[leger]

Adding crash keyword

Comment 52

[Matt Perry]

Verified duplicate.