84544 - Trunk and Branch crash in [@ nsMsgComposeAndSend::GetDefaultPrompt ]

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P1)

Description

[Scott MacGregor]

I just noticed this is showing up as a top crasher on trunk builds. I didn't see
a bug already on file for this so I'm filing this one. 

I'll post the stack trace once I find some matching talk back IDs.

Comment 1

[Scott MacGregor]

People's comments make it sound like it's happening when they quit the app in
the middle of sending a message. We must be trying to throw up the dialog
warning them that we haven't sent the message yet. 

Example incident IDs:
 31452643
 31313954

Stack Trace:
nsMsgComposeAndSend::GetDefaultPrompt
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 229]
nsMsgComposeAndSend::Fail
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 3009]
nsMsgComposeAndSend::DoDeliveryExitProcessing
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 3054]
nsMsgComposeAndSend::DeliverAsMailExit
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 3112]
SendDeliveryCallback
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 2694]
nsMsgDeliveryListener::OnStopRunningUrl
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgDeliveryListener.cpp,
line 82]
nsUrlListenerManager::BroadcastChange
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsUrlListenerManager.cpp, line 84]
nsUrlListenerManager::OnStopRunningUrl
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsUrlListenerManager.cpp, line 113]
nsMsgMailNewsUrl::SetUrlState
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgMailNewsUrl.cpp, line 114]
nsMsgProtocol::OnStopRequest
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgProtocol.cpp, line 285]
nsSmtpProtocol::OnStopRequest
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsSmtpProtocol.cpp, line 352]
nsOnStopRequestEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsRequestObserverProxy.cpp, line 159]
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591]
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 524]
nsEventQueueImpl::ProcessPendingEvents
[d:\builds\seamonkey\mozilla\xpcom\threads\nsEventQueue.cpp, line 375] 

Comment 2

[scottputterman]

adding PDT+

Comment 3

[scottputterman]

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

Comment 4

[Håkan Waara]

(Re-adding comment from my dup. bug)

I had closed all browser windows, and all mail windows except one - the 
msgcompose window which was in the process of being sent. Now, when I closed 
that the whole program tried to shut down.

However, just as it had shut down the nsMsgComposeAndSend object did something 
strange and crashed.

It tried to use the mailsession, but the mailsession was already freed (all the 
windows was closed) so the nsMsgComposeAndSend object was alone. It tried to 
dereference the mailsession, which was null and I crashed.

There are two solutions to this problem:

1. We let the mailsession take down the nsMsgComposeAndSend object(s) upon 
destruction. Since this service is always alive, I think that's a good solution.
2. We nullcheck where I crashed to prevent others from crashing.

Comment 5

[Jean-Francois Ducarroz]

The solution 1 would be hard to do as there is no relation between mail and
mailcompose. I'll use solution 2...

Comment 6

[Jean-Francois Ducarroz]

Håkan, when you close the compose window, did you get an alert warming you that 
the message was currently beeing sent?

Comment 7

[Jean-Francois Ducarroz]

Attachment: proposed fix, V1

Comment 8

[Jean-Francois Ducarroz]

I am not able to reproduce the crash. However, I can clearly see where we crash. 
The fix, as proposed by Håkan is to test if mailsession is not null. In the case 
GetDefaultPrompt return a null prompt, we are still safe as the function that 
will use it (nsMsgDisplayMessageByString) will first try to get a promt from the 
windowsWatcher and if it failed to return one again will just not display the 
message. 

Comment 9

[Håkan Waara]

Yeah I did get a prompt, but when I canceled it and closed the last window
(which made the whole app quit down) it crashed. 

Comment 10

[Håkan Waara]

r=hwaara. but you might want to indent the stuff inside that new if clause.

Comment 11

[David :Bienvenu]

sr=bienvenu, as long as you indent as Hakan suggests.

Comment 12

[Scott MacGregor]

sr=mscott for the fix. hwaara, can you reliably reproduce the crash to the point
that you can verify that this fix actually works?

Comment 13

[Håkan Waara]

