diff -r fe34dfea16b0 Doc/library/socket.rst --- a/Doc/library/socket.rst Fri Apr 03 18:12:32 2015 +0300 +++ b/Doc/library/socket.rst Fri Apr 10 13:35:46 2015 -0400 @@ -229,6 +229,11 @@ .. versionchanged:: 2.7 *source_address* was added. + .. versionchanged:: 2.7.10 + If a call is interrupted by a signal, it will now be automaticaly + retried rather than raising a :exc`socket.error` with :py:data:`~errno.EINTR` + set. + .. function:: getaddrinfo(host, port[, family[, socktype[, proto[, flags]]]]) @@ -670,6 +675,11 @@ *mode* and *bufsize* arguments are interpreted the same way as by the built-in :func:`file` function. + .. versionchanged:: 2.7.10 + If a blocking call is interrupted by a signal, it will now be automaticaly + retried rather than raising a :exc:`socket.error` with :py:data:`~errno.EINTR` + set. + .. note:: On Windows, the file-like object created by :meth:`makefile` cannot be diff -r fe34dfea16b0 Lib/socket.py --- a/Lib/socket.py Fri Apr 03 18:12:32 2015 +0300 +++ b/Lib/socket.py Fri Apr 10 13:35:46 2015 -0400 @@ -47,6 +47,7 @@ import _socket from _socket import * from functools import partial +import time from types import MethodType try: @@ -103,6 +104,42 @@ _realsocket = socket +# Wrapper for handling interruptable system calls. +def _retryable_call(s, func, *args, **kwargs): + # Some modules (SSL) use the _fileobject wrapper directly and + # implement a smaller portion of the socket interface, thus we + # need to let them continue to do so. + timeout, deadline = None, 0.0 + try: + timeout = s.gettimeout() + except AttributeError: + pass + + if timeout: + deadline = time.time() + timeout + + try: + while True: + if timeout: + now = time.time() + if now >= deadline: + raise error(errno.EWOULDBLOCK, "timed out") + else: + # Overwrite the timeout on the socket object + # to take into account elapsed time. + s.settimeout(deadline - now) + try: + return func(*args, **kwargs) + except error as e: + if e.args[0] == EINTR: + continue + raise + finally: + # Set the existing timeout back for future + # calls. + if timeout: + s.settimeout(timeout) + # WSA error codes if sys.platform.lower().startswith("win"): errorTab = {} @@ -304,6 +341,8 @@ view = memoryview(data) try: while write_offset < data_size: + + # The sendall method on a socket implements proper EINTR support. self._sock.sendall(view[write_offset:write_offset+buffer_size]) write_offset += buffer_size finally: @@ -350,13 +389,9 @@ if size < 0: # Read until EOF self._rbuf = StringIO() # reset _rbuf. we consume it via buf. + while True: - try: - data = self._sock.recv(rbufsize) - except error, e: - if e.args[0] == EINTR: - continue - raise + data = _retryable_call(self._sock, self._sock.recv, rbufsize) if not data: break buf.write(data) @@ -380,12 +415,7 @@ # than that. The returned data string is short lived # as we copy it into a StringIO and free it. This avoids # fragmentation issues on many platforms. - try: - data = self._sock.recv(left) - except error, e: - if e.args[0] == EINTR: - continue - raise + data = _retryable_call(self._sock, self._sock.recv, left) if not data: break n = len(data) @@ -447,12 +477,7 @@ buf.seek(0, 2) # seek end self._rbuf = StringIO() # reset _rbuf. we consume it via buf. while True: - try: - data = self._sock.recv(self._rbufsize) - except error, e: - if e.args[0] == EINTR: - continue - raise + data = _retryable_call(self._sock, self._sock.recv, self._rbufsize) if not data: break nl = data.find('\n') @@ -476,12 +501,7 @@ return rv self._rbuf = StringIO() # reset _rbuf. we consume it via buf. while True: - try: - data = self._sock.recv(self._rbufsize) - except error, e: - if e.args[0] == EINTR: - continue - raise + data = _retryable_call(self._sock, self._sock.recv, self._rbufsize) if not data: break left = size - buf_len @@ -563,7 +583,8 @@ sock.settimeout(timeout) if source_address: sock.bind(source_address) - sock.connect(sa) + + _retryable_call(sock, sock.connect, sa) return sock except error as _: diff -r fe34dfea16b0 Lib/test/test_socket.py --- a/Lib/test/test_socket.py Fri Apr 03 18:12:32 2015 +0300 +++ b/Lib/test/test_socket.py Fri Apr 10 13:35:46 2015 -0400 @@ -1139,14 +1139,18 @@ """Test that the file object correctly handles EINTR internally.""" class MockSocket(object): - def __init__(self, recv_funcs=()): + def __init__(self, recv_funcs=(), timeout=None): # A generator that returns callables that we'll call for each # call to recv(). self._recv_step = iter(recv_funcs) + self._timeout = timeout def recv(self, size): return self._recv_step.next()() + def gettimeout(self): + return self._timeout + @staticmethod def _raise_eintr(): raise socket.error(errno.EINTR) @@ -1311,15 +1315,22 @@ """ class NetworkConnectionNoServer(unittest.TestCase): - class MockSocket(socket.socket): + class MockSocketTimeout(socket.socket): def connect(self, *args): raise socket.timeout('timed out') + class MockSocketInterrupted(socket.socket): + _eintr_triggered = False + def connect(self, *args): + if not self._eintr_triggered: + self._eintr_triggered = True + raise socket.error(errno.EINTR, "interrupted") + @contextlib.contextmanager - def mocked_socket_module(self): + def mocked_socket_module(self, mock_class): """Return a socket which times out on connect""" old_socket = socket.socket - socket.socket = self.MockSocket + socket.socket = mock_class try: yield finally: @@ -1363,10 +1374,14 @@ def test_create_connection_timeout(self): # Issue #9792: create_connection() should not recast timeout errors # as generic socket errors. - with self.mocked_socket_module(): + with self.mocked_socket_module(self.MockSocketTimeout): with self.assertRaises(socket.timeout): socket.create_connection((HOST, 1234)) + def test_create_connection_interrupted(self): + with self.mocked_socket_module(self.MockSocketInterrupted): + # This should not raise on an interrupted system call. + socket.create_connection((HOST, 1234)) @unittest.skipUnless(thread, 'Threading required for this test.') class NetworkConnectionAttributesTest(SocketTCPTest, ThreadableTest): diff -r fe34dfea16b0 Modules/socketmodule.c --- a/Modules/socketmodule.c Fri Apr 03 18:12:32 2015 +0300 +++ b/Modules/socketmodule.c Fri Apr 10 13:35:46 2015 -0400 @@ -732,12 +732,6 @@ 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 @@ -1434,9 +1428,9 @@ { struct sockaddr_hci *addr = (struct sockaddr_hci *)addr_ret; #if defined(__NetBSD__) || defined(__DragonFly__) - char *straddr = PyBytes_AS_STRING(args); - - _BT_HCI_MEMB(addr, family) = AF_BLUETOOTH; + char *straddr = PyBytes_AS_STRING(args); + + _BT_HCI_MEMB(addr, family) = AF_BLUETOOTH; if (straddr == NULL) { PyErr_SetString(socket_error, "getsockaddrarg: " "wrong format"); @@ -2036,10 +2030,11 @@ int res, timeout; timeout = 0; + Py_BEGIN_ALLOW_THREADS res = connect(s->sock_fd, addr, addrlen); + Py_END_ALLOW_THREADS #ifdef MS_WINDOWS - if (s->sock_timeout > 0.0) { if (res < 0 && WSAGetLastError() == WSAEWOULDBLOCK && IS_SELECTABLE(s)) { @@ -2053,7 +2048,13 @@ FD_SET(s->sock_fd, &fds); FD_ZERO(&fds_exc); FD_SET(s->sock_fd, &fds_exc); + + /* Select blocks, so slightly more fine grained in the Windows + * connect implementation. */ + Py_BEGIN_ALLOW_THREADS res = select(s->sock_fd+1, NULL, &fds, &fds_exc, &tv); + Py_END_ALLOW_THREADS + if (res == 0) { res = WSAEWOULDBLOCK; timeout = 1; @@ -2087,36 +2088,62 @@ #else if (s->sock_timeout > 0.0) { + + /* POSIX.1c defines errno as thread safe, so there's no race condition. */ if (res < 0 && errno == EINPROGRESS && IS_SELECTABLE(s)) { - timeout = internal_select(s, 1); - if (timeout == 0) { - /* Bug #1019808: in case of an EINPROGRESS, - use getsockopt(SO_ERROR) to get the real - error. */ - socklen_t res_size = sizeof res; - (void)getsockopt(s->sock_fd, SOL_SOCKET, - SO_ERROR, &res, &res_size); - if (res == EISCONN) - res = 0; - errno = res; - } - else if (timeout == -1) { - res = errno; /* had error */ - } - else - res = EWOULDBLOCK; /* timed out */ + + double begin_connect, remaining; + int saved_errno; + begin_connect = _PyTime_FloatTime(); + remaining = s->sock_timeout; + + while (remaining > 0.0) { + Py_BEGIN_ALLOW_THREADS + timeout = internal_select_ex(s, 1, remaining); + Py_END_ALLOW_THREADS + + /* Select complete with no timeout. */ + if (!timeout) + break; + + saved_errno = errno; + if (PyErr_CheckSignals()) { + break; + } + + /* Total delay, sub elapsed time. */ + remaining = s->sock_timeout - (_PyTime_FloatTime() - begin_connect); + if (timeout == -1) { + if (saved_errno != EINTR) + break; + } + } + + /* Process select return state. */ + if (timeout == 0) { + /* Bug #1019808: in case of an EINPROGRESS, + use getsockopt(SO_ERROR) to get the real + error. */ + socklen_t res_size = sizeof res; + (void)getsockopt(s->sock_fd, SOL_SOCKET, + SO_ERROR, &res, &res_size); + if (res == EISCONN) + res = 0; + errno = res; + } + else if (timeout == -1) + res = errno; /* had error */ + else + res = EWOULDBLOCK; /* timed out */ } } +#endif + *timeoutp = timeout; if (res < 0) res = errno; - -#endif - *timeoutp = timeout; - return res; } - /* s.connect(sockaddr) method */ static PyObject * @@ -2130,16 +2157,26 @@ if (!getsockaddrarg(s, addro, SAS2SA(&addrbuf), &addrlen)) return NULL; - Py_BEGIN_ALLOW_THREADS res = internal_connect(s, SAS2SA(&addrbuf), addrlen, &timeout); - Py_END_ALLOW_THREADS + + /* Make sure a failed connect is closed if this connection was + * handled via nonblocking, we can't reuse the sock_fd anyway. + * This also ensures the socket isn't still pending connection. + */ + if ((s->sock_timeout > 0.0) && (timeout || res)) + SOCKETCLOSE(s->sock_fd); if (timeout == 1) { PyErr_SetString(socket_timeout, "timed out"); return NULL; } - if (res != 0) + + if (res != 0) { + if (PyErr_Occurred()) + return NULL; return s->errorhandler(); + } + Py_INCREF(Py_None); return Py_None; } @@ -2164,9 +2201,11 @@ if (!getsockaddrarg(s, addro, SAS2SA(&addrbuf), &addrlen)) return NULL; - Py_BEGIN_ALLOW_THREADS res = internal_connect(s, SAS2SA(&addrbuf), addrlen, &timeout); - Py_END_ALLOW_THREADS + + /* Make sure the socket is closed if we've err'd out nonblocking. */ + if ((s->sock_timeout > 0.0) && (timeout || res)) + SOCKETCLOSE(s->sock_fd); /* Signals are not errors (though they may raise exceptions). Adapted from PyErr_SetFromErrnoWithFilenameObject(). */ @@ -2174,7 +2213,6 @@ if (res == EINTR && PyErr_CheckSignals()) return NULL; #endif - return PyInt_FromLong((long) res); } @@ -4187,9 +4225,9 @@ #if defined(__APPLE__) && defined(AI_NUMERICSERV) if ((flags & AI_NUMERICSERV) && (pptr == NULL || (pptr[0] == '0' && pptr[1] == 0))) { /* On OSX upto at least OSX 10.8 getaddrinfo crashes - * if AI_NUMERICSERV is set and the servname is NULL or "0". - * This workaround avoids a segfault in libsystem. - */ + * if AI_NUMERICSERV is set and the servname is NULL or "0". + * This workaround avoids a segfault in libsystem. + */ pptr = "00"; } #endif