Suggested fix on Windows: handle also WSAETIMEDOUT after recv() returns failure

[Simon Josefsson]

"Tor Lillqvist" <tml at iki.fi> writes:

> I have been debugging an interesting problem in Evolution's use of
> GnuTLS on Windows. The root cause for the problem was elsewhere, see
> below if you are interested. From GnuTLS's point of view the
> interesting effect was that the recv() call in _gnutls_read() in
> gnutls_buffers.c returned -1, with WSAGetLastError() returning
> WSAETIMEDOUT. I think WSAETIMEDOUT should be handled like
> WSAEWOULDBLOCK here? At least in this case, doing that made the code
> work also without fixing the root cause.

According to MSDN, WSAETIMEDOUT means the connection has been "dropped":

http://msdn.microsoft.com/en-us/library/ms740121(VS.85).aspx

  "The connection has been dropped because of a network failure or
  because the peer system failed to respond."

Are you saying that calling recv again after a WSAETIMEDOUT actually
results in data sooner or later?  If so, I suggest we install this:

diff --git a/lib/gnutls_buffers.c b/lib/gnutls_buffers.c
index 8d9be9c..f21c57a 100644
--- a/lib/gnutls_buffers.c
+++ b/lib/gnutls_buffers.c
@@ -318,6 +318,10 @@ _gnutls_read (gnutls_session_t session, void *iptr,
 	      int tmperr = WSAGetLastError();
 	      switch (tmperr)
 		{
+		case WSAETIMEDOUT:
+		  gnutls_assert();
+		  /* fall through */
+
 		case WSAEWOULDBLOCK:
 		  session->internals.errnum = EAGAIN;
 		  break;

This triggers a debug message when debugging is enabled, which may help
people notice that something may be fishy in the recv functions, and
possibly even trace it back to this e-mail.

> FYI: The root cause for the problem was that libsoup uses setsockopt's
> SO_RCVTIMEO and SO_SNDTIMEO options to set the socket's timeout,
> passing a pointer to a struct timeval, like on Unix. But... the
> Winsock designers, for some reason, decided to change the semantics of
> these options: In Winsock the option value is an int, the timeout in
> milliseconds. So when libsoup tried to set the timeout to 30 seconds,
> it actually set it to 30 milliseconds, eek.) What on Earth were they
> thinking when they changed the type of the option value for
> SO_RCVTIMEO and SO_SNDTIMEO like this? They should at least have used
> a different names for the options when they change the type of their
> values. The type unsafety of setsockopt() means the compiler can't
> warn either.

Sigh.

/Simon