389898 - IMAP mail lost when filtered to local folders

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Andy Boze]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007072702 SeaMonkey/2.0a1pre Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007072702 SeaMonkey/2.0a1pre

I'm noticing that IMAP mails are being lost when filtered to local folders. The filter log shows that an attempt was made to move the message, but either no message appears in the local folder, or a blank entry appears in the local folder. This began with the 20070727 nightly build. When I manually move e-mails from IMAP to local folders, I don't seem to lose them. IMAP messages filtered to another IMAP folder seem to get moved properly.

Reproducible: Always

Steps to Reproduce:
1.Set a filter to move IMAP e-mails to a local folder
2.
3.
Actual Results:  
Filtered e-mails are removed from IMAP inbox, but do not get placed in local folder specified in filter rule.

Expected Results:  
Filtered e-mails should be removed from IMAP inbox, and be placed in the local folder specified in filter rule.

Here's a section of the filter log:

Applied filter "[e-mail]" to message from Xxx Xxy <[e-mail]> - Pendragon stats - continued. at 7/27/2007 4:01:03 PM moved message id = [e-mail] to mailbox://nobody@Local%20Folders/BIC-L

Applied filter "[e-mail]" to message from Xxx Xxy <[e-mail]> - Pendragon stats - continued. at 7/27/2007 4:02:06 PM moved message id = [e-mail] to mailbox://nobody@Local%20Folders/BIC-L

Applied filter "[e-mail]" to message from Xxx Xxy <[e-mail]> - Pendragon stats - continued. at 7/27/2007 4:02:14 PM moved message id = [e-mail] to mailbox://nobody@Local%20Folders/BIC-L 

And here's what was appended to the local folder file:

From - Fri Jul 27 16:10:01 2007
X-Mozilla-Status: 0000
X-Mozilla-Status2: 00000000

From - Fri Jul 27 16:10:01 2007
X-Mozilla-Status: 0000
X-Mozilla-Status2: 00000000

From - Fri Jul 27 16:10:01 2007
X-Mozilla-Status: 0000
X-Mozilla-Status2: 00000000

Comment 1

[Mark Banner (:standard8)]

No need to specify suiterunner in title now that it is default on trunk.

Comment 2

[Wayne Mery (:wsmwk)]

dogfood problem. also seen thunderbird. 

regression range using nightly builds is 2007-07-25 ... 2007-07-26.  possibly caused by one of these http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fmailnews%2F&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-07-25+04%3A00%3A00&maxdate=2007-07-26+06%3A00%3A00&cvsroot=%2Fcvsroot

perhaps caused crashes noted in Bug 391252 and Bug 391357

Comment 3

[David :Bienvenu]

taking

Comment 4

[David :Bienvenu]

might be networking: imap but I'll put it at filters. 

cc'ing Nick, since if this really is a regression from the imap changes, it might be affecting him as well.

Comment 5

[David :Bienvenu]

taking - I'll figure this out this afternoon

Comment 6

[David :Bienvenu]

the problem as I see it, so far is that in nsImapService::FetchMessage, we create a channel:

      rv = NewChannel(url, getter_AddRefs(channel));

and call

      rv = channel->AsyncOpen(streamListener, aCtxt);

but no one is holding a reference to the channel, so it immediately gets deleted when the comptr goes out of scope.

Comment 7

[David :Bienvenu]

one more piece of data - in the filter case, there's no load group (I bet that's true in Nick's case as well). Normally, the load group holds onto the request/channel, keeping it alive.

Comment 8

[David :Bienvenu]

and the reason there's no load group is that there's no msgWindow associated with the operation. For some background operations, we don't have a msgWindow because we don't want the user bothered with things like network errors and password prompts, e.g., for the background check for new mail.

So, we could require a msgWindow, and have a different way of indicating that an operation shouldn't prompt/block, though I hate to require a msgWindow/docshell. 

Comment 9

[Wayne Mery (:wsmwk)]

(In reply to comment #8)
> and the reason there's no load group is that there's no msgWindow associated
> with the operation. 

David, does this fact correlate to my crash Bug 391357?

Comment 10

[David :Bienvenu]

Attachment: proposed fix

I verified that the load group does get destroyed, with the mock channel destructor on the stack, so I'm pretty sure this isn't introducing a cycle. And it fixes the problem.

Comment 11

[David :Bienvenu]

Attachment: fix that includes fix for bug 391402

need to do this for fetching a mime part as well (which wasn't setting the channel at all)

Comment 12

[David :Bienvenu]

er, wasn't setting the loadgroup on the channel at all.

Comment 13

[Scott MacGregor]

Comment on attachment 275900 [details] [diff] [review]
fix that includes fix for bug 391402

looks good David.

Comment 14

[David :Bienvenu]

fixed

Comment 15

[David :Bienvenu]

Attachment: fix a couple other issues this bug brought up

the reason the folder was ending up locked was that we weren't sending an OnStopRequest to the listener in the case where we closed the channel before running the url - I think the same thing would happen if we didn't get to run the url, so I added code to do that. Is it possible that we'll create and destroy/close imap mock channels in other cases, e.g., retries? In which case, this code would be wrong...

The second part of the patch is more important - when I added the first notification, I ended up in nsLocalMailFolder::EndCopy with m_curDstKey set to -1, which made us create a 4GB file. Someone told me they were getting in a situation where that happened, so I hope this fixes that...

Comment 16

[Scott MacGregor]

Comment on attachment 276221 [details] [diff] [review]
fix a couple other issues this bug brought up

I'm starting to get nervous about the retry case :). I'll try setting that breakpoint again to see if I can see a retry in the debugger...

Comment 17

[David :Bienvenu]

Try the mozilla.com server - it drops connections all the time :-)

Comment 18

[Wayne Mery (:wsmwk)]

i can confirm you concerns about retry - fails using 0810 build.  our mail server was down and thunderbird hung on the retry.

Comment 19

[David :Bienvenu]

Comment on attachment 276221 [details] [diff] [review]
fix a couple other issues this bug brought up

I've checked in the local mail folder part of this patch - I'm a little worried about the OnStopRequest part - it could lead to multiple OnStopRequest notifications, and worse, early OnStopRequest notifications...

Comment 20

[David :Bienvenu]

I believe this issue is fixed - we should open new bugs for other issues, e.g., the retry issue.  If your server was down, we'd never get to the imap filtering code in the first place.