355333 - [IMAP] Crash when moving or deleting messages [@ nsImapProtocol::HandleMessageDownLoadLine]

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Adam Guthrie]

This is the #8 topcrash for Thunderbird 1.5.0.7. Many of the comments say the crash occurred when deleting a message.

The Linux stacks have bogus line numbers, so here's a Windows stack:

Incident ID: 24102405
Stack Signature	nsImapProtocol::HandleMessageDownLoadLine 999652e8
Product ID	Thunderbird15
Build ID	2006090918
Trigger Time	2006-10-03 19:43:50.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	thunderbird.exe + (004c45e2)
URL visited	
User Comments	detaching attachment from message in inbox
Since Last Crash	497855 sec
Total Uptime	497855 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Tb-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 3384
Stack Trace 	
nsImapProtocol::HandleMessageDownLoadLine  [mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 3384]
nsImapServerResponseParser::msg_fetch_literal  [mozilla/mailnews/imap/src/nsImapServerResponseParser.cpp, line 2739]
nsImapServerResponseParser::msg_fetch_content  [mozilla/mailnews/imap/src/nsImapServerResponseParser.cpp, line 2059]
nsImapServerResponseParser::msg_fetch_headers  [mozilla/mailnews/imap/src/nsImapServerResponseParser.cpp, line 2029]
nsImapServerResponseParser::ParseIMAPServerResponse  [mozilla/mailnews/imap/src/nsImapServerResponseParser.cpp, line 246]
nsImapProtocol::FetchMessage  [mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 3053]
nsImapProtocol::FolderMsgDumpLoop  [mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 3771]
nsImapProtocol::FolderMsgDump  [mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 3672]
nsImapProtocol::ProcessSelectedStateURL  [mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 2037]
nsImapProtocol::ProcessCurrentURL  [mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 1420]
nsImapProtocol::ImapThreadMainLoop  [mozilla/mailnews/imap/src/nsImapProtocol.cpp, line 1142]

Comment 1

[Russell Howe]

I've seen crashes here on Thunderbird 1.0.x, 1.5.x and 2.0 beta 2.

All on win32, all within a Citrix Metaframe XP session and all for one particular user.

See talkback incident TB28944338M for a recent example

Comment 2

[Wayne Mery (:wsmwk)]

#3 crasher for 1.5.0.9

most mention deleting of some form as mentioned in comment 0.
but some also are "sending email" and typing email.

Comment 3

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

This is also a top 5 crasher for Thunderbird 2.0 and mainly on Windows. Perhaps this should be marked as topcrasher+ because there are at least more than 500 crash reports within the last month.

http://talkback-public.mozilla.org/reports/thunderbird/TB20/TB20-topcrashers.html

Comment 4

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

This mostly appears when moving or deleting messages on an IMAP account => adjusting summary.

Comment 5

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

I was running for myself into this crash yesterday while repeatedly deleting old mails within my sent folder without moving them to trash. I set sorting by size and started with the largest messages first. The stack is the same like above: TB38982946W

Comment 6

[Wayne Mery (:wsmwk)]

odd ... for v1.5.x builds newer than 1.5.0.10 this is WAY down in the ranking (#66). But TB 2.x is holding steady at #6.  Since there is no "topcrash team", I'm setting topcrash+

FWIW, bug 255581 addressed this top of stack a long time ago

Comment 7

[Magnus Melin [:mkmelin]]

The crash leads here: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mailnews/imap/src/nsImapProtocol.cpp&mark=3474&rev=MOZILLA_1_8_BRANCH#3474

is m_curHdrInfo null? It looks like it could be in some error situation.

Comment 8

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Comment 9

[David :Bienvenu]

Comment on attachment 295807 [details] [diff] [review]
proposed fix

I'd love to know why m_curHdrInfo is null, though.

Comment 10

[Wayne Mery (:wsmwk)]

(In reply to comment #9)
> (From update of attachment 295807 [details] [diff] [review])
> I'd love to know why m_curHdrInfo is null, though.

should we file a new bug on that? 
indeed, m_curHdrInfo seems to like being null (bug 350179 comment 3).
could this be related?

note: there is one hit on crash-stats bp-f0620026-b1ae-11dc-818d-001a4bd43ed6

Comment 11

[Magnus Melin [:mkmelin]]

(In reply to comment #9)
> I'd love to know why m_curHdrInfo is null, though.

Currently we have
    if (!m_curHdrInfo)
      BeginMessageDownLoad(....);
    m_curHdrInfo->CacheLine(....);

At least in the sequence BeginMessageDownLoad -> StartNewHdr -> GetFreeHeaderInfo

... GetFreeHeaderInfo() can get get into error situations and cause m_curHdrInfo not to get set up. 

Comment 12

[David :Bienvenu]

Right, thx, yes, that's the immediate cause, but I meant how do those error conditions arise? I've stared at this code a long time over the past couple years, and pretty much convinced myself that the underlying cause is mismatched calls to begin and end message download, and fixed a few possible causes of that, but I think there must be some remaining cases not handled. I'd love to understand it and bulletproof in a more robust way - maybe this is a case where more eyes will help.

Comment 13

[Magnus Melin [:mkmelin]]

Yeah, that may be harder to figure out. Besides the out of memory situation of course.

Checking in mailnews/imap/src/nsImapProtocol.cpp;
/cvsroot/mozilla/mailnews/imap/src/nsImapProtocol.cpp,v  <--  nsImapProtocol.cpp
new revision: 1.674; previous revision: 1.673
done

->FIXED (fingers crossed)

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 295807 [details] [diff] [review]
proposed fix

Asking branch approval for this top crasher.

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 295807 [details] [diff] [review]
proposed fix

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 16

[Magnus Melin [:mkmelin]]

MOZILLA_1_8_BRANCH
Checking in mailnews/imap/src/nsImapProtocol.cpp;
/cvsroot/mozilla/mailnews/imap/src/nsImapProtocol.cpp,v  <--  nsImapProtocol.cpp
new revision: 1.612.2.27; previous revision: 1.612.2.26
done

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Looking at the above, there does not seem to be any specific repro scenario for this. Is there one or should be just monitor reports to see if the crash is no longer reported as a topcrash after the fact?

Comment 18

[Russell Howe]

For what it's worth, one of our users was experiencing this crash several times a day, and since I moved him onto the 20080117 nightly build of Thunderbird, the crash has not reoccurred.

Comment 19

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

Russel, do you mean trunk or 1.8 branch nightly build? If it's 1.8 branch we could mark this verified.

Also Talkback and Socorro don't report a crash with a build id newer as the check-in for trunk and 1.8 nightly builds.

Comment 20

[Russell Howe]

Hm, not sure. I think trunk - the about box reads

"version 3.0a1pre (2008011703)"

I'll find the 1.8 nightly and give that a try.

Comment 21

[Al Billings [:abillings - ex-MoCo]]

This should be verified in ftp://ftp.mozilla.org/pub/thunderbird/nightly/2.0.0.12-candidates/rc1/, not a nightly, since that is the build we are planning on releasing for Thunderbird 2.0.0.12. The nightly may have other changes since it is from after we forked.

Comment 22

[Al Billings [:abillings - ex-MoCo]]

Any luck, Russell?

Comment 23

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

Lets mark it verified for 1.9. No reports listed within the last months.