202439 - frequent crashes when closing/dismissing windows - Trunk [@ nsDocShell::Destroy]

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[sairuh (rarely reading bugmail)]

i've noticed that with the 2003.04.17.09 build (on linux rh8.0, so far) i've
been crashing quite often when i've closed various windows. unfortunately, i
haven't found a consistent set of steps --other than closing windows-- and the
stack traces vary. :(  but after encountering three crashes within ~30min, i
thought it couldn't hurt to file a bug. ;)  pls reassign or dup as needed.

i'll attach the talkback reports soon.

here is what i've done when i've encountered the crash:

a. had multiple tabs open (3 or 4) and clicked the close widget to close one of
the tabs.

b. had a browser and help window open, and hit ctrl+w to close the help window.

c. had a browser and editor window open, and hit ctrl+w to close the editor window.

Comment 1

[sairuh (rarely reading bugmail)]

Attachment: tb report for (a) closing a tab widget

Comment 2

[sairuh (rarely reading bugmail)]

Attachment: tb report for (b): closing help window

Comment 3

[sairuh (rarely reading bugmail)]

Attachment: tb report for (c): closing editor window

Comment 4

[sairuh (rarely reading bugmail)]

this wasn't a problem with the 2003.04.15.08 build.

Comment 5

[Leston Buell]

A crash also occurs when the window closes as the result of some other action,
such as clicking on the send button in Mail/News.

Comment 6

[Paul Wyskoczka]

I'm seeing this on Linux 8.0 with the 2003041709 build.

Comment 7

[sairuh (rarely reading bugmail)]

leston has a good point --i also crashed when i dismissed the table properties
dlg   (clicked OK button).

Comment 8

[Calum Mackay]

I've seen a few crashes in 20030417 when hitting "send" in mailnews; this is
closing the compose window, of course. No further details as yet.

Was OK in 20030416.

Comment 9

[Ronald Tilby]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030417
See Talkback ID's: TB19252388M and TB19252199Q

Comment 10

[Boris Zbarsky [:bzbarsky]]

Can someone get a narrower regression range (12 hours would be good...)?

The stacks all end on line 1248 of nsWebShell; the only reason that should crash
is if we call Destroy() on an already-deleted webshell....

Comment 11

[Calum Mackay]

I have a build 2003041619 which works (that's 7pm BST == GMT + 1)

It would help if I could turn on talkback for my cvs builds; is this possible?

Comment 12

[sairuh (rarely reading bugmail)]

i've been playing with the 2003.04.17.05 mozilla build (linux rh8.0) for about
20min and have yet to crash. has anyone experienced crashes with that particular
build?

Comment 13

[Boris Zbarsky [:bzbarsky]]

> It would help if I could turn on talkback for my cvs builds; is this possible?

How about you just enable symbols and use a debugger?  That'll let you tell me
pretty much anything that talkback can and possibly more.... ;)

Comment 14

[Hermann Schwab]

Build ID 2003041709 crashes on Win98 when closing a tab, DocWatson & Talkback
are coming up: 
TB19257114Z 
TB19257156E
TB19257307Z

corresponding Stack Summaries from DocWatson
(can someone give me a hint how to copy&paste this data from file.wlg?)

0167:0009
DOCSHELL.DLL:.text+0x2436
GKLAYOUT.DLL:.text+0x1b71a4
GKLAYOUT.DLL:.text+0x81b8a


0167:028ac46e
DOCSHELL.DLL:.text+0x2474
GKLAYOUT.DLL:.text+0x1b71a4
GKLAYOUT.DLL:.text+0x81b8a


GKLAYOUT.DLL:.rdata+0xd2c0
DOCSHELL.DLL:.text+0x2474
GKLAYOUT.DLL:.text+0x1b71a4
GKLAYOUT.DLL:.text+0x81b8a

Comment 15

[Matthias Versen [:Matti]]

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

Comment 16

[Hermann Schwab]

WORKING 2003041704 Win98 (until now, will post again, when it crashes)
crashes 2003041709 Win98

Had just installed build ID 2003041704, and had no crashes in the time I had ten
with the newer build.
But it came up with an oversized window, or two, and with an empty toolbar.
Normally I´ve got lots of BookmarksFolders in my personal Toolbar.
I deleted a window, maximized the rest, entered manually a URL, but DUN wasn´t
starting, so I stopped Mozilla.
At second start of Mozilla Personal Toolbar was ok, I selected one of the
bookmarks from there, DUN started, all was going well.

Comment 17

[Matthias Versen [:Matti]]

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

Comment 18

[Matthias Versen [:Matti]]

-> blocker (I crash evertime i close a tab or window)

Comment 19

[Boris Zbarsky [:bzbarsky]]

Hermann, thanks for getting that regression window!  Over to brade as the
honorary "only person to touch docshell in that time period"; reading over the
diffs now.

Comment 20

[Boris Zbarsky [:bzbarsky]]

mozilla% cvs diff -u -r 1.538 -r 1.539 docshell/base/nsDocShell.cpp

-    NS_IF_RELEASE(NS_STATIC_CAST(nsIClipboardDragDropHookList *,
mTransferableHookData));
+    nsIClipboardDragDropHookList *list =
NS_STATIC_CAST(nsIClipboardDragDropHookList *, mTransferableHookData);
+    NS_IF_RELEASE(list);
     delete mTransferableHookData;
     mTransferableHookData = nsnull;

So the old code would NS_IF_RELEASE mTransferableHookData, which called
Release() on it and nulled out the pointer ('cause that's what NS_IF_RELEASE
does).  Then the |delete 0;| and |mTransferableHookData = nsnull;| were no-ops.

With this change, we release the mTransferableHookData (which will sometimes
call its destructor, if we're holding the last ref).  Then we delete it (which
either deletes an object someone holds a ref to or double-deletes an object we
held the last ref to).  Then we set it to null (if we ever get to that line
without crashing).

Removing the "delete" line made the crashes go away here.

Not posting a patch because I can't tell whether you mean to NS_RELEASE here or
just delete explicitly.  I _suspect_ you want the former...

(My personal preference here would be to change the member variable into an
|nsCOMPtr<nsTransferableHookData> or even an
|nsCOMPtr<nsIClipboardDragDropHookList>| (since we never access this thing as an
nsTransferableHookData) and dispense with the manual memory-management, make
GetInterface just CallQI on the member or assign directly out of it, etc.)

Comment 21

[Simon Fraser [no longer active]]

Attachment: Patch to fix docshell hooks

Comment 22

[Kathleen :Brade]

Comment on attachment 120926 [details] [diff] [review]
Patch to fix docshell hooks

other than some whitespace differences, this is what I had

Comment 23

[Simon Fraser [no longer active]]

Checked in.

Comment 24

[Olivier Cahagne]

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

Comment 25

[Matthias Versen [:Matti]]

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

Comment 26

[Boris Zbarsky [:bzbarsky]]

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

Comment 27

[Jay Patel [:jay]]

Adding topcrash info for future reference.  Also, no crashes since 4/17
checkin... marking verified based on Talkback data.

Comment 28

[Jay Patel [:jay]]

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