89150 - [regression] sending each unsent messages multiple times when sending unsent messages

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P1)

Description

[(not reading, please use [e-mail] instead)]

this regression is caused by monahb's checkin for bug #79865

when sending unsent messages, you'll get assertions.

the problem is SendFolderUnsentMessages() iterates over all identities and 
calls messenger.SendUnsentMessages(identity) for each identity.

the problem is all identities are sharing the same unsent messages folder.

so we end up sending the unsent messages multiple times!  (once per identity!)

here's the assertion:

NTDLL! 77f9f9df()
nsDebug::Assertion(const char * 0x03388838, const char * 0x03388818, const char 
* 0x033887e0, int 948) line 290 + 13 bytes
nsMsgThread::ChangeChildCount(int -1) line 948 + 38 bytes
nsMsgThread::RemoveChildHdr(nsMsgThread * const 0x05a6daf0, nsIMsgDBHdr * 
0x06ed9610, nsIDBChangeAnnouncer * 0x04574820) line 513
nsMsgDatabase::RemoveHeaderFromThread(nsMsgHdr * 0x06ed9610) line 1468 + 36 
bytes
nsMsgDatabase::DeleteHeader(nsMsgDatabase * const 0x04574820, nsIMsgDBHdr * 
0x06ed9610, nsIDBChangeListener * 0x00000000, int 0, int 1) line 1433
nsMsgLocalMailFolder::DeleteMessage(nsISupports * 0x06ed9610, nsIMsgWindow * 
0x00000000, int 1, int 0) line 1998 + 49 bytes
nsMsgLocalMailFolder::DeleteMessages(nsMsgLocalMailFolder * const 0x064d45fc, 
nsISupportsArray * 0x05a6ad50, nsIMsgWindow * 0x00000000, int 1, int 0, 
nsIMsgCopyServiceListener * 0x00000000, int 0) line 1493
nsMsgSendLater::DeleteCurrentMessage() line 707 + 48 bytes
SendOperationListener::OnStopSending(SendOperationListener * const 0x06ed3950, 
const char * 0x00000000, unsigned int 0, const unsigned short * 0x00000000, 
nsIFileSpec * 0x00000000) line 410
nsMsgComposeAndSend::NotifyListenerOnStopSending(nsMsgComposeAndSend * const 
0x06ed1ed0, const char * 0x00000000, unsigned int 0, const unsigned short * 
0x00000000, nsIFileSpec * 0x00000000) line 3281
nsMsgComposeAndSend::DoDeliveryExitProcessing(nsIURI * 0x06ed0c24, unsigned int 
0, int 0) line 3175
nsMsgComposeAndSend::DeliverAsMailExit(nsMsgComposeAndSend * const 0x06ed1ed0, 
nsIURI * 0x06ed0c24, unsigned int 0) line 3195
SendDeliveryCallback(nsIURI * 0x06ed0c24, unsigned int 0, nsMsgDeliveryType 
nsMailDelivery, nsISupports * 0x06ed1ed0) line 2776
nsMsgDeliveryListener::OnStopRunningUrl(nsMsgDeliveryListener * const 
0x06ed2070, nsIURI * 0x06ed0c24, unsigned int 0) line 81 + 33 bytes
nsUrlListenerManager::BroadcastChange(nsIURI * 0x06ed0c24, nsUrlNotifyType 
nsUrlNotifyStopRunning, unsigned int 0) line 97
nsUrlListenerManager::OnStopRunningUrl(nsUrlListenerManager * const 0x06ed0be0, 
nsIMsgMailNewsUrl * 0x06ed0c24, unsigned int 0) line 110 + 18 bytes
nsMsgMailNewsUrl::SetUrlState(nsMsgMailNewsUrl * const 0x06ed0c24, int 0, 
unsigned int 0) line 114
nsSmtpProtocol::ProcessProtocolState(nsIURI * 0x06ed0c24, nsIInputStream * 
0x06ed34a0, unsigned int 393, unsigned int 46) line 1451
nsMsgProtocol::OnDataAvailable(nsMsgProtocol * const 0x06ed02a0, nsIRequest * 
0x06ed32d0, nsISupports * 0x06ed0c24, nsIInputStream * 0x06ed34a0, unsigned int 
393, unsigned int 46) line 243 + 32 bytes
nsOnDataAvailableEvent::HandleEvent() line 175 + 70 bytes
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x05a6ea44) line 64
PL_HandleEvent(PLEvent * 0x05a6ea44) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00501470) line 520 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x0007011a, unsigned int 49390, unsigned int 0, 
long 5248112) line 1071 + 9 bytes
USER32! 77e13eb0()
USER32! 77e1401a()
USER32! 77e192da()
nsAppShellService::Run(nsAppShellService * const 0x040eb4e0) line 419
main1(int 2, char * * 0x00484190, nsISupports * 0x00000000) line 1174 + 32 bytes
main(int 2, char * * 0x00484190) line 1478 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e8790

