diff -r fe34dfea16b0 Lib/socket.py --- a/Lib/socket.py Fri Apr 03 18:12:32 2015 +0300 +++ b/Lib/socket.py Fri Apr 03 22:56:46 2015 -0400 @@ -103,6 +103,16 @@ _realsocket = socket +# Wrapper for handling interruptable system calls. +def _retryable_call(call, *args, **kwargs): + while True: + try: + return call(*args, **kwargs) + except error as e: + if e.args[0] == EINTR: + continue + raise + # WSA error codes if sys.platform.lower().startswith("win"): errorTab = {} @@ -304,6 +314,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 +362,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.recv, rbufsize) if not data: break buf.write(data) @@ -380,12 +388,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.recv, left) if not data: break n = len(data) @@ -447,12 +450,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.recv, self._rbufsize) if not data: break nl = data.find('\n') @@ -476,12 +474,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.recv, self._rbufsize) if not data: break left = size - buf_len @@ -563,7 +556,8 @@ sock.settimeout(timeout) if source_address: sock.bind(source_address) - sock.connect(sa) + + _retryable_call(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 03 22:56:46 2015 -0400 @@ -1311,15 +1311,23 @@ """ 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): + def __init__(self, *args): + self._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 +1371,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 03 22:56: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,32 +2088,57 @@ #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 + + 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; + res = errno; return res; } @@ -2130,16 +2156,18 @@ 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 - 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; } @@ -4187,9 +4215,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