Re: [PATCH v3] Add socket send/receive timeout options

2022-12-02 Thread Kevin J. McCarthy

On Thu, Dec 01, 2022 at 04:54:21PM -0800, Matthew Sotoudeh via Mutt-dev wrote:

On an unreliable connection (e.g., laptop put to sleep and changing wifi
networks) I've had mutt fairly regularly become stuck in SSL_read and
have to be killed.

Per some of the comments on
https://stackoverflow.com/questions/46517875/ssl-read-blocks-indefinitely
adding a timeout to the socket should carry over to the SSL_read call.

Using this socket_receive_timeout option appears to resolve the issue
for me.

Signed-off-by: Matthew Sotoudeh 
---

Thanks all for the comments!


Thank you Matthew.  It's been a busy week, but I'll get this tested and 
committed this weekend!


--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA


signature.asc
Description: PGP signature


[PATCH v3] Add socket send/receive timeout options

2022-12-01 Thread Matthew Sotoudeh via Mutt-dev
On an unreliable connection (e.g., laptop put to sleep and changing wifi
networks) I've had mutt fairly regularly become stuck in SSL_read and
have to be killed.

Per some of the comments on
https://stackoverflow.com/questions/46517875/ssl-read-blocks-indefinitely
adding a timeout to the socket should carry over to the SSL_read call.

Using this socket_receive_timeout option appears to resolve the issue
for me.

Signed-off-by: Matthew Sotoudeh 
---

Thanks all for the comments!

Changelog:
* Add socket_ prefix and socket_send_timeout option per Kevin, Petr
  suggestions 
* Wrap the init.h, global.h additions in #ifdef USE_SOCKET (I think
  that flag should control compilation of mutt_socket.c as well?)

I decided to stick to the < 0 check for consistency with the rest of the
code (as noted). From my reading of the man pages, setsockopt is
supposed to return either 0 or -1, so < 0 is already a
broader-than-necessary check, if anything. But happy to revise that if
others have strong preferences.

 globals.h |  2 ++
 init.h| 16 
 mutt_socket.c | 16 
 3 files changed, 34 insertions(+)

diff --git a/globals.h b/globals.h
index 06ce410e..3f22ad35 100644
--- a/globals.h
+++ b/globals.h
@@ -103,6 +103,8 @@ WHERE char *MsgFmt;
 WHERE char *Preconnect;
 WHERE char *Tunnel;
 WHERE short NetInc;
+WHERE short SocketReceiveTimeout;
+WHERE short SocketSendTimeout;
 #endif /* USE_SOCKET */
 
 #ifdef MIXMASTER
diff --git a/init.h b/init.h
index e160d3d3..ea0bb975 100644
--- a/init.h
+++ b/init.h
@@ -4150,6 +4150,22 @@ struct option_t MuttVars[] = {
   ** Also see $$write_bcc.
   */
 #endif /* USE_SMTP */
+#ifdef USE_SOCKET
+  { "socket_receive_timeout",  DT_NUM, R_NONE, {.p=}, 
{.l=0} },
+  /*
+  ** .pp
+  ** Causes Mutt to timeout any socket read operation (e.g. SSL_read) after
+  ** this many seconds.  A zero (default) or negative value causes Mutt to wait
+  ** indefinitely for the read to complete.
+  */
+  { "socket_send_timeout",  DT_NUM, R_NONE, {.p=}, {.l=0} },
+  /*
+  ** .pp
+  ** Causes Mutt to timeout any socket write operation (e.g. SSL_write) after
+  ** this many seconds.  A zero (default) or negative value causes Mutt to wait
+  ** indefinitely for the write to complete.
+  */
+#endif /* USE_SOCKET */
   { "sort",DT_SORT, R_INDEX|R_RESORT, {.p=}, {.l=SORT_DATE} },
   /*
   ** .pp
diff --git a/mutt_socket.c b/mutt_socket.c
index 3e192072..898faa07 100644
--- a/mutt_socket.c
+++ b/mutt_socket.c
@@ -433,6 +433,22 @@ static int socket_connect (int fd, struct sockaddr* sa)
 
   save_errno = 0;
 
+  if (SocketReceiveTimeout > 0)
+  {
+struct timeval tv = { SocketReceiveTimeout, 0 };
+if (setsockopt (fd, SOL_SOCKET, SO_RCVTIMEO, (char *), sizeof(tv)) < 0)
+  dprint (1, (debugfile, "socket_connect: error setting receive timeout 
(%s)\n",
+  strerror(errno)));
+  }
+
+  if (SocketSendTimeout > 0)
+  {
+struct timeval tv = { SocketSendTimeout, 0 };
+if (setsockopt (fd, SOL_SOCKET, SO_SNDTIMEO, (char *), sizeof(tv)) < 0)
+  dprint (1, (debugfile, "socket_connect: error setting send timeout 
(%s)\n",
+  strerror(errno)));
+  }
+
   if (connect (fd, sa, sa_size) < 0)
   {
 save_errno = errno;
-- 
2.34.1