on a related note, we should remove SendUnsentMessages() from mailCommands.js, 
it is no longer used.

126 function SendUnsentMessages(folder)
127 {
128         if(folder) {    
129                 var identity = getIdentityForServer(folder.server);
130                 messenger.SendUnsentMessages(identity);
131         }
132 }

Comment 1

[(not reading, please use [e-mail] instead)]

I'm working on a fix.

Comment 2

[(not reading, please use [e-mail] instead)]

a simple fix is to break out of the loop and only call 
messenger.SendUnsentMessages() once.

I'll attach a patch.

Comment 3

[Håkan Waara]

Looks like you forgot to attach it. ;)

Comment 4

[(not reading, please use [e-mail] instead)]

Attachment: patch, until we fix this the right way (not sure what that is yet)

Comment 5

[grylchan]

Seth question for you.

I am using 
Commercial builds
2001-07-05-09-trunk/ win nt 4.0
2001-07-05-08-trunk/ linux 2.2, red hat 7.0
2001-07-05-08-trunk/ mac os 9.0.4

I just want to confirm my understanding.

If i have multiple mail accounts and i send an unsent
message from the unsent folder, I should get multiple
copies of that message based on the number of mail
accounts? (ie if I have 3 mail accounts, then I should
receive the unsent message 3 times).

If this is correct, it's hard to replicate on windows/mac.
It happened pretty fast on linux. Maybe it just takes
longer on windows/mac. Anyways just wanted to make
sure I understood what the bug did. Thanks in advance.

Comment 6

[(not reading, please use [e-mail] instead)]

gchan, with three accounts you should be seeing 3 copies of each message in 
your unsent messages folder (when you send them.)

I'm definitely seeing this problem on the trunk.

try having a bunch of messages in your unsent messages folder.

my connection to my smtp server might be slower than yours.  I think "send 
unsent messages" happens asynchronously.

so if we call messenger.sendUnsentMessages() again before the messages have 
been sent, you'll see this problem.  if the messages have already been sent, 
you won't see the problem.

Comment 7

[(not reading, please use [e-mail] instead)]

I'll work on landing this today.  the real problem will be covered as part of 
bug #88931

Comment 8

[varada]

r=varada

Comment 9

[Scott MacGregor]

sr=mscott

is monhab's checkin on the branch? If it's not on the branch then we don't need
this fix for the branch right? so no need for a nsbranch keyword. 

Looking at monhab's bug it looks like it has not been checked into the branch.
Given the size of that patch I highly doubt it's going to make into the branch
now.  so this bug probably doesn't need to go into the branch.

Comment 10

[(not reading, please use [e-mail] instead)]

I agree, neither should go on the branch.

I've removed nsBranch from both bugs.

too risky / too big of a change.  

I'll land my fix on the trunk later today.

Comment 11

[(not reading, please use [e-mail] instead)]

fixed on the trunk.

Comment 12

[Sheela Ravindran]

I used 2001-07-06-06 build and did see the problem with each message received 
multiple times in all the different accounts.

Using build below verify that this problem is fixed now.

2001-08-13-06 win98
2001-08-13-04 mac
2001-08-13-06 linux

Comment 13

[Günter]

This seems to be a very OLDIE !!
Just come over this because I'm going to use messenger.sendUnsentMessages() with an extension.

Any plan to change the behavior of the "unsent messages folder"? 
With the posting of: "on a related note, we should remove SendUnsentMessages() from mailCommands.js, it is no longer used."
7 years ago it sounds there will be some change / removal with this function.

With current Tb 2.0.0.14 I'm seeing for each account an "unsent messages folder", but AFAIKS only the one with the "Local Folder" is used.

Will this be true with further TB revisions?