484377 - IMAP connections hanging

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Mark Banner (:standard8)]

STR:

1) Start up Thunderbird, get and read some mail.
2) Leave it alone for a while (without doing other stuff).
3) Come back to it, try to access imap folders or messages

Result: Thunderbird doesn't load/finish loading messages or folders.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090319 Shredder/3.1a1pre

Comment 1

[Mark Banner (:standard8)]

With a self-built debug build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090323 Shredder/3.0b3pre

this is still a big issue for me. The obvious thing I'm seeing is:

-1335603200[1cbd6b50]: 1c116800:localhost:S-INBOX:ProcessCurrentURL: entering
-1335603200[1cbd6b50]: 1c116800:localhost:S-INBOX:ProcessCurrentURL:imap://user@localhost/select%3E.INBOX:  = currentUrl
-1335603200[1cbd6b50]: 1c116800:localhost:S-INBOX:SendData: 10 noop
-1335603200[1cbd6b50]: ReadNextLine [stream=1cb5c37c nb=0 needmore=1]
-1335603200[1cbd6b50]: 1c116800:localhost:S-INBOX:CreateNewLineFromSocket: clearing IMAP_CONNECTION_IS_OPEN - rv = 804b0014
-1335603200[1cbd6b50]: 1c116800:localhost:S-INBOX:TellThreadToDie: close socket connection
-1335603200[1cbd6b50]: 1c116800:localhost:S-INBOX:CreateNewLineFromSocket: (null)
-1335603200[1cbd6b50]: WARNING: NS_ENSURE_SUCCESS(rv, 0) failed with result 0x80004005: file /Users/moztest/comm/main/src/mailnews/imap/src/nsImapProtocol.cpp, line 1793
WARNING: NS_ENSURE_SUCCESS(rv, 0) failed with result 0x80004005: file /Users/moztest/comm/main/src/mailnews/imap/src/nsImapProtocol.cpp, line 1793
-1335603200[1cbd6b50]: ImapThreadMainLoop leaving [this=1c116800]
-1339240448[1df26a50]: 21bc200:localhost:S-INBOX:SendData: DONE
-1339240448[1df26a50]: ReadNextLine [stream=1df26f4c nb=0 needmore=1]
-1339240448[1df26a50]: 21bc200:localhost:S-INBOX:CreateNewLineFromSocket: clearing IMAP_CONNECTION_IS_OPEN - rv = 804b0014
-1339240448[1df26a50]: 21bc200:localhost:S-INBOX:TellThreadToDie: close socket connection
-1339240448[1df26a50]: 21bc200:localhost:S-INBOX:CreateNewLineFromSocket: (null)
-1339240448[1df26a50]: WARNING: NS_ENSURE_SUCCESS(rv, 0) failed with result 0x80004005: file /Users/moztest/comm/main/src/mailnews/imap/src/nsImapProtocol.cpp, line 1793
WARNING: NS_ENSURE_SUCCESS(rv, 0) failed with result 0x80004005: file /Users/moztest/comm/main/src/mailnews/imap/src/nsImapProtocol.cpp, line 1793
-1339240448[1df26a50]: ImapThreadMainLoop leaving [this=21bc200]

The warning was added by bug 466318 and although it looks like a reasonable fix, it seems we're not doing quite the right thing, or we've regressed in some manner...

Comment 2

[David :Bienvenu]

Standard8, I think we don't need the NS_ENSURE_POINTER(aMockChannel) in PrepareToRetryUrl - can you try removing that? It's probably ok if the mock channel is null.

Comment 3

[Mark Banner (:standard8)]

(In reply to comment #2)
> Standard8, I think we don't need the NS_ENSURE_POINTER(aMockChannel) in
> PrepareToRetryUrl - can you try removing that? It's probably ok if the mock
> channel is null.

I don't see what you mean.

PrepareToRetryUrl is here: http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapIncomingServer.cpp#451

I can remove NS_ENSURE_ARG_POINTER(aChannel); but aImapUrl->GetMockChannel(aChannel); will check it again (because its checking for the out arg).

The only other point I could see would be http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapUrl.cpp#1171 (nsImapUrl::GetMockChannel) but then we'd be breaking xpcom rules and returning null without an error result (which we can do, but I don't think its what you are suggesting).

Comment 4

[David :Bienvenu]

+  // the channel might have gone away - we'll just not retry in that case.
+    NS_ENSURE_SUCCESS(rv, PR_FALSE);

you could try to remove this code - the old code didn't error out on a null mock channel.

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> +  // the channel might have gone away - we'll just not retry in that case.
> +    NS_ENSURE_SUCCESS(rv, PR_FALSE);
> 
> you could try to remove this code - the old code didn't error out on a null
> mock channel.

So far this is looking much better:

-1337643008[1b6bd0d0]: 1c22ea00:localhost:S-INBOX:CreateNewLineFromSocket: clearing IMAP_CONNECTION_IS_OPEN - rv = 804b0014
-1337643008[1b6bd0d0]: 1c22ea00:localhost:S-INBOX:TellThreadToDie: close socket connection
-1337643008[1b6bd0d0]: 1c22ea00:localhost:S-INBOX:CreateNewLineFromSocket: (null)
-1337643008[1b6bd0d0]: WARNING: Warning the channel went away
: 'NS_SUCCEEDED(rv)', file /Users/moztest/comm/main/src/mailnews/imap/src/nsImapProtocol.cpp, line 1794
WARNING: Warning the channel went away
: 'NS_SUCCEEDED(rv)', file /Users/moztest/comm/main/src/mailnews/imap/src/nsImapProtocol.cpp, line 1794
-1608014048[1f0abb0]: creating protocol instance to retry queued url:imap://user@localhost/select>.INBOX
-1608014048[1f0abb0]: retrying  url:imap://user@localhost/select>.INBOX


Note that I replaced the NS_ENSURE_SUCCESS with an NS_WARN_IF_FALSE which is why we're still getting debug output (I wanted to make sure I was still hitting instances of it).

After the section above, TB continues normally without issue.

Comment 6

[David :Bienvenu]

Attachment: proposed fix

this is what I suggested earlier, which seems to be working for standard8 - I haven't seen this problem myself...

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 369083 [details] [diff] [review]
proposed fix

I see this very frequently (if I leave Thunderbird for periods of time), and the change fixed it here so let's go with this.

Comment 8

[David :Bienvenu]

fix checked in.