189363 - network timeout after a certain number of seconds [like OE]

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

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

[smtp] timeout after a certain number of seconds, like OE

I've got the beginnings of a backend fix for this.  

I doubt we'll add UI for this pref, but I'll attach OE's UI.

also, 4.x had a pref for this, "network.tcptimeout"

Comment 1

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

Attachment: some of the UI that OE has

Comment 2

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

code snippets from 4.x:

http://lxr.mcom.com/nova/source/lib/libnet/mkconect.c#159

159 #define DEFAULT_TCP_CONNECT_TIMEOUT 35  /* seconds */
160 PRIVATE uint32 net_tcp_connect_timeout = DEFAULT_TCP_CONNECT_TIMEOUT;/*seconds*/
161 
162 /* set the number of seconds for the tcp connect timeout
163  *
164  * this timeout is used to end connections that do not
165  * timeout on there own
166  */
167 PUBLIC void
168 NET_SetTCPConnectTimeout(uint32 seconds)
169 {
170     net_tcp_connect_timeout = seconds;
171 }


http://lxr.mcom.com/nova/source/cmd/winfe/netscape.cpp#1172

1166     //
1167     // Only mess with the TCP connect time out if we have gotten a
1168     //   positive number out of the Registry/INI file.  Don't let the
1169     //   number go below 30 seconds or above 4 minutes
1170     //
1171     int32 nTCPTimeOut;
1172     PREF_GetIntPref("network.tcptimeout",&nTCPTimeOut);
1173     if(nTCPTimeOut > 30 && nTCPTimeOut < 240)
1174         NET_SetTCPConnectTimeout(nTCPTimeOut);
1175     else if(nTCPTimeOut > 240)
1176         NET_SetTCPConnectTimeout(240);

coming soon, my patch for mozilla.

Comment 3

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

backend, not database

Comment 4

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

Attachment: patch, for the back end part of this

should this code live in nsSocketTransport.cpp, so it would affect all sockets?


also, I lxr'd, and no one is calling SetSocketTimeout()

Comment 5

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

Comment on attachment 111755 [details] [diff] [review]
patch, for the back end part of this

darin, when you get cycles (I know you are busy with the async i/o patch), can
you take a look?

would moving this to the socket transport code be better?

Comment 6

[Darin Fisher]

seth: i actually ripped out the SetSocketTimeout method on nsISocketTransport
because no one was using it.  i can easily put it back though... i was planning
on adding a general timeout and are-you-still-alive check to all sockets.  i can
do that now that the new socket code actually holds references to all "idle"
sockets.

how important is this bug for moz 1.3? 

Comment 7

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

not important.  It can wait.

I think the timeout should be pref controlled, as it was in 4.x and as in OE.