Yes. I managed to reproduce it just now when I tried again.

The testcase is:

1. Create a message with some email address in it (possibly your own).
2. Attach something big that will take time to send.
3. Close all windows except the compose one.
3. Hit send.
4. Click the closebutton in the window, and dismiss the dialog that comes up.

** Crash **

Comment 14

[Jean-Francois Ducarroz]

The code is correclty indented except that the line rv=msgWindow->... was using
4 space instead of 2. The diff add +/- in front of midified lines but doesn't
add spaces in front of other lines, that why it looks stange. Anyway, I've
converted all remaining tabs to space in this file.

Comment 15

[Jean-Francois Ducarroz]

Håkan, do you mind applying the patch and see if it really fix your problem. Thanks.

Comment 16

[Jean-Francois Ducarroz]

When you close the compose window while sending and dismiss the alert, we try to
cancel the smtp process but this is not working yet, this is part of bug
65209.Therefore the send process continue despite all windows are gone. Once fix
for bug 65209 is checked in, we should not see this problem anymore.

Comment 17

[Håkan Waara]

Sure, I'll do that today

Comment 18

[Håkan Waara]

Your fix does not fix the problem. It just reveals another crash (after some
assertions):

nsMsgFilePostHelper::OnStopRequest(nsMsgFilePostHelper * const 0x03723c50,
nsIRequest * 0x03723ec4, nsISupports * 0x00000000, unsigned int 2152398850) line
826 + 6 bytes
nsOnStopRequestEvent::HandleEvent() line 161
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x033e2ae4) line 64
PL_HandleEvent(PLEvent * 0x033e2ae4) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f708f0) line 520 + 9 bytes
nsEventQueueImpl::ProcessPendingEvents(nsEventQueueImpl * const 0x00f709c0) line
374 + 12 bytes
nsEventQueueImpl::ProcessPendingEvents(nsEventQueueImpl * const 0x037ae6c0) line 381
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 480
main(int 1, char * * 0x00a24ce0) line 1434 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! bff8b560()
KERNEL32! bff8b412()
KERNEL32! bff89dd5()

It's very easy to reproduce (just use my steps above), if you want somewhere to
debug from...

Comment 19

[Håkan Waara]

This is a P1, topcrasher, mozilla 0.9.2 bug. Is there any way I can help to get
some progress here?

Comment 20

[Jean-Francois Ducarroz]

I am still not able to reproduce the crash. Håkan, can you try the following:

