713611 - Thunderbird Daily starting from 20111225 messed-up some messages (showing their source in the preview pane due to offset shift by 2 bytes, when pop3 download of multiple msgs with quarantinging turned off and leave on server turned on)

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[[:Aureliano Buendía]]

Nightly builds starting from 20111224 messed-up some messages dysplaing message source in preview pane. Executing rebuilding index (repair folder resolve the issue).
It seems that appairs on threaded view when the message is the first message of a conversation.
I have tried also in safe-mode

Comment 1

[[:Aureliano Buendía]]

regression start from build 20111224

Comment 2

[[:Aureliano Buendía]]

(In reply to [:Aureliano Buendía] from comment #1)
> regression start from build -20111224-

My mistake: build 20111224 works fine. Regression date is *20111225* --- push-log is http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-12-25&enddate=2011-12-26

Comment 3

[Magnus Melin [:mkmelin]]

You probably want to use http://hg.mozilla.org/comm-central not http://hg.mozilla.org/mozilla-central

Fallout from bug 402392?

Comment 4

[[:Aureliano Buendía]]

(In reply to Magnus Melin from comment #3)
> You probably want to use http://hg.mozilla.org/comm-central not
> http://hg.mozilla.org/mozilla-central

Yes magnus, i'm sorry

> Fallout from bug 402392?
probably also for me, but I'm not sure
http://hg.mozilla.org/comm-central/pushloghtml?startdate=2011-12-24&enddate=2011-12-26

Comment 5

[Frederic Bezies]

Seeing this too on linux. Don't know if it is related, but got this in error console : 

Timestamp: 28/12/2011 16:45:30
Warning: Use of getAttributeNodeNS() is deprecated. Use getAttributeNS() instead.
Source File: chrome://messenger/content/messenger.xul
Line: 0

Comment 6

[[:Aureliano Buendía]]

platforms\OS => All per comment #5

Comment 7

[David :Bienvenu]

Aureliano, are you using POP3 or IMAP? Are these message in your inbox, or messages moved to other folders?

Comment 8

[[:Aureliano Buendía]]

(In reply to David :Bienvenu from comment #7)
> Aureliano, are you using POP3 or IMAP? 
POP3

>Are these message in your inbox, or messages moved to other folders?
Both cases: my mozilla TB messages that are affected by this behavior are moved in a subfolder by a specific filter; but the problem occurs also for messages that stay in Inbox folder and don't are moved by filters

Comment 9

[David :Bienvenu]

Is today's build any better? I did fix an issue having to do with the msgOffset for moved messages (though I think that didn't affect messages moved by incoming mail filters)

Comment 10

[[:Aureliano Buendía]]

I have updated now from 20111224 to today build. I have enter a comment in bug #713814 and the received message is fine... based on this first observation it seems that the regression is gone

Comment 11

[[:Aureliano Buendía]]

(In reply to [:Aureliano Buendía] from comment #10)
> I have updated now from 20111224 to today build. I have enter a comment in
> bug #713814 and the received message is fine... based on this first
> observation it seems that the regression is gone

I'm sorry... this message in my folder look again bad. I revert to 20111224 build

Comment 12

[[:Aureliano Buendía]]

(In reply to [:Aureliano Buendía] from comment #11)
> I'm sorry... this message in my folder look again bad. I revert to 20111224
> build
Different case of comment #10, in comment #11 I have received 2 messages at the same time and one of this look messed-up: could be a concurrent receiving?

Comment 13

[David :Bienvenu]

Are you using the global inbox, with multiple pop3 servers storing messages in the same inbox?

Comment 14

[[:Aureliano Buendía]]

(In reply to David :Bienvenu from comment #13)
> Are you using the global inbox, with multiple pop3 servers storing messages
> in the same inbox?

Yes, it is

Comment 15

[David :Bienvenu]

When this happened, did you receive two messages from two different servers at the same time? Or two messages from the same server? And was it from an automatic check for new mail, or did you click get new mail? The latter is supposed to serialize requests; the former relies on folder locking to prevent contention.

Comment 16

[[:Aureliano Buendía]]

(In reply to David :Bienvenu from comment #15)
> When this happened, did you receive two messages from two different servers
> at the same time? 
Same server

> And was it from an automatic check for new mail, or did you click 
> get new mail? 
click get new mail

Comment 17

[David :Bienvenu]

(In reply to [:Aureliano Buendía] from comment #16)
> (In reply to David :Bienvenu from comment #15)
> > When this happened, did you receive two messages from two different servers
> > at the same time? 
> Same server
> 
> > And was it from an automatic check for new mail, or did you click 
> > get new mail? 
> click get new mail

That should all be fine, in that it works for me here - any chance you were reading one new message while the other one was being downloaded? I take it that some of your mail is filtered to other folders, and some stays in the global inbox. I'm trying to reproduce the problem here but not having any luck.

Comment 18

[Joe Sabash [:JoeS1]]

I just saw this on both the 27th nightly and again after an update to:
Mozilla/5.0 (Windows NT 5.1; rv:12.0a1) Gecko/20111229 Thunderbird/12.0a1
pop3 manual check for new mail (leave messages on server)
Only one server checked for mail.
No global inbox.

Just to be clear..what I see in message pane or standalone message window is:
Nothing in the message reader headers are, and everything displayed (headers and text) in the content area.

Comment 19

[David :Bienvenu]

Attachment: fix the db's of pop3 filter moves

I don't know if this is related to any of the issues reported in this bug, but it's the one thing I can reproduce - we weren't marking the db valid after applying a move filter action for pop3 filters. In theory, an invalid db shouldn't cause any corruption, but it does prevent people from knowing that a message has been filtered into a folder.

Comment 20

[David :Bienvenu]

Attachment: w/ whitespace changes.

prev patch was a -w patch to make the change clear, but I will clean up the whitespace indenting before landing.

Comment 21

[Joe Sabash [:JoeS1]]

I should have added that no filter moves were involved on the system where I observed the problem. Fortunately, I can stay on a current nightly on that machine to evaluate.
In every case "rebuild folder" has fixed things.

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 584846 [details] [diff] [review]
fix the db's of pop3 filter moves

Looks like the pluggable store changes mean that destMailDB now gets modified a few lines above this point:

http://hg.mozilla.org/comm-central/annotate/c25a8efb9585/mailnews/local/src/nsParseMailbox.cpp#l2572

So I agree, this change seems the right one to make.

Comment 23

[David :Bienvenu]

first patch landed - http://hg.mozilla.org/comm-central/rev/068dfc71f91d

I agree that this doesn't explain the inbox corruptions - I'm still trying to reproduce that.

Comment 24

[[:Aureliano Buendía]]

Attachment: bug-in-action (build 20111230)

(In reply to David :Bienvenu from comment #23)
> first patch landed - http://hg.mozilla.org/comm-central/rev/068dfc71f91d

20111230 still don't work for me (see attached bug-in-action screenshot)

Comment 25

[[:Aureliano Buendía]]

(In reply to [:Aureliano Buendía] from comment #24)
> 20111230 still don't work for me (see attached bug-in-action screenshot)

for messages moved by filters

Comment 27

[Joe Sabash [:JoeS1]]

Bumped severity and changed component.
I personally am not willing to test Daily 12.0 with database corruption involved.
(Even though "repair Folder" seems to fix it.)

Comment 28

[David :Bienvenu]

If you turn on anti-virus quarantining, I don't think you'll see any corruptions, fwiw.

Comment 29

[Joe Sabash [:JoeS1]]

(In reply to David :Bienvenu from comment #28)
> If you turn on anti-virus quarantining, I don't think you'll see any
> corruptions, fwiw.

Yep,
Sent myself 3 consecutive messages and all viewed fine.
Back on the daily here. This has got to be one tough nut to crack.

Comment 30

[David :Bienvenu]

OK, I think this is because we're overzealous about not flushing the mailbox file when getting multiple messages. So when we're re-using a stream, we need to use the stream pos of the end of the stream instead of the mailbox file size as the msg offset of the new message. I think I have a fix that does this; testing it now.

Comment 31

[David :Bienvenu]

Attachment: proposed fix

This fixes it for me. I'll try to figure out a unit test for this.

Comment 32

[:aceman]

I can also see this, when I downloaded messages from only one server.

Comment 33

[David :Bienvenu]

(In reply to :aceman from comment #32)
> I can also see this, when I downloaded messages from only one server.

Yes, this bug has nothing to do with multiple servers - it specifically has to do with downloading multiple mails in the same session to the same inbox.

Comment 34

[:aceman]

OK, then I can try to run with the patch.

Comment 35

[:aceman]

Today's hg checkout plus this patch applied makes new downloaded messages display fine (more of them fetched in single check).

Comment 36

[David :Bienvenu]

Attachment: fix with unit test

This adds a unit test. Since reproducing this bug requires that quarantining be turned off, and pop3 leave on server be set to true, the test has to set the prefs accordingly. But since the pop3 fake server didn't implement UIDL, which is needed for leave on server, I had to fix that as well.

Comment 37

[David :Bienvenu]

Attachment: remove debugging code

oops, prev patch had some debugging code to verify the cause of the issue.

Comment 38

[Mark Banner (:standard8)]

Comment on attachment 585575 [details] [diff] [review]
remove debugging code

Review of attachment 585575 [details] [diff] [review]:
-----------------------------------------------------------------

I think the code looks fine. However adding the UIDL to the capabilities appears to have broken some of the unit tests that are now sending an extra XTND XLST Message-Id message to the fake server and that's showing up in the results. It probably just needs those tests adapting.

::: mailnews/local/test/unit/test_pop3Download.js
@@ +33,5 @@
> +{
> +  // get message headers for the inbox folder
> +  let enumerator = gLocalInboxFolder.msgDatabase.EnumerateMessages();
> +  var msgCount = 0;
> +  let hdr;

nit: unnecessary definition - duplicated in the while loop below.

Comment 39

[David :Bienvenu]

Attachment: fix tests

Comment 40

[David :Bienvenu]

http://hg.mozilla.org/comm-central/rev/34489081da88

Comment 41

[David :Bienvenu]

oops, forgot unit test http://hg.mozilla.org/comm-central/rev/bf6957ab2201

Comment 43

[Jonathan Protzenko [:protz]]

Hi,

Sorry for not reading the 42 (42 !) comments ;-) but is this going to fix the issue with the same symptoms on IMAP, or is it just for POP?