On Mon, Nov 28, 2022 at 11:24:13PM -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 receive_timeout option appears to resolve the issue for me,
> thought others may find it useful.
>
> Signed-off-by: Matthew Sotoudeh <matt...@masot.net>
Thanks, Matthew. Looks good other than 1 suggestion below.
Acked-by: David Vernet <v...@manifault.com>
> ---
>
> Thanks for the great tips! Hope this is a bit nicer formatted :)
> (Also, happy to move to gitlab if preferable & more changes desired for
> this. Sent here just bc doc/devel-notes.txt suggested.)
>
> Changelog per recent David/Kevin suggestions:
> * move setsockopt to right before the connect
> * check return value of setsockopt, log the error if it fails
>
> globals.h | 1 +
> init.h | 7 +++++++
> mutt_socket.c | 8 ++++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/globals.h b/globals.h
> index 06ce410e..b782114e 100644
> --- a/globals.h
> +++ b/globals.h
> @@ -236,6 +236,7 @@ WHERE short PagerContext;
> WHERE short PagerIndexLines;
> WHERE short PagerSkipQuotedContext;
> WHERE short ReadInc;
> +WHERE short ReceiveTimeout;
> WHERE short ReflowWrap;
> WHERE short SaveHist;
> WHERE short SendmailWait;
> diff --git a/init.h b/init.h
> index e160d3d3..e268d796 100644
> --- a/init.h
> +++ b/init.h
> @@ -3221,6 +3221,13 @@ struct option_t MuttVars[] = {
> ** .pp
> ** Also see $$postponed variable.
> */
> + { "receive_timeout", DT_NUM, R_NONE, {.p=&ReceiveTimeout}, {.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.
> + */
> { "record", DT_PATH, R_NONE, {.p=&Outbox}, {.p="~/sent"} },
> /*
> ** .pp
> diff --git a/mutt_socket.c b/mutt_socket.c
> index 3e192072..9468319e 100644
> --- a/mutt_socket.c
> +++ b/mutt_socket.c
> @@ -433,6 +433,14 @@ static int socket_connect (int fd, struct sockaddr* sa)
>
> save_errno = 0;
>
> + if (ReceiveTimeout > 0)
> + {
> + struct timeval tv = { ReceiveTimeout, 0 };
> + if (setsockopt (fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv)) <
> 0)
I personally don't think the < 0 is necessary here. setsockopt() fails
on any non-zero return value. connect() checks < 0 though, so it's
probably fine to leave it as is to match the rest of the code.
> + dprint (1, (debugfile, "socket_connect: error setting receive timeout
> (%s)\n",
> + strerror(errno)));
> + }
> +
> if (connect (fd, sa, sa_size) < 0)
> {
> save_errno = errno;
> --
> 2.34.1
>
