284876 - Trunk TB10 crash while sending mail [@ nsMsgLocalMailFolder::WriteStartOfNewMessage() ]

Status: VERIFIED FIXED

(MailNews Core :: Database, defect)

Description

[Adam Hauner]

2005030105/Seamonkey-trunk/W2K

I met this crash while sending pretty blank message (blank unsubscribe request,
no subject, no body).

TB4127709W:
nsMsgLocalMailFolder::WriteStartOfNewMessage 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/mailnews/local/src/nsLocalMailFolder.cpp,
line 2187]
nsMsgLocalMailFolder::StartMessage 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/mailnews/local/src/nsLocalMailFolder.cpp,
line 2652]
nsInputStreamPump::OnStateStop 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/netwerk/base/src/nsInputStreamPump.cpp,
line 507]
0x03dca920
nsMailboxProtocol::`vftable'
nsMsgProtocol::AddRef
0x25ff6081

There are about 40 incidents of Thunderbird and Suite with Win32, Linux and Mac
OS X builds: http://tinyurl.com/4u7mc

Comment 1

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 2

[Gervase Markham [:gerv]]

This bug has been automatically resolved after a period of inactivity (see above
comment). If anyone thinks this is incorrect, they should feel free to reopen it.

Comment 3

[timeless]

Incident ID: 16479397
Stack Signature	nsMsgLocalMailFolder::WriteStartOfNewMessage 7a0ecc8b
Product ID	Thunderbird15
Build ID	2005120115
Trigger Time	2006-03-16 21:59:58.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	thunderbird.exe + (004f5140)
URL visited	
User Comments	just woke up from hibernate mode on laptop
Since Last Crash	3188550 sec
Total Uptime	3188550 sec
Trigger Reason	Access violation
Source File, Line No.	e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/mailnews/local/src/nsLocalMailFolder.cpp, line 2261
Stack Trace 	
nsMsgLocalMailFolder::WriteStartOfNewMessage  [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/mailnews/local/src/nsLocalMailFolder.cpp, line 2261]
nsMsgLocalMailFolder::StartMessage  [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/mailnews/local/src/nsLocalMailFolder.cpp, line 2730]
XPTC_InvokeByIndex  [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102]
EventHandler  [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/xpcom/proxy/src/nsProxyEvent.cpp, line 562]
0x778b0c24
nsWebBrowserPersist::OnProgress  [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp, line 940]
0x1974c085

Comment 4

[timeless]

Attachment: spackle with complaints about code

Comment 5

[David :Bienvenu]

Comment on attachment 219114 [details] [diff] [review]
spackle with complaints about code

I doubt this will compile - did you mean mCopyState?

Comment 6

[timeless]

yes i did. but it's spackle i don't actually want to see committed. note that i have no compiler and as of tomorrow morning no computers for a while. and when i have a place to live, i will have no time or interest in anything remotely resembling mailnews. heck, i won't even have an interest in xul. i'm sorry i typod mCopyState as nCopyState.

i'm still **** that these bugs were resolved EXPIRED.

Comment 7

[timeless]

Attachment: spackle with complaints about code without the typo

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Timeless, any news about this one? We have to other bugs with a similar top frame: bug 293978, bug 361311.

Comment 9

[timeless]

why should their be news? i still am not working on xul or mailnews. I'm not the assignee, but if I were a random contributor of a one off patch, I can assure you I wouldn't be pleased. please find someone else to complain to the reviewer about failing to review or address the bug.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

David, the latest patch has bitrotted for over a year now. Were there any big changes to that file and we don't need it anymore or is it still needed?

Comment 13

[timeless]

[e-mail]: fwiw, the patch merges without issue:
timeless@boffo:/tmp/_world$ mkdir 284876; cp -r q/CVS 284876; cd 284876
timeless@boffo:/tmp/_world/284876$ wget --no-check-certificate -Oa 'https://bugzilla.mozilla.org/attachment.cgi?id=219243'
timeless@boffo:/tmp/_world/284876$ mergePcvs.pl a; patch -d `pdir.pl a` -p0 < a
timeless@boffo:/tmp/_world/284876$ cvs up `cvsu --types=MUAR --find`


http://viper.haque.net/~timeless/patchtools/

Comment 14

[Wayne Mery (:wsmwk)]

does this problem and what the "patch: fixes extend beyond *sending* of mail? 

The reason I ask is I've examined the talkback incidents with this top of stack and only 5 comments mention send or sent. And none are thunderbird 2 -  all are thunderbird 1.5. (perhaps we will see some for thunderbird 2 once the auto update is thrown?)

~150 incidents have comments, of which 73 mention startup/opening thunderbird.

38 of the 150 are version 2, of which 20 mention startup/opening thunderbird.
the remainder of the v2 comments are:

Checking emails
receiving mail
??recevoir mes mails
catching mails. Nothing special???
collecting mails from the server when it crashed
delete mails marked as junk
Deleting email
downloading and viewing mail
downloading emails
downloading imap mail
downloading mail accounts
I just receive e-mails
move email
moving a shit load of mail, around 12 gig, I work security, so there theres a lot of viruses and spam.
Picking up massive amounts of spam headers only from one of my accounts at 30000 or so messages when it crashed. 
Powering up after hibernation.(winXP)
reading incoming mail, classifying junk
reading rss feeds

Comment 15

[timeless]

the code in question is used by anything that generates an entry in a local folder, sending and receiving both would.

Comment 16

[Wayne Mery (:wsmwk)]

topcrash - ~80 crashes in 10 days, which puts this in the top 25 for TB 2.0.0.6**
examples:

TB37850367 start, "get email" ... Some email might have been sent too which had been composed offline
TB7748805 Specified that I wished to delete files marked as junk (400+) while simultaneously reading a second, pop3 account
TB37543095 Clicked to open thunderbird
TB37439068 TB37288786 had just put in password


** sure would be nice if talkback did a better job of ranking these crashes.  does breakpad do any better?

Comment 17

[Wayne Mery (:wsmwk)]

- reporter Adam writes "No [never see the crash again] but I'm several years unable to trace SeaMonkey [branch] crashes because missing TalkBack in its builds."
- Alexey "No, I am using Trunk any more"

anyone who sees this should try alpha release http://www.mozillamessaging.com/en-US/thunderbird/early_releases/

still topcrash - #32 crash for 2.0.0.12. But trunk has no crashes which match either of first two stacks listed in this bug 

Comment 18

[Wayne Mery (:wsmwk)]

bp-7bdb47ae-4778-11dd-a84f-001a4bd43ef6 is close match (my "unattended" crash)

bp-a8772cba-4188-11dd-8f3f-001cc45a2c28 looks to be a crash on startup

Comment 19

[Wayne Mery (:wsmwk)]

(In reply to comment #18)
> bp-7bdb47ae-4778-11dd-a84f-001a4bd43ef6 is close match (my "unattended" crash)

TB20019 topcrash still, possibly 3.0b2 also, and has a draft patch, requesting blocking-thunderbird3 for 
a) decision on whether this is same as current trunk crash(s)
b) bienvenu comment on where to go with the draft patch

quoting bienvenu bug 391357 comment 7 "we can easily check for that [mCopyState is null], but figuring out why it's null, and how to really handle it correctly is more difficult."

bp-5f64fa47-92eb-4dcc-95f0-c51fc2090307 and bp-e26e2235-7b7d-4f6b-a43d-7621e2090309 are 3.0b2 examples

"While TB was opening, moused-over one of my inboxes to see what the new messages were; tooltip appeared (listing about 7 new messages), but then TB crashed. (no clicks involved)need some and 3.0b2"

Comment 20

[Kent James (:rkent)]

I checked about 8 crash reports, and the all come through the following routine:

nsImapMailFolder::StartMessage(nsIMsgMailNewsUrl * aUrl)

One suggestion would be to add a stupid fix (one where we don't understand the underlying cause) with a check for the existence of mCopyState for the target at that point, or (slightly easier) at

NS_IMETHODIMP nsMsgLocalMailFolder::StartMessage()

I'm guessing (though bienvenu would be a better guesser) that this is related to IMAP threading. We would just be changing one failure for another though, that is a copy somewhere will end up failing instead of the program crashing - but surely that is a better result?

I would be happy to do the little patch, with bienvenu as r/sr, if bienvenu wants it.

Comment 21

[David :Bienvenu]

I doubt imap threading is involved, since all this happens on the ui thread, and we block both threads when calling from the imap thread. I suppose it's possible that the copy state might get cleared on the UI thread after the imap thread checks it, but I don't see offhand what would cause that. Also, in theory, we should not be having two operations copying messages into the same destination folder, since both folder locking and the copy service conspire to prevent that.

If we don't understand this, the failure is likely to be that you can't move/copy messages until you shutdown and restart, which is painful enough that crashing might seem merciful in comparison.

Looking at the code path, it would seem that nsMsgLocalMailFolder::OnCopyCompleted must be getting called prematurely somehow. I'll try to think about how that could happen. In any case, I think adding a check in nsMsgLocalMailFolder::StartMessage would be the right band-aid.

Comment 22

[David :Bienvenu]

when copying to a local folder the usual sequence is that nsLocalMailFolder::CopyMessages is called on the dest folder (generally by the copy service). That initializes the copy state and then calls nsLocalMailFolder::CopyMessage(s)To. In the single message case, we then call 
    rv = mCopyState->m_messageService->CopyMessage(uri.get(), streamListener, isMove, nsnull, aMsgWindow, nsnull);

which streams the message to the the local folder via our copyStreamListener. We never get here unless we've created the copy state correctly.

The multiple message case is very similar, except that we call the msg service's CopyMessages method. In that case, we do have to call StartMessage for each message being copied. I suppose it's possible that somehow the copy could fail between one message and an other, causing the copy state to get cleared, but not aborting the url, so we still get data from the imap server, and call StartMessage between messages. It seems unlikely that all these users could be having multiple messages moved/copied to local folders from imap, though.

Comment 23

[Kent James (:rkent)]

Attachment: stupid fix

Many of the comments mention startup. I routinely have all sorts of interesting things go wrong on startup (not related to this bug.) I believe that the stress of starting lots of things, and downloading lots of things, causes various poorly handled errors. I suspect that this current crash occurs when some error is not fully handled correctly, and startup is the most stressful time for errors.

I promised a stupid patch and here it is. I am unhappy though with the simple NS_ENSURE_ARG_POINTER error response. The return value is ignored upstream. Ultimately it should be handled in nsIMapProtocol.cpp, unfortunately on the IMAP thread as I understand things:

http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapProtocol.cpp#2894

  m_imapMailFolderSink->StartMessage(mailurl);

Bienvenu, how can we propagate this error upstream to some point that we can actually take some sort of reasonable action?

Comment 24

[David :Bienvenu]

Comment on attachment 369753 [details] [diff] [review]
stupid fix

I'd have to think about how to propagate the error, but I'm not sure what the imap protocol code would do differently in this case...

Comment 25

[Kent James (:rkent)]

Let's check this in to see what it does to crash stats.

Comment 26

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/344da6272b01

Do we want to resolve this as fixed for the time being?

Comment 27

[Kent James (:rkent)]

Sorry, didn't notice the checkin or the comment.

Let's leave it open, that will help remind me to check tinderbox for any improvements in the crash stats. Plus I'm still a little nervous about side-effects, as this was a stupid fix (treating the symptom without understanding the cause).

Comment 28

[Kent James (:rkent)]

The frequency at which this was occurring in prebeta3 TB was about every 5 days, which is too infrequent to have any hope of seeing secondary effects of fixing this above the noise. So I'm going to mark this as fixed, as there is no point to continuing to be looking for secondary effects.

Comment 30

[Wayne Mery (:wsmwk)]

This is a topcrash for TB20022 and seems to be a solid fix with no regressions, so nominating for 1.8.1.next

v.fixed - crashes gone from crash-stats after 2009-03-30  (a good many had been eudora crashes), so thanks rkent

a couple are bsmedberg's 
 bp-3485a62d-5963-4816-ba30-3b6902090520
 bp-8fcde747-c1d2-45db-8136-3f34c2090712

a couple more examples with comments:
 bp-2f7a8ac5-dc94-4040-a2af-18b2e2090402 start up TB or go back online after being offline -- it's loading messages successfully (and showing the popups for new messages) when it crashes.
 bp-3ca1eee8-7483-4950-8cec-505a32090330 just returning online

Comment 31

[Wayne Mery (:wsmwk)]

rkent, bienvenu, can you supply a patch for v2?  

#10 crash for TB20023, and dveditz has approved blocking (but needed for 8.0.next, not 8.1.next)

Comment 32

[Wayne Mery (:wsmwk)]

er, i was right the first time - 1.8.1.next is what is wanted.

note: there is a not null crash on v3 that was not fixed by this bug. filed as  Bug 540191 - crash [@ nsMsgLocalMailFolder::WriteStartOfNewMessage()]

Comment 33

[Kent James (:rkent)]

The stupid fix in the patch is fairly trivial to do, but my issue is that I am not currently organized to build on the TB2 branch. Perhaps bienvenu could do this?

Comment 34

[Daniel Veditz [:dveditz]]

Comment on attachment 369753 [details] [diff] [review]
stupid fix

You need to request branch approval on patches to get noticed. Assuming you meant to approved for 1.8.1.24, a=dveditz

Comment 35

[Daniel Veditz [:dveditz]]

Checking in mailnews/local/src/nsLocalMailFolder.cpp;
/cvsroot/mozilla/mailnews/local/src/nsLocalMailFolder.cpp,v  <-- nsLocalMailFolder.cpp
new revision: 1.506.2.30; previous revision: 1.506.2.29