200391 - roaming to a different IP address, get new mail (for pop) stops working (silently fails)

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[(not reading, please use [e-mail] instead)]

roaming to a different IP address, get new mail (for pop) stops working
(silently fails)

brendan writes:

"I see this all the time now running 1.3 on RH8, roaming my thinkpad from work
to home, or even from DHCP-based home wireless network to old fixed-IP home
network.  I haven't snooped the wire, but could it be that some old POP
connection is being reused?  Any way to avoid the annoying quit/restart cycle
I've been using to fix things?"

some initial thoughts:

we don't cache pop connections, so I wonder want is going on there.  
we might have a dns cache in netwerk (to save calls to gethostbyname()), but I'm
trying to think how that would cause your problem...

it might be necko, silently failing to establish the connection, or us dropping
the error (which I know we do for some errors), or both.

in nsMsgProtocol.cpp, see nsMsgProtocol::OnStopRequest(), where we silently drop
certain errors. 

there's an existing bug about us doing that when you try to do pop over ssl, or
nntp over ssl, and psm is not installed.

brendan, can you get a NSPR log for pop?
http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html#pop

darin might have another module for you to log, like sockettransport or something.

Comment 1

[David :Bienvenu]

yeah, we don't cache pop connections. It could be the necko dns cache. I've also
heard reports of this going on at an os level, but if shutting down mozilla and
restarting fixes it, then that is most likely not the problem.

Comment 2

[Darin Fisher]

brendan: have you tried toggling the online/offline switch?

Comment 3

[gordon]

I don't have problems with my POP connection going between work and home on Mac
OS X.  Just a data point.

Comment 4

[Darin Fisher]

brendan: what OS are you running?  (if linux, then what version of glibc.)

Comment 5

[Brendan Eich [:brendan]]

RH8, glibc-2.2.93-5.

/be

Comment 6

[Darin Fisher]

Attachment: v1 patch

i think this patch does the trick.  i setup a POP server, connected to pull
down my mail.  then i put the POP server offline, and pressed Get Msgs.  the
result: the throbber would just spin indefinitely, and nothing could be done to
recover from the situation.  bringing the server back online and pressing Get
Msgs again wouldn't help.  i determined that this can happen if the socket
transport enters the closed state (due to connection failure) while a
notification is pending.  the notification would just get dropped in this case,
resulting in the error not getting propogated.

Comment 7

[Darin Fisher]

i suspect that it is easy to put mozilla in this state when you consider that we
pin ip addresses.  i'm thinking that when brendan comes to work, his mozilla
still has the old ip address pinned.  then when he hits Get Msgs, we immediately
discover that there is no server listening and lose the race condition i just
described.

Comment 8

[Darin Fisher]

i think bug 200181 might be a duplicate of this...

Comment 9

[Darin Fisher]

-> me

Comment 10

[Darin Fisher]

Attachment: v1.1 patch

oops.. left some debugging crude in the last patch.

Comment 11

[Darin Fisher]

reviewers:

this is the critical part of the patch:

     if (NS_FAILED(mCondition)) {
         // ignore event since we're apparently already dead.
         LOG(("  ignoring event [condition=%x]\n", mCondition));
+        //
+        // notify input/output streams in case either has a pending notify.
+        //
+        mInput.OnSocketReady(mCondition);
+        mOutput.OnSocketReady(mCondition);
         return NS_OK;
     }

without this code, we have the race condition i described.  the rest of the
patch is mainly cosmetic.

Comment 12

[gordon]

Nit: maybe change the comment, since we're no longer (completely) ignoring the
event.  Logging still might be nice, but maybe it should say something a little
different.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 120924 [details] [diff] [review]
v1.1 patch

>Index: nsSocketTransport2.cpp
>-            if (NS_FAILED(rv)) {
>-                mNotify = 0;
>-                if (NS_SUCCEEDED(mCondition))
>-                    mCondition = rv;
>-            }
>+            if (NS_FAILED(rv)) return rv;

So we no longer need to update mCondition on failure here?  Why not?

Same for the output stream.

sr=me if there is a reason; if so, comment it.

Comment 14

[Darin Fisher]

gordon: good point.. thx!

boris: 1) consistency with nsPipe3.cpp and 2) that failure would only indicate
lack of sufficient memory, and it doesn't make any sense to alter the state of
the transport as a result.  the call to AsyncWait should just fail with no
side-effects (hence the reason why i defer clearing the old mNotify until later).

Comment 15

[Boris Zbarsky [:bzbarsky]]

Yeah, just put that in a nice comment. ;)

Comment 16

[Darin Fisher]

bz: you got it ;-)

Comment 17

[Darin Fisher]

fixed-on-trunk