82217 - Trunk [@ nsParseNewMailState::MoveIncorporatedMessage]

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect, P1)

Description

[greer]

A couple of these crashes showed up on 5/22/01 from the 2001501922 and 2022 
nightly builds. The stack looks like they may have had a problem with filters. 

nsParseNewMailState::MoveIncorporatedMessage
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsParseMailbox.cpp  line 1870]
         nsParseNewMailState::ApplyFilterHit
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsParseMailbox.cpp  line 1677]
         nsMsgFilterList::ApplyFiltersToHdr
[d:\builds\seamonkey\mozilla\mailnews\base\search\src\nsMsgFilterList.cpp  line 
159]
         nsParseNewMailState::ApplyFilters
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsParseMailbox.cpp  line 1585]
         nsParseNewMailState::PublishMsgHeader
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsParseMailbox.cpp  line 1494]
         nsPop3Sink::IncorporateComplete
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsPop3Sink.cpp  line 429]
         nsPop3Protocol::HandleLine
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsPop3Protocol.cpp  line 2402]
         nsMsgLineBuffer::ConvertAndSendBuffer
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgLineBuffer.cpp  line 218]
         nsMsgLineBuffer::BufferInput
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgLineBuffer.cpp  line 191]
         nsPop3Protocol::RetrResponse
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsPop3Protocol.cpp  line 2223]
         nsPop3Protocol::ProcessProtocolState
[d:\builds\seamonkey\mozilla\mailnews\local\src\nsPop3Protocol.cpp  line 2823]
         nsMsgProtocol::OnDataAvailable
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgProtocol.cpp  line 239]
         nsOnDataAvailableEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerProxy.cpp  line 
183]
         PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  
line 591]
         PL_ProcessPendingEvents        
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line
524]
         _md_EventReceiverProc  
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 1072]
         KERNEL32.DLL + 0x24407 (0xbff94407)
         0x00688b5e

Comment 1

[greer]

adding crash and top crash keywords for tracking and qawanted until a firm repro 
case is found.

Comment 2

[scottputterman]

Any ideas on what could be causing this?  There aren't many comments, though
someone mentions they were downloading a lot of message and have about 30 filters.

Comment 3

[lchiang]

Laurel or Esther - please see if you are able to reproduce this crash.  Take a
look at the Talkback reports if necessary and contact the folks who have
submitted a report for this crash to ask them any questions you may have.  
(I've done this before and it was fairly successful - everyone wants to help to
reproduce.)

Jay - can you send a link to us of this stack trace, showing the user comments
and the email address? That will help a lot.  Thanks.

Comment 4

[Navin Gupta]

From the stack trace it looks like we are crashing on this line. 
nsresult msgErr = destMailDB->CopyHdrFromExistingHdr(newMsgPos, mailHdr, 
PR_TRUE, &newHdr);

few lines above this line we assert if destMailDB is null. I think we should 
return also. 

Comment 5

[Håkan Waara]

Can we get some reproducable steps here?

Comment 6

[David :Bienvenu]

No, we shouldn't return - we should still do the move, but just not touch the
database - next time the user opens the folder, it will be reparsed, but the
move will have succeeded.

Comment 7

[Navin Gupta]

I just tried to reproduce this on my debug build from yesterday. I had 141 
messages in the pop3 test account and I set up 4 filters and I was able to 
download them successfully, all the filters worked correctly. 

Comment 8

[Navin Gupta]

Attachment: bullet proofing to do the db stuff if db

Comment 9

[David :Bienvenu]

Attachment: fix so that opening db fails when it should (i.e., db out of date or missing)

Comment 10

[David :Bienvenu]

I'm going to apply your patch, Navin. You should apply my patch and then try a
couple tests:

1. Delete the .msf file for a filter target folder and restart, filter mail to
that folder and make sure it works.

2. Instead of deleteing the .msf file "touch" the berkeley folder to change the
timestamp of the folder, so that opening the db will fail, restart, filter mail
to that folder, etc...

Comment 11

[David :Bienvenu]

And both patches should be checked in. your change looks fine, sr=bienvenu, but
I'm going to run it to be sure.

Comment 12

[David :Bienvenu]

the patches combined ran fine.

Comment 13

[Navin Gupta]

I have also tested the combined patches and it works for both db out of 
date and db missing. It should be pretty safe to check in. 

Comment 14

[scottputterman]

moving to 0.9.1

So instead of crashing the message counts will be messed up until the next time 
they load the folder?  That sounds like a good tradeoff.

Does this only affect filtering messages to a local folder? Are there other 
operations this affects and if so do you feel confident that this will not 
break them.  If the answer is that it won't break anything else then I say go 
for it.

Comment 15

[David :Bienvenu]

Yes, it only affects filtering to a local folder from a pop server.

Comment 16

[Navin Gupta]

hey, scott I want to check this in for mozilla 0.9.1. It will not affect 
anything else. seth, I need r/sr. 

Comment 17

[scottputterman]

I want you to get this in too.

Comment 18

[David :Bienvenu]

r=bienvenu

Comment 19

[Asa Dotzler [:asa]]

approved for checkin to 0.9.1 pending super review. 
--Asa (on behalf of drivers)

Comment 20

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

sr=sspitzer

Comment 21

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

ok, quick question:

are we doing the right thing in the 
other few places where we call OpenFolderDB()?

why was that 3rd param PR_TRUE before?  (cut and paste?)

we've had a few regressions lately, too close to 0.9.1

it would just be nice to have all our bases covered on this one.

Comment 22

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

one last question, do we need to test mbox folder migration?  either a 4.x pop 
account, or 4.x imap account [check the local folders after migration]

Comment 23

[Navin Gupta]

Here the OpenFolderDB is being called from within GetDatabaseWOReparse() which 
is used only in nsParseNewMailState::MoveIncorporatedMessage() so it will 
not affect anything else. 

Also the third param is about upgrading the db, making it PR_FALSE is correct
because the caller does not upgrade the db. 

Comment 24

[David :Bienvenu]

Yes, the other places calling OpenFolderDB are doing the right thing, and yes,
GetDatabaseWOReparse is doing the right thing now. I don't know if it was a cut
and paste error or just a plain old screwup. And Navin is right that this code
is only called during pop mail filtered to local folders so it is safe in that
respect.

Comment 25

[David :Bienvenu]

Navin, can you check this in or do you want me to?

Comment 26

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

thanks for answering my questions.  sr=sspitzer

Comment 27

[Navin Gupta]

fix checked in

Comment 28

[laurel]

OK using 2001-05--30-04 commercial trunk build win98.
Tested filtering from POP account to a user folder in Local Folders. Tried when
no msf file present for the target folder.  Tried after touching the target
folder file to create out-of-date situation between folder and msf file.  No
crashes. In both cases target folder unread count in folder pane incorrect until
opening folder again.

on to test other platforms...

Comment 29

[laurel]

OK, same results as win98 testing with:
2001-05-30-08 commercial trunk linux rh6.2
2001-05-30-05 commercial trunk mac OS 9.0