125503 - Interrupted and resumed pop3 download results in incorrect data in summary file

Status: VERIFIED FIXED

(MailNews Core :: Networking: POP, defect)

Description

[David :Bienvenu]

From bug 50155:

This is a bad bug.  Just last weekend, I lost some very important emails when I 
lost connection during download of about 30 emails.  I reconnected and 
continued the download only to find that all the message bodys no longer 
matched the subjects.  Before I realized this, I deleted (what I thought) was 
the typical spam messages, only to find out that their bodys were my 
*important* emails!  This was with Netscape 6.2, but I come to find out that 
Mozilla STILL has this problem.

Data Loss.  This is no good folks!

I report this as a regular user would, having (at the time) NO idea about 
the .msf files and their flaky-ness.  You can't leave this bug out there on an 
unsuspecting public, and no company in their right mind would use this for 
their Mail application.



------- Additional Comment #18 From Stefan Magdalinski 2002-02-14 03:59 -------

What he said! I had this happen to me again yesterday on 2002021203 on XP

it doesn't even seem to be getting any attention at all. 

dataloss, dataloss, dataloss.



------- Additional Comment #19 From [e-mail] 2002-02-14 07:36 -------

Now, the bug that cmorley describes seems to have to do with our handling of
getting disconnected from the server while doing a pop3 download, which is the
first mention of that bug that I can see in this bug report. I assume cmorley is
doing a pop3 download, though he doesn't specify imap or pop3. Navin, could you
try that? Also, cmorley and stefan, it might be useful to Navin to know things
like whether you've got leave on server set, and how you were disconnected from
the server. Was it an async connection that just got dropped?

Comment 1

[David :Bienvenu]

adding keywords

Comment 2

[David :Bienvenu]

Stefan, you could turn on pop3 protocol logging, though that would really only
tell us what data we'd received from the pop3 server, and for that to be useful,
we'd need to sync that up with what messages were incorrectly displayed in the
thread pane. So the first thing really is to see if interrupting the pop3
download leaves invalid db entries.
Here are the instructions for pop3 protocol logging:

http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html#imap

Comment 3

[Navin Gupta]

accepting. I'll look into it. 

Comment 4

[Oliver Klee]

See bug 50155 for a workaround.

Comment 5

[cmorley]

bienvenu, Navin,
Pop3, do not leave messages on server, modem just dropped connection while 
getting 30+ emails.  I also noticed that all those with Html were hosed.

Comment 6

[Navin Gupta]

Attachment: proposed fix

The fix is to commit the db and mark it valid because we truncate the Inbox
to the last fully downloaded msg (upon aborting/cancelling download). 
David, can you review ?

Comment 7

[Navin Gupta]

*** Bug 122361 has been marked as a duplicate of this bug. ***

Comment 8

[David Balch]

