Hi all, I’m happy the socket patch is working out for people!
With regards to the offers to test or look at the code, by all means, do. Since it is quite a low-level change to the socket functions, there may be edge cases which I have not thought of. If people think we’re on the right track so far, then I also wanted to add the following points. One, is that I have only altered the receive timeout on sockets (SO_RCVTIMEO). But I think it should also apply to the send timeout (SO_SNDTIMEO). For example, if you are uploading a large file, and your internet dies, mutt will, AFAIK, hang. Please confirm or refute this as necessary. Another thing is that this breaks the rule of not changing old behavior by default. If we want that, then both of the config vars I added should be set to 0 or a negative number. In my opinion, though, the old behavior was buggy, so that’s why I didn’t continue it. And the last thing, it might be nice to have a way to automatically re-open the default mailbox once the current mailbox is closed. Does mutt already have such a thing? Allen On Sat, May 02, 2015 at 10:04:29PM -0400, Santiago Torres wrote: > Hi everyone, > > I've been testing your patch with my local build of Mutt (along w/ > new_mail_command) and, so far, it seems pretty stable. I'm quite happy > with the fix. > > Should I take a look at the code or anything of like that? How can I > help to move this forward? > > Thanks, > -Santiago. > > On Sat, May 02, 2015 at 01:20:25PM -0700, Yoshiki Vazquez-Baeza wrote: > > Hi Allen, > > > > Thanks for sending this out, I've been using this since you sent it and > > it is working as expected! This is awesome because I no longer have to > > close my IMAP connections before closing my laptop! > > > > Do let me know if you need some specific testing or if there's something > > else I can help with! > > > > Yoshiki. > > > > On (Apr-28-15|21:06), egbert. wrote: > > > Hi all, > > > > > > > > There was an irritating glitch, which I have also seen mentioned a > > > > > few times > > > > > online. It is that if you have mutt open, pack up your laptop, and go > > > > > somewhere else, mutt is completely unresponsive and needs to be > > > > > killed. I > > > > > tracked down what was causing this and to my satisfaction my fix > > > > > works. > > > > > > > > > > Would you be interested in this patch? And, if the intended behaviour > > > > > is > > > > > that it should hang like this, then perhaps my fix could be > > > > > implemented as a > > > > > config variable. (It’s basically just a simple timeout + retry > > > > > mechanism in > > > > > the socket_connect function). > > > > > > > > Please go ahead and post your patch, either to this list, or perhaps to > > > > a ticket. It looks like http://dev.mutt.org/trac/ticket/3369 or > > > > http://dev.mutt.org/trac/ticket/3491 might be related. > > > > > > > > Most of the committers are quite busy right now, so it may take a while > > > > to get a response. > > > > > > Attached is my patch for the hanging socket problem. > > > > > > I hope this fixes 3369 and 3491, which indeed sound very similar to the > > > problem I reported. > > > > > > All comments welcome. > > > > > > I used hg export against the tip of the default branch to produce the > > > diff; > > > please let me know if you would rather have it another way. > > > > > > > > > > > > Also, you’ve probably talked about this many times before, but is > > > > > there a > > > > > reason not to implement a config option to call a command when there > > > > > is new > > > > > mail? > > > > > > > > Yoshiki Vazquez Baeza posted a patch for this a couple months ago, and > > > > David Champion is incorporating it into a related patch set. > > > > > > > > > > Yours, Allen > > > > > # HG changeset patch > > > # User misterfish <al...@netherrealm.net> > > > # Date 1430247531 -7200 > > > # Tue Apr 28 20:58:51 2015 +0200 > > > # Node ID 65d3b99e229cefaec62e8205fec116a7c00fce72 > > > # Parent 755a18da99bc0a57bd6e462f5971eba7b9f61604 > > > Fix bug where mutt hangs indefinitely when, for example, you pack up your > > > laptop and open it again somewhere else. > > > > > > Introduce a timeout on sockets ("socket_timeout" in muttrc, default 5 > > > seconds) and a fixed number of retries for GNUTLS ("ssl_socket_num_tries" > > > in > > > muttrc, default is 2). > > > > > > Hopefully fixes #3369 and #3491. > > > > > > diff -r 755a18da99bc -r 65d3b99e229c globals.h > > > --- a/globals.h Sat Apr 25 19:00:13 2015 -0700 > > > +++ b/globals.h Tue Apr 28 20:58:51 2015 +0200 > > > @@ -193,6 +193,8 @@ > > > WHERE unsigned short Counter INITVAL (0); > > > > > > WHERE short ConnectTimeout; > > > +WHERE short SocketTimeout; > > > +WHERE short GNUTLSSocketNumTries; > > > WHERE short HistSize; > > > WHERE short MenuContext; > > > WHERE short PagerContext; > > > diff -r 755a18da99bc -r 65d3b99e229c init.h > > > --- a/init.h Sat Apr 25 19:00:13 2015 -0700 > > > +++ b/init.h Tue Apr 28 20:58:51 2015 +0200 > > > @@ -2888,6 +2888,14 @@ > > > ** variable. > > > */ > > > #endif /* USE_SMTP */ > > > + { "socket_timeout", DT_NUM, R_NONE, UL &SocketTimeout, 5 }, > > > + /* > > > + ** .pp > > > + ** Close inactive sockets after this many seconds. When set to zero or > > > a > > > + ** negative value, mutt will wait indefinitely on sockets, but be > > > warned > > > + ** that this can cause mutt to hang. If using GNUTLS, you must also set > > > + ** $$ssl_socket_num_tries to a non-zero value. > > > + */ > > > { "sort", DT_SORT, R_INDEX|R_RESORT, UL &Sort, SORT_DATE > > > }, > > > /* > > > ** .pp > > > @@ -3025,6 +3033,15 @@ > > > ** for use in any Diffie-Hellman key exchange. A value of 0 will use > > > ** the default from the GNUTLS library. > > > */ > > > + { "ssl_socket_num_tries", DT_NUM, R_NONE, UL &GNUTLSSocketNumTries, 2 > > > }, > > > + /* > > > + ** .pp > > > + ** When using GNUTLS, this can be used in conjunction with > > > + ** $$socket_timeout to cause the mailbox to close after the socket has > > > + ** timed out this many times. If set to zero or a negative number, the > > > + ** socket will wait for data indefinitely, but be aware that this can > > > + ** cause mutt to hang. > > > + */ > > > # endif /* USE_SSL_GNUTLS */ > > > { "ssl_starttls", DT_QUAD, R_NONE, OPT_SSLSTARTTLS, M_YES }, > > > /* > > > diff -r 755a18da99bc -r 65d3b99e229c mutt_socket.c > > > --- a/mutt_socket.c Sat Apr 25 19:00:13 2015 -0700 > > > +++ b/mutt_socket.c Tue Apr 28 20:58:51 2015 +0200 > > > @@ -344,6 +344,7 @@ > > > { > > > int sa_size; > > > int save_errno; > > > + struct timeval socket_timeout_tv = {0}; > > > > > > if (sa->sa_family == AF_INET) > > > sa_size = sizeof (struct sockaddr_in); > > > @@ -364,6 +365,28 @@ > > > > > > save_errno = 0; > > > > > > + /* Set a timeout on the socket. > > > + * Can be set to 0 for no timeout, but then, mutt can hang forever > > > waiting > > > + * for data. > > > + * For OpenSSL this is enough to avoid hanging, but with GNUTLS, > > > + * ssl_socket_num_tries must also be set in the config. > > > + */ > > > + > > > + if (SocketTimeout > 0) { > > > + socket_timeout_tv.tv_sec = SocketTimeout; > > > + if (setsockopt(fd, > > > + SOL_SOCKET, /* protocol level */ > > > + SO_RCVTIMEO, /* optname */ > > > + &socket_timeout_tv, /* const void *optval */ > > > + sizeof(struct timeval) > > > + )) > > > + { > > > + mutt_perror(_("Couldn't set socket_timeout")); > > > + mutt_sleep(3); > > > + return -1; > > > + } > > > + } > > > + > > > if (connect (fd, sa, sa_size) < 0) > > > { > > > save_errno = errno; > > > diff -r 755a18da99bc -r 65d3b99e229c mutt_ssl_gnutls.c > > > --- a/mutt_ssl_gnutls.c Sat Apr 25 19:00:13 2015 -0700 > > > +++ b/mutt_ssl_gnutls.c Tue Apr 28 20:58:51 2015 +0200 > > > @@ -131,6 +131,7 @@ > > > { > > > tlssockdata *data = conn->sockdata; > > > int ret; > > > + int try = 0; > > > > > > if (!data) > > > { > > > @@ -139,7 +140,20 @@ > > > return -1; > > > } > > > > > > + /* When the socket times out, ret is GNUTLS_E_AGAIN. > > > + * When socket_timeout (SocketTimeout) is 0, the socket is set to > > > + * blocking, and this will hang forever on gnutls_record_recv. > > > + * If the timeout is set, but ssl_socket_num_tries > > > (GNUTLSSocketNumTries) > > > + * is equal to zero or a negative value, this will become an infinite > > > loop > > > + * and mutt will hang. > > > + */ > > > + > > > do { > > > + if (GNUTLSSocketNumTries > 0 && ++try > GNUTLSSocketNumTries) { > > > + mutt_error(_("Timed out reading from socket, closing mailbox.")); > > > + mutt_sleep(2); > > > + return -1; > > > + } > > > ret = gnutls_record_recv (data->state, buf, len); > > > if (ret < 0 && gnutls_error_is_fatal(ret) == 1) > > > { > > > > > >
signature.asc
Description: Digital signature
