From 3e1fd27b74af4f1f040f8b11379015140240deff Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 28 Sep 2010 21:23:11 +0000 Subject: [PATCH] Issue #9090: When a socket with a timeout fails with EWOULDBLOCK or EAGAIN, retry the select() loop instead of bailing out. This is because select() can incorrectly report a socket as ready for reading (for example, if it received some data with an invalid checksum). --- Include/pytime.h | 11 +++++ Misc/NEWS | 5 ++ Modules/socketmodule.c | 103 +++++++++++++++++++++++++++++++++++------ 3 files changed, 104 insertions(+), 15 deletions(-) diff --git a/Include/pytime.h b/Include/pytime.h index 8ace98221ff..964d0968b0b 100644 --- a/Include/pytime.h +++ b/Include/pytime.h @@ -25,6 +25,17 @@ typedef struct { */ PyAPI_FUNC(void) _PyTime_gettimeofday(_PyTime_timeval *tp); +#define _PyTime_ADD_SECONDS(tv, interval) \ +do { \ + tv.tv_usec += (long) (((long) interval - interval) * 1000000); \ + tv.tv_sec += (time_t) interval + (time_t) (tv.tv_usec / 1000000); \ + tv.tv_usec %= 1000000; \ +} while (0) + +#define _PyTime_INTERVAL(tv_start, tv_end) \ + ((tv_end.tv_sec - tv_start.tv_sec) + \ + (tv_end.tv_usec - tv_start.tv_usec) * 0.000001) + /* Dummy to force linking. */ PyAPI_FUNC(void) _PyTime_Init(void); diff --git a/Misc/NEWS b/Misc/NEWS index a25da82a6bd..434a443ae41 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -74,6 +74,11 @@ Core and Builtins Library ------- +- Issue #9090: When a socket with a timeout fails with EWOULDBLOCK or EAGAIN, + retry the select() loop instead of bailing out. This is because select() + can incorrectly report a socket as ready for reading (for example, if it + received some data with an invalid checksum). + - Issue #3612: Added new types to ctypes.wintypes. (CHAR and pointers) - Issue #9950: Fix socket.sendall() crash or misbehaviour when a signal is diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index cd56ee31c19..017a63e5761 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -474,6 +474,14 @@ select_error(void) return NULL; } +#ifdef MS_WINDOWS +#define CHECK_ERRNO(expected) \ + (WSAGetLastError() == WSA ## expected) +#else +#define CHECK_ERRNO(expected) \ + (errno == expected) +#endif + /* Convenience function to raise an error according to errno and return a NULL pointer from a function. */ @@ -637,7 +645,7 @@ internal_setblocking(PySocketSockObject *s, int block) after they've reacquired the interpreter lock. Returns 1 on timeout, -1 on error, 0 otherwise. */ static int -internal_select(PySocketSockObject *s, int writing) +internal_select_ex(PySocketSockObject *s, int writing, double interval) { int n; @@ -649,6 +657,10 @@ internal_select(PySocketSockObject *s, int writing) if (s->sock_fd < 0) return 0; + /* Handling this condition here simplifies the select loops */ + if (interval < 0.0) + return 1; + /* Prefer poll, if available, since you can poll() any fd * which can't be done with select(). */ #ifdef HAVE_POLL @@ -660,7 +672,7 @@ internal_select(PySocketSockObject *s, int writing) pollfd.events = writing ? POLLOUT : POLLIN; /* s->sock_timeout is in seconds, timeout in ms */ - timeout = (int)(s->sock_timeout * 1000 + 0.5); + timeout = (int)(interval * 1000 + 0.5); n = poll(&pollfd, 1, timeout); } #else @@ -668,8 +680,8 @@ internal_select(PySocketSockObject *s, int writing) /* Construct the arguments to select */ fd_set fds; struct timeval tv; - tv.tv_sec = (int)s->sock_timeout; - tv.tv_usec = (int)((s->sock_timeout - tv.tv_sec) * 1e6); + tv.tv_sec = (int)interval; + tv.tv_usec = (int)((interval - tv.tv_sec) * 1e6); FD_ZERO(&fds); FD_SET(s->sock_fd, &fds); @@ -690,6 +702,53 @@ internal_select(PySocketSockObject *s, int writing) return 0; } +static int +internal_select(PySocketSockObject *s, int writing) +{ + return internal_select_ex(s, writing, s->sock_timeout); +} + +/* + Two macros for automatic retry of select() in case of false positives + (for example, select() could indicate a socket is ready for reading + but the data then discarded by the OS because of a wrong checksum). + Here is an example of use: + + BEGIN_SELECT_LOOP(s) + Py_BEGIN_ALLOW_THREADS + timeout = internal_select_ex(s, 0, interval); + if (!timeout) + outlen = recv(s->sock_fd, cbuf, len, flags); + Py_END_ALLOW_THREADS + if (timeout == 1) { + PyErr_SetString(socket_timeout, "timed out"); + return -1; + } + END_SELECT_LOOP(s) +*/ + +#define BEGIN_SELECT_LOOP(s) \ + { \ + _PyTime_timeval now, deadline = {0, 0}; \ + double interval = s->sock_timeout; \ + int has_timeout = s->sock_timeout > 0.0; \ + if (has_timeout) { \ + _PyTime_gettimeofday(&now); \ + deadline = now; \ + _PyTime_ADD_SECONDS(deadline, s->sock_timeout); \ + } \ + while (1) { \ + errno = 0; \ + +#define END_SELECT_LOOP(s) \ + if (!has_timeout || \ + (!CHECK_ERRNO(EWOULDBLOCK) && !CHECK_ERRNO(EAGAIN))) \ + break; \ + _PyTime_gettimeofday(&now); \ + interval = _PyTime_INTERVAL(now, deadline); \ + } \ + } \ + /* Initialize a new socket object. */ static double defaulttimeout = -1.0; /* Default timeout for new sockets */ @@ -1591,8 +1650,9 @@ sock_accept(PySocketSockObject *s) if (!IS_SELECTABLE(s)) return select_error(); + BEGIN_SELECT_LOOP(s) Py_BEGIN_ALLOW_THREADS - timeout = internal_select(s, 0); + timeout = internal_select_ex(s, 0, interval); if (!timeout) newfd = accept(s->sock_fd, SAS2SA(&addrbuf), &addrlen); Py_END_ALLOW_THREADS @@ -1601,6 +1661,7 @@ sock_accept(PySocketSockObject *s) PyErr_SetString(socket_timeout, "timed out"); return NULL; } + END_SELECT_LOOP(s) if (newfd == INVALID_SOCKET) return s->errorhandler(); @@ -2151,6 +2212,7 @@ will allow before refusing new connections."); * also possible that we return a number of bytes smaller than the request * bytes. */ + static Py_ssize_t sock_recv_guts(PySocketSockObject *s, char* cbuf, Py_ssize_t len, int flags) { @@ -2171,8 +2233,9 @@ sock_recv_guts(PySocketSockObject *s, char* cbuf, Py_ssize_t len, int flags) } #ifndef __VMS + BEGIN_SELECT_LOOP(s) Py_BEGIN_ALLOW_THREADS - timeout = internal_select(s, 0); + timeout = internal_select_ex(s, 0, interval); if (!timeout) outlen = recv(s->sock_fd, cbuf, len, flags); Py_END_ALLOW_THREADS @@ -2181,6 +2244,7 @@ sock_recv_guts(PySocketSockObject *s, char* cbuf, Py_ssize_t len, int flags) PyErr_SetString(socket_timeout, "timed out"); return -1; } + END_SELECT_LOOP(s) if (outlen < 0) { /* Note: the call to errorhandler() ALWAYS indirectly returned NULL, so ignore its return value */ @@ -2202,16 +2266,18 @@ sock_recv_guts(PySocketSockObject *s, char* cbuf, Py_ssize_t len, int flags) segment = remaining; } + BEGIN_SELECT_LOOP(s) Py_BEGIN_ALLOW_THREADS - timeout = internal_select(s, 0); + timeout = internal_select_ex(s, 0, interval); if (!timeout) nread = recv(s->sock_fd, read_buf, segment, flags); Py_END_ALLOW_THREADS - if (timeout == 1) { PyErr_SetString(socket_timeout, "timed out"); return -1; } + END_SELECT_LOOP(s) + if (nread < 0) { s->errorhandler(); return -1; @@ -2372,9 +2438,10 @@ sock_recvfrom_guts(PySocketSockObject *s, char* cbuf, Py_ssize_t len, int flags, return -1; } + BEGIN_SELECT_LOOP(s) Py_BEGIN_ALLOW_THREADS memset(&addrbuf, 0, addrlen); - timeout = internal_select(s, 0); + timeout = internal_select_ex(s, 0, interval); if (!timeout) { #ifndef MS_WINDOWS #if defined(PYOS_OS2) && !defined(PYCC_GCC) @@ -2395,6 +2462,7 @@ sock_recvfrom_guts(PySocketSockObject *s, char* cbuf, Py_ssize_t len, int flags, PyErr_SetString(socket_timeout, "timed out"); return -1; } + END_SELECT_LOOP(s) if (n < 0) { s->errorhandler(); return -1; @@ -2532,8 +2600,9 @@ sock_send(PySocketSockObject *s, PyObject *args) buf = pbuf.buf; len = pbuf.len; + BEGIN_SELECT_LOOP(s) Py_BEGIN_ALLOW_THREADS - timeout = internal_select(s, 1); + timeout = internal_select_ex(s, 1, interval); if (!timeout) #ifdef __VMS n = sendsegmented(s->sock_fd, buf, len, flags); @@ -2541,13 +2610,14 @@ sock_send(PySocketSockObject *s, PyObject *args) n = send(s->sock_fd, buf, len, flags); #endif Py_END_ALLOW_THREADS - - PyBuffer_Release(&pbuf); - if (timeout == 1) { + PyBuffer_Release(&pbuf); PyErr_SetString(socket_timeout, "timed out"); return NULL; } + END_SELECT_LOOP(s) + + PyBuffer_Release(&pbuf); if (n < 0) return s->errorhandler(); return PyLong_FromSsize_t(n); @@ -2667,17 +2737,20 @@ sock_sendto(PySocketSockObject *s, PyObject *args) return NULL; } + BEGIN_SELECT_LOOP(s) Py_BEGIN_ALLOW_THREADS - timeout = internal_select(s, 1); + timeout = internal_select_ex(s, 1, interval); if (!timeout) n = sendto(s->sock_fd, buf, len, flags, SAS2SA(&addrbuf), addrlen); Py_END_ALLOW_THREADS - PyBuffer_Release(&pbuf); if (timeout == 1) { + PyBuffer_Release(&pbuf); PyErr_SetString(socket_timeout, "timed out"); return NULL; } + END_SELECT_LOOP(s) + PyBuffer_Release(&pbuf); if (n < 0) return s->errorhandler(); return PyLong_FromSsize_t(n);