Was adding comments on 122361, which amount to getting POP3 messages getting
corrupted as described above, on build 2002020406
(0.9.8) on WindowsME (and on previous builds, but I'm not sure of the numbers).

Also, I think this arises when my dialup connection is at or near saturation. I
didn't realise it at first, but then saw that Windows update had been
downloading in the background.

Will set up logging (I assume the 95/98 instructions will work on ME...)

Comment 9

[David :Bienvenu]

Navin, can you explain what was causing the problem and why this fixes it?
Leaving the DB invalid and uncommitted, on the face of it, should not produce a
corrupt db, because we'll just delete the invalid db next time we try to open
it. Committing and marking valid an invalid db, on the other hand, would produce
a corrupt, incorrect db. 

Comment 10

[Navin Gupta]

If you look at normal end of mail EndMailDelivery() we have to mark the db
valid because of the timestamp problem, that you fixed. Now in case
of abort or Cancel or dropped connection, we truncate the inbox to 
the last published header. Both the msf file and berkeley mailbox
are in sync so we have got to mark the db valid. 

Comment 11

[David :Bienvenu]

but what does that have to do with this bug? Were you able to recreate the
problem and see that this fixes it? 

Comment 12

[Navin Gupta]

oh, I was not able to exactly reproduce the bug but I found that hour-glass
problem on stopping and resuming pop3 msg downloading problem. I suspect
that leads to this problem of mismatched body/ hdrs.

Comment 13

[Navin Gupta]

With my fix I don't see any corruption and see no body hdr mismatch. I even
tried filtering and stopping and that also didn't result in body hdr mismatch
or hour-glass. It works. 

Comment 14

[David :Bienvenu]

but did you see corruption before your fix?

Comment 15

[Navin Gupta]

As I said before (I was not able to exactly reproduce the problem ..). 
I did not see any corruption before the fix but we still have to solve the
hour-glass problem, right?

Comment 16

[David :Bienvenu]

I can try this at home with my isp pop server and disconnecting the network
connection to see if I can recreate the corruption, and if I can, I'll try
applying your patch to see if it somehow fixes the problem.

Comment 17

[David :Bienvenu]

oh, sure. I just want to understand the corruption problem and make sure that
this patch doesn't cause problems in some situation that you're not encountering
yourself (since you're not seeing the corruption, you're not
encountering/testing the problem scenario, if that makes sense...). But most of
all, I want to fix the corruption problem. Figuring out the cause of that will
definitely help to know if your patch is the right thing.

Comment 18

[David :Bienvenu]

oh, another question - do you know why this fixes the hourglass problem? that
might be a useful piece of information.

Comment 19

[Navin Gupta]

because we close the outputstream to mailbox we have to commit the db so that
summary is valid. Otherwise it will try to parse the folder again. I can dig
into this more but I remember having you fixed another similar pop bug like
this. 

I am seeing some new problems of not able to re-download if you do not select
any message after interruption, I will work on this one also. 

Comment 20

[David :Bienvenu]

oh, right, I remember that hourglass problem now. It's not causing the
corruption directly. It could be that reparsing the folder while downloading get
new mail is causing the corruption, though I thought we'd had that pretty well
nailed.

Comment 21

[David :Bienvenu]

Stefan and Cmorley, do either of you have check for new mail turned on (and at
what interval), and do you have download new mail on check for new mail turned on?

Comment 22

[cmorley]

bienvenu,
For fear of just such a problem, I haven't even been using Mail, until this one 
time.  I loaded Netscape and hit the 'get mail' button.  Then lost connection.  
The rest is hystery...

Comment 23

[Navin Gupta]

Attachment: fix (protocol, sink part of diffs)

The fix is to make sure we have a valid at all times don't matter if it is
dropped connection, stop button or normal end of mail. So, to do that we have 
to commit the db once we know we have reached any of these states. 

I have added a method abort so that we know when we have dropped connection.
It seems necko has changed since last time I worked on this code and they 
no longer call Cancel on dropping connection like physically taking wire
out of the box. Also we don't want to call Abort() when it is
NS_BINDING_ABORTED 
because it means user pressed "stop". Also necko on normal end gives aStatus
= NS_BINDING_ABORTED. So this patch handles all the cases and it fixes the
case where I was unable to download mail after dropping connection. 

This part is optional but we may want to fix this, just in case...

Now if we dig into detail why we were unable to download mail after dropping
connection was because we had a db that was invalid so we were bailing out. 
More specifically this code...

rv = inbox->GetMsgDatabase(aWindow, getter_AddRefs(db));
    if (NS_SUCCEEDED(rv) && db)
      rv = db->GetSummaryValid(&valid);
    if (valid)
      rv = localMailServer->GetNewMail(aWindow, aListener, inbox, nsnull); 
    else
      rv = localInbox->SetCheckForNewMessagesAfterParsing(PR_TRUE);

Now we earlier had a boolean that used to tell us if we were parsing Inbox -
parsingInbox.We will have to add that boolean back because we cannot simply
null out db and getDB again because we may already be reparsing the folder. so 

we need that boolean. I couldn't find anything like that in nsIMsgDatabase 
interface. 

So the above code would look something like this.
  if(valid)
    ..
  else
   {
    GetParsingInbox(&parsingInbox);
     if (!parsingInbox)
     {	
       parse it...
     }
    Set...
   }

david, what do you think?

Comment 24

[David :Bienvenu]

make sure we have a valid ? at all times. DB? I'm still not sure how this
relates to getting a corrupt db. I'm not saying it's not right; just not sure
how it's related. Also, why don't we want to do this when the user presses stop?
Is that somehow different from dropping the connection? I would think we would
want to do the same cleanup. As far as the parsingInbox flag is concerned, I'd
be happier if we had a parsingFolder flag that applied to any folder that was
being parsed, as opposed to a parsingInbox flag for every local folder, whether
it was an inbox or not. You may be right that we need to know that.

Comment 25

[Navin Gupta]

One reason why I am not seeing mismatch body/hdr is because all this 
summaryValid fix is available only in 0.9.8 (latest milestone). looks like
atleast some users are using older milestones. 

Comment 26

[Navin Gupta]

I am doing the same clean-up on dropping connection and pressing stop if
you look at my patch. Regarding why we are getting mismatch problem, see
my earlier comment. 

Comment 27

[David :Bienvenu]

Navin, I don't see an earlier comment explaining the mismatch. Which comment did
you mean?

I tried disconnecting my connection while doing get new mail, and then doing get
new mail again. I immediately ran into a problem which could be related: when I
had a filter fire, it was unable to truncate the inbox here:

In nsParseNewMailState::MoveIncorporatedMessage,

  destIFolder->SetHasNewMessages(PR_TRUE);
  destFile->close();
  delete destFile;
  m_inboxFileStream->close();
  // How are we going to do this with a stream?
  nsresult truncRet = m_inboxFileSpec.Truncate(messageOffset);
  NS_ASSERTION(NS_SUCCEEDED(truncRet), "unable to truncate file");

Now, this can lead to all sorts of problems, because the offset of the messages
in the inbox will immediately become incorrect, because we expect truncate to
work. If we can't truncate, that probably means we couldn't close the file
either. I'm not sure we can't truncation - I suspect it's because we're not
cleaning up the mailbox folder stream in the disconnect case (not the DB, mind
you, the actual folder with the messages).

Comment 28

[David :Bienvenu]

oh, sorry, I see - you're claiming that everyone having this problem is running
a pre 9.8 build. Is that true?

Comment 29

[David :Bienvenu]

Comment on attachment 69772 [details] [diff] [review]
fix (protocol, sink part of diffs)

r=bienvenu on the second patch.

Comment 30

[David :Bienvenu]

to summarize a bit more, people (e.g., Stefan) are still seeing this with new
builds, so the problem wasn't fixed in .98. The fix to call Abort correctly will
help because it will correctly Truncate the Inbox instead of leaving potentially
partial messages in the inbox.

Comment 31

[Scott MacGregor]

Comment on attachment 69772 [details] [diff] [review]
fix (protocol, sink part of diffs)

sr=mscott

Comment 32

[Navin Gupta]

The basic problem that I was able to reproduce is fixed. We can spin off
additional bugs for more specific problems. We already have bug 122361 
for filter mismatch body/hdr. 

Comment 33

[Navin Gupta]

*** Bug 125664 has been marked as a duplicate of this bug. ***

Comment 34

[esther]

qa assigned to esther, I will be verifying this

Comment 35

[esther]

Using build 20020326 on winxp, I tested this by manually disconnecting from the
network while downloading a pop account.   
I did this with a new profile that had a pop account with 789 msgs. I
disconnected and reconnected 3 times before the download completed.  Each time I
reconnected I had to click Get Msg to initiate the download which is expected. 
Each time I sorted by order so I could note the last message downloaded for each
disconnect.  I had created a filter to filter known messages into a Local
foldler. I did not open any mail messages until I completed the download.  I
then viewed messages above and below the last downloaded for each disconnect. 
All messages matched their headers.  I viewed several messages in the filtered
folder, all messages matched their headers. 
Note: I did not reproduce the problem on eariler builds but did ask Navin how to
test this.  I will verify this based on my tests above, if anyone sees a need
for additional testing, please give me more scenarios.  

Comment 36

[gaele]

Please test the following scenario:
While downloading messages
1. read a just downloaded message
2. delete this message
This is where I got in trouble.

Comment 37

[Mirek]

Please check also my report on Bug 125664 which, according to my feeling, is no
longer duplicate of this one. However, it might serve as a testing design.