(but I don't think we necessarily need UI for it.)

depending on my connection and other factors, I could see the need for being
able to set the timeout from 30 seconds to 5 minutes, default to 1 minute

what do you think?  should I re-assign to you?

Comment 8

[Henrik Gemal]

please dont make any UI for this. I have enough of weird UI settings already...

Comment 9

[Darin Fisher]

Comment on attachment 111755 [details] [diff] [review]
patch, for the back end part of this

i'm sorry... nsISocketTransport no longer has a SetSocketTimeout method, and
the underlying functionality was never added into the rewritten
nsSocketTransport.

my apologies for letting this slip off my radar for so long.  now that the
patch has bit-rotted, i feel doubly bad :(

Comment 10

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

darin, no worries about the bit rot.

do you think we should add support for this hidden network.tcptimeout pref?

or should we do this another way (see your comment #6)

Comment 11

[David :Bienvenu]

this would be really helpful for IMAP - if for some reason, an IMAP server fails
to respond, we lock that folder out from the user until they restart, because we
have no timeout. If we had a timeout and could kill the connection, and then
allow the user to connect to the folder again...

Comment 12

[Darin Fisher]

Seth: oh, right.. that comment :)

hmm... the more i think of it, the more i'm uncertain what we are actually
trying to do here.  are we concerned with delays waiting for a connection to be
established or delays waiting for the server to accept the outgoing SMTP message.

the "are you alive" stuff i mentioned would only help in detecting a closed
socket.  that probably doesn't apply here the more i think of it.

perhaps a general maximum wait time could be configured and enforced... such
that if you don't get a writable or readable socket after N seconds, we'd close
the socket ourselves.  is that what you believe OE does?

Comment 13

[John T]

Mail timeouts and even webpage (browser) timeouts have become a problem since
perhaps the middle of January when my ISP apparently made some changes.

The first type of timeout problem occurs because of slow DNS lookups. When I
connect to my mail server (receiving mail), it sometimes times out because the
mail server address resolves too slowly. The same sometimes happens on sending
(SMTP server). I sent a message 3 times and got a "connection to SMTP server
failed" message before the message was successfully sent. These appear to be due
to slow DNS lookups. In trying to access the New York Times (www.nytimes.com),
my PC sat waiting for 71 seconds for the website address to be resolved, before
the message changed and the page started loading.

Being able to access a general timeout variable and increase it to fit the
connection quality would be helpful. It would also help to have error messages
that indicate just what operation timed out (DNS lookup, accessing the numeric
address of the mail server, etc).

Comment 14

[Bogdan Stroe]

Is this the same as bug 119740?
The other bug already have some duplicates, but this one has a patch.

Comment 15

[Met - Martin Hassman]

Is good to have only global network.tcptimeout prefs?

See, the user can use SMTP on the local network, but POP3 via some pop3proxy (so
for the POP3 is the long timeout necessary, but for SMTP is not).
And what more. This can differ from identity to identity - user can download
mails from quick local POP3 and also from POP3 of some freemail, which is often
busy.

So we need timenout for every SMTP or POP3/IMAP server.

Look in the solution in the Pegasus Mail. There is "Default timeout for network
connections" (30s) - this is global value (like your network.tcptimeout).
But for every every POP3 server and every SMTP server you can in Pegasus set
your own timeouts which has great priority then default value. 

So there should be also prefs 
"mail.server.server1.timeout" (for POP3/IMAP) and "mail.smtpserver.smtp1.timeout
"(for SMTP) o user can defined own value for every SMTP or POP3/IMAP server
which can ovewrite global network.tcptimeout value.

Comment 16

[John T]

DNS lookup must be done before the numeric address is used to try to access any
URL. DNS is sometimes very slow. Perhaps there should be global or per-profile
DNS lookup timeout setting. Perhaps the numeric addresses of POP3 and SMTP
servers could be cached or stored as variables. Would that present any technical
or security problems?

Comment 17

[David :Bienvenu]

the dns lookup stuff is outside the control of the mailnews protocol handlers;
that's up to necko as I understand it. What we're going to do, for pop3
initially, is add a timeout that takes effect for commands executed after the
initial connection.

Comment 18

[J. Scott Berg]

(In reply to comment #12)

> hmm... the more i think of it, the more i'm uncertain what we are actually
> trying to do here.

Let me point out where I'm running into the problem.  I have a laptop which
often ends up behind NAT.  I use IMAP to get mail, and IDLE is a wonderful
thing.  The problem is that for IDLE to work, you need a long timeout.  If I'm
not getting enough spam that day, I might go for 15 minutes without IDLE spewing
anything back to me.  The NAT table on the NAT box times out, and now I have to
wait half an hour for Mozilla to recognize it.  I change the server timeout to 5
minutes, and now I have no problems with the NAT table timeouts, but IDLE is
essentially useless (unless I get more spam...).  I would be happy if the stop
button on mailnews just blasted all the IMAP connections to the corresponding
server I'm working on.  As it is, the stop button doesn't do anything.

So, I think a working stop button would probably do the trick for these sorts of
things: let the user decide if something funky is going on.

Comment 19

[David :Bienvenu]

taking, patches upcoming for imap and base/util, which will handle pop3, nntp,
and smtp(I think)

Comment 20

[David :Bienvenu]

Attachment: fix for imap timeout

add mailnews.tcptimeout pref, and use it for imap. Right now, I'm thinking a
global pref is fine for this.

Comment 21

[David :Bienvenu]

I do wonder if we should use the pref "network.tcptimeout" instead...I think we
should have a UI for this, because some virus checkers can be so slow that it
looks like the server has timed out...we may want to add the retry option like
OE, though I'm not sure how easy that's going to be. I think for IMAP it should
be fairly easy but I don't know about pop3 or smtp.

Comment 22

[David :Bienvenu]

Attachment: fix for pop3 and other protocols derived from nsMsgProtocol

Comment 23

[Scott MacGregor]

Comment on attachment 171271 [details] [diff] [review]
fix for pop3 and other protocols derived from nsMsgProtocol

do we need to initialize gGotTimeoutPref?

Do we need to do anything special with the timeout we set on the socket when we
go away? i.e. like tell it to clear the timer? Or does the socket transport do
all of that for us?

Comment 24

[Scott MacGregor]

Comment on attachment 171270 [details] [diff] [review]
fix for imap timeout

Can we delete the commented out section of code since that seems to have moved
to CloseStreams or is it still there for testing/comparison purposes?

Comment 25

[David :Bienvenu]

global vars are initialized to 0 automatically...I don't think we need to do
anything special to cancel the timeout - it's not a timer, per se, in necko,
just a saved timeout value.

I'll clean up the commented out code...

Comment 26

[David :Bienvenu]

fix checked in for base protocol code, which should fix pop3.

Comment 27

[David :Bienvenu]

Attachment: fix for news

need to adjust the timeout for cached news connections, and reset the timeout
when ever we run a url (news, unlike pop3, caches connections)

Comment 28

[Volker Moell]

JFYI: With the latest Build I got no timeouts in news any more.

Comment 29

[David :Bienvenu]

Attachment: new fix for imap timeouts

I had to rework the imap timeout fix because setTimeout in necko to a short
value will cause a timeout to fire if there hasn't been any socket activity. So
I had to write data to the socket before changing the timeout to a short value.
So I had to add a param to EndIdle, Logout, and Close that indicated whether we
were shutting down and thus wanted a short timeout...

Comment 30

[Scott MacGregor]

Comment on attachment 173194 [details] [diff] [review]
new fix for imap timeouts

what's this...your removinng the infamous jefft-isms? no more me-> :)

Comment 31

[David :Bienvenu]

fix for imap checked in, so resolving.