Last Comment Bug 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)
: Thunderbird Daily starting from 20111225 messed-up some messages (showing the...
Status: RESOLVED FIXED
: regression
Product: MailNews Core
Classification: Components
Component: Database (show other bugs)
: 12
: All All
: -- major (vote)
: Thunderbird 12.0
Assigned To: David :Bienvenu
:
:
Mentors:
: 714182 715118 (view as bug list)
Depends on:
Blocks: 402392
  Show dependency treegraph
 
Reported: 2011-12-26 23:49 PST by [:Aureliano Buendía]
Modified: 2012-01-16 03:52 PST (History)
13 users (show)
mozilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
fix the db's of pop3 filter moves (1.28 KB, patch)
2011-12-29 14:12 PST, David :Bienvenu
standard8: review+
Details | Diff | Splinter Review
w/ whitespace changes. (2.82 KB, patch)
2011-12-29 14:19 PST, David :Bienvenu
no flags Details | Diff | Splinter Review
bug-in-action (build 20111230) (38.77 KB, image/png)
2011-12-30 06:36 PST, [:Aureliano Buendía]
no flags Details
proposed fix (3.47 KB, patch)
2012-01-03 08:22 PST, David :Bienvenu
no flags Details | Diff | Splinter Review
fix with unit test (10.08 KB, patch)
2012-01-03 15:19 PST, David :Bienvenu
no flags Details | Diff | Splinter Review
remove debugging code (9.95 KB, patch)
2012-01-03 15:21 PST, David :Bienvenu
standard8: review-
Details | Diff | Splinter Review
fix tests (13.04 KB, patch)
2012-01-04 08:07 PST, David :Bienvenu
standard8: review+
Details | Diff | Splinter Review

Description [:Aureliano Buendía] 2011-12-26 23:49:58 PST
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] 2011-12-27 03:58:35 PST
regression start from build 20111224
Comment 2 [:Aureliano Buendía] 2011-12-27 23:25:10 PST
(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 2011-12-28 00:45:16 PST
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] 2011-12-28 00:56:06 PST
(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 2011-12-28 07:52:28 PST
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] 2011-12-28 08:43:10 PST
platforms\OS => All per comment #5
Comment 7 David :Bienvenu 2011-12-29 07:20:50 PST
Aureliano, are you using POP3 or IMAP? Are these message in your inbox, or messages moved to other folders?
Comment 8 [:Aureliano Buendía] 2011-12-29 07:25:06 PST
(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 2011-12-29 07:48:50 PST
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] 2011-12-29 07:56:48 PST
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] 2011-12-29 07:58:45 PST
(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] 2011-12-29 08:00:16 PST
(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 2011-12-29 08:03:29 PST
Are you using the global inbox, with multiple pop3 servers storing messages in the same inbox?
Comment 14 [:Aureliano Buendía] 2011-12-29 08:05:35 PST
(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 2011-12-29 08:11:07 PST
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] 2011-12-29 08:21:26 PST
(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 2011-12-29 08:27:09 PST
(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] 2011-12-29 11:28:22 PST
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 2011-12-29 14:12:39 PST
Created attachment 584846 [details] [diff] [review]
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 2011-12-29 14:19:05 PST
Created attachment 584853 [details] [diff] [review]
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] 2011-12-29 14:37:19 PST
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, limited time in Dec) 2011-12-29 14:37:43 PST
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 2011-12-29 17:56:00 PST
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] 2011-12-30 06:36:32 PST
Created attachment 584958 [details]
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] 2011-12-30 06:38:33 PST
(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 26 David :Bienvenu 2011-12-30 07:31:26 PST
*** Bug 714182 has been marked as a duplicate of this bug. ***
Comment 27 Joe Sabash [:JoeS1] 2011-12-31 12:13:47 PST
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 2011-12-31 12:14:57 PST
If you turn on anti-virus quarantining, I don't think you'll see any corruptions, fwiw.
Comment 29 Joe Sabash [:JoeS1] 2011-12-31 13:19:05 PST
(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 2012-01-02 08:16:43 PST
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 2012-01-03 08:22:10 PST
Created attachment 585415 [details] [diff] [review]
proposed fix

This fixes it for me. I'll try to figure out a unit test for this.
Comment 32 :aceman 2012-01-03 08:29:31 PST
I can also see this, when I downloaded messages from only one server.
Comment 33 David :Bienvenu 2012-01-03 08:32:15 PST
(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 2012-01-03 08:39:33 PST
OK, then I can try to run with the patch.
Comment 35 :aceman 2012-01-03 11:30:54 PST
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 2012-01-03 15:19:08 PST
Created attachment 585571 [details] [diff] [review]
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 2012-01-03 15:21:43 PST
Created attachment 585575 [details] [diff] [review]
remove debugging code

oops, prev patch had some debugging code to verify the cause of the issue.
Comment 38 Mark Banner (:standard8, limited time in Dec) 2012-01-04 03:16:40 PST
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 2012-01-04 08:07:21 PST
Created attachment 585760 [details] [diff] [review]
fix tests
Comment 40 David :Bienvenu 2012-01-04 13:40:25 PST
http://hg.mozilla.org/comm-central/rev/34489081da88
Comment 41 David :Bienvenu 2012-01-04 14:45:39 PST
oops, forgot unit test http://hg.mozilla.org/comm-central/rev/bf6957ab2201
Comment 42 Sune Mølgaard 2012-01-04 15:04:22 PST
*** Bug 715118 has been marked as a duplicate of this bug. ***
Comment 43 Jonathan Protzenko [:protz] 2012-01-04 22:33:52 PST
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?

Note You need to log in before you can comment on or make changes to this bug.