? diff.txt
Index: nsMsgProtocol.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/base/util/nsMsgProtocol.cpp,v
retrieving revision 1.77
diff -u -2 -w -r1.77 nsMsgProtocol.cpp
--- nsMsgProtocol.cpp	2001/06/06 00:43:38	1.77
+++ nsMsgProtocol.cpp	2001/06/15 15:59:17
@@ -820,4 +820,6 @@
 NS_IMETHODIMP nsMsgFilePostHelper::OnStopRequest(nsIRequest * aChannel, 
nsISupports *ctxt, nsresult aStatus)
 {
+  if (!mProtInstance) return NS_OK; 
+
   if (!mSuspendedPostFileRead)
     mProtInstance->PostDataFinished();

Comment 21

[varada]

I was finally able to reproduce both crashes successfully and the checking for
the null in both mailsession(in nsMsgComposeAndSend::GetDefaultPrompt)  and
mProtInstance(in nsMsgFilePostHelper::OnStopRequest) seems to take care of the
crashes.
r=varada

Comment 22

[Håkan Waara]

Ducarroz, is this a "real" fix or just hiding it?

Comment 23

[Jean-Francois Ducarroz]

This is the real fix. But when we will be able to cancel a smtp process, we
should not see this scenario anymore!

Comment 24

[Håkan Waara]

r=hwaara then

Comment 25

[Scott MacGregor]

sr=mscott for both the patch posed and the patch pasted by JF.

Comment 26

[Scott MacGregor]

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

Comment 27

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

In my tree, in which I have a bunch of leak fixes (bug 81289) that I was hoping 
to check in soon, I'm seeing a crash at the following stack in any browser 
session where I send a mail message.  (Note that the cause of the crash is a 
null result of do_GetService since you can't call GetService during XPCOM 
shutdown.)

nsDebug::PreCondition(const char * 0x04e3da04 `string', const char * 0x04e3da40 
`string', const char * 0x04e3da50 `string', int 649) line 434 + 21 bytes
nsCOMPtr<nsIMsgMailSession>::operator->() line 649 + 34 bytes
nsMsgComposeAndSend::GetDefaultPrompt(nsMsgComposeAndSend * const 0x058f1710, 
nsIPrompt * * 0x0012fcc4) line 227 + 32 bytes
nsMsgComposeAndSend::NotifyListenerOnStopCopy(nsMsgComposeAndSend * const 
0x058f1710, unsigned int 2147500037) line 3360 + 36 bytes
CopyListener::OnStopCopy(CopyListener * const 0x05b31280, unsigned int 
2147500037) line 143
nsMsgCopyService::ClearRequest(nsCopyRequest * 0x05b31220, unsigned int 
2147500037) line 162
nsMsgCopyService::~nsMsgCopyService() line 145
nsMsgCopyService::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsMsgCopyService::Release(nsMsgCopyService * const 0x05b31930) line 286 + 132 
bytes
DeleteEntry(nsHashKey * 0x05b31310, void * 0x05b318f0, void * 0x00000000) line 
258 + 18 bytes
_hashEnumerateRemove(PLHashEntry * 0x05b312d0, int 26, void * 0x0012fdd0) line 
369 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x00485e08, int (PLHashEntry *, int, 
void *)* 0x10027710 _hashEnumerateRemove(PLHashEntry *, int, void *), void * 
0x0012fdd0) line 413 + 15 bytes
nsHashtable::Reset(int (nsHashKey *, void *, void *)* 0x1005fb80 DeleteEntry
(nsHashKey *, void *, void *), void * 0x00000000) line 385 + 21 bytes
nsObjectHashtable::Reset() line 628
nsObjectHashtable::~nsObjectHashtable() line 594
nsObjectHashtable::`vector deleting destructor'(unsigned int 1) + 81 bytes
nsServiceManagerImpl::~nsServiceManagerImpl() line 283 + 33 bytes
nsServiceManagerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsServiceManagerImpl::Release(nsServiceManagerImpl * const 0x00484030) line 292 
+ 152 bytes
nsServiceManager::ShutdownGlobalServiceManager(nsIServiceManager * * 
0x00000000) line 544 + 17 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 476 + 7 bytes
main(int 2, char * * 0x00484210) line 1445 + 8 bytes

Is that the same crash that was happening here?  (It seems like the same line 
of code, except it looks like some of the stacks have event queue stuff on 
them.)  Would it be fixed by the patches here?  Is it likely to be related to 
my leak fixes?

Comment 28

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 29

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

/me realizes danm was seeing the same thing, so it's not my leak fix...

Comment 30

[Jean-Francois Ducarroz]

Fixed and checked in. Thanks everybody for your help.

Comment 31

[Scott MacGregor]

I'm worried about this bug again. I'm not starting to see this on normal
sessions of quitting the app. That is, when there isn't a send in progress.  I
think someone else tried to say that in here but I didn't think anything of it.
Now in the last day or so I'm hitting it all the time. 

I'm pretty sure my messages are being sent so why would we still be trying to
bring up this alert dialog? We're talking 5 or 6 minutes after i sent the
message,  I ended up quitting the app and I'm triggering this crash. Do we have
an understanding about why that would be happening?

Comment 32

[Sheela Ravindran]

I have seen the crash on the 18th trunk builds.
verified on the builds that on exit and dismissing the dlg that comes up while 
sending in progress did not cause any crash. There are no more crash reports 
that I see in the talkback for nsMsgComposeAndSend.  Please reopen the bug if 
you see the same crash on the newer builds.

verified using builds:  2001062111 -linux, 2001062106 win98, 2001062104 mac