Trunk [@ nsParseNewMailState::MoveIncorporatedMessage]

VERIFIED FIXED in mozilla0.9.1

Status

MailNews Core
Filters
P1
major
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: greer, Assigned: Navin Gupta)

Tracking

({crash, topcrash})

Trunk
mozilla0.9.1
x86
Windows NT
crash, topcrash

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta1+] ready to checkin as soon as the tree opens, crash signature)

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
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
(Reporter)

Comment 1

17 years ago
adding crash and top crash keywords for tracking and qawanted until a firm repro 
case is found.
Keywords: crash, qawanted, topcrash

Comment 2

17 years ago
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

17 years ago
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.
Severity: normal → major
(Assignee)

Comment 4

17 years ago
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

17 years ago
Can we get some reproducable steps here?

Comment 6

17 years ago
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.
(Assignee)

Comment 7

17 years ago
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. 
(Assignee)

Comment 8

17 years ago
Created attachment 35928 [details] [diff] [review]
bullet proofing to do the db stuff if db

Comment 9

17 years ago
Created attachment 35930 [details] [diff] [review]
fix so that opening db fails when it should (i.e., db out of date or missing)

Comment 10

17 years ago
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

17 years ago
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

17 years ago
the patches combined ran fine.
(Assignee)

Comment 13

17 years ago
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

17 years ago
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.
Keywords: nsbeta1
Priority: -- → P1
Whiteboard: [nsbeta1+]
Target Milestone: --- → mozilla0.9.1

Comment 15

17 years ago
Yes, it only affects filtering to a local folder from a pop server.
(Assignee)

Comment 16

17 years ago
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

17 years ago
I want you to get this in too.

Comment 18

17 years ago
r=bienvenu

Comment 19

17 years ago
approved for checkin to 0.9.1 pending super review. 
--Asa (on behalf of drivers)
sr=sspitzer
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.
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]
(Assignee)

Comment 23

17 years ago
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

17 years ago
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

17 years ago
Navin, can you check this in or do you want me to?
thanks for answering my questions.  sr=sspitzer
(Assignee)

Updated

17 years ago
Whiteboard: [nsbeta1+] → [nsbeta1+] ready to checkin as soon as the tree opens
(Assignee)

Comment 27

17 years ago
fix checked in
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 28

17 years ago
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

17 years ago
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
Status: RESOLVED → VERIFIED

Updated

17 years ago
Keywords: qawanted
Product: MailNews → Core
Product: Core → MailNews Core
Crash Signature: [@ nsParseNewMailState::MoveIncorporatedMessage]
You need to log in before you can comment on or make changes to this bug.