From 66b5df68f82e3b4097cf2d5d9ec5892019a7dc16 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 27 Sep 2010 18:16:46 +0000 Subject: [PATCH] Merged revisions 85032 via svnmerge from svn+ssh://pythondev@svn.python.org/python/branches/py3k ........ r85032 | antoine.pitrou | 2010-09-27 19:52:25 +0200 (lun., 27 sept. 2010) | 6 lines Issue #9950: Fix socket.sendall() crash or misbehaviour when a signal is received. Now sendall() properly calls signal handlers if necessary, and retries sending if these returned successfully, including on sockets with a timeout. ........ --- Lib/test/test_socket.py | 37 +++++++++++++++++++++++++++++++ Misc/NEWS | 5 +++++ Modules/socketmodule.c | 48 +++++++++++++++++++++-------------------- 3 files changed, 67 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 5b5d4fb237c..37756b145da 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -15,6 +15,7 @@ import array import contextlib from weakref import proxy import signal +import math def try_address(host, port=0, family=socket.AF_INET): """Try to bind a socket on the given host:port and return True @@ -611,6 +612,42 @@ class GeneralModuleTests(unittest.TestCase): socket.AI_PASSIVE) + def check_sendall_interrupted(self, with_timeout): + # socketpair() is not stricly required, but it makes things easier. + if not hasattr(signal, 'alarm') or not hasattr(socket, 'socketpair'): + self.skipTest("signal.alarm and socket.socketpair required for this test") + # Our signal handlers clobber the C errno by calling a math function + # with an invalid domain value. + def ok_handler(*args): + self.assertRaises(ValueError, math.acosh, 0) + def raising_handler(*args): + self.assertRaises(ValueError, math.acosh, 0) + 1 // 0 + c, s = socket.socketpair() + old_alarm = signal.signal(signal.SIGALRM, raising_handler) + try: + if with_timeout: + # Just above the one second minimum for signal.alarm + c.settimeout(1.5) + with self.assertRaises(ZeroDivisionError): + signal.alarm(1) + c.sendall(b"x" * (1024**2)) + if with_timeout: + signal.signal(signal.SIGALRM, ok_handler) + signal.alarm(1) + self.assertRaises(socket.timeout, c.sendall, b"x" * (1024**2)) + finally: + signal.signal(signal.SIGALRM, old_alarm) + c.close() + s.close() + + def test_sendall_interrupted(self): + self.check_sendall_interrupted(False) + + def test_sendall_interrupted_with_timeout(self): + self.check_sendall_interrupted(True) + + @unittest.skipUnless(thread, 'Threading required for this test.') class BasicTCPTest(SocketConnectedTest): diff --git a/Misc/NEWS b/Misc/NEWS index 99d905df2c5..33dc1f15a49 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -48,6 +48,11 @@ Core and Builtins Library ------- +- Issue #9950: Fix socket.sendall() crash or misbehaviour when a signal is + received. Now sendall() properly calls signal handlers if necessary, + and retries sending if these returned successfully, including on sockets + with a timeout. + - Issue #9947: logging: Fixed locking bug in stopListening. - Issue #9945: logging: Fixed locking bugs in addHandler/removeHandler. diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 5da7809ea13..884cfe54dce 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2729,7 +2729,7 @@ static PyObject * sock_sendall(PySocketSockObject *s, PyObject *args) { char *buf; - int len, n = -1, flags = 0, timeout; + int len, n = -1, flags = 0, timeout, saved_errno; Py_buffer pbuf; if (!PyArg_ParseTuple(args, "s*|i:sendall", &pbuf, &flags)) @@ -2742,42 +2742,44 @@ sock_sendall(PySocketSockObject *s, PyObject *args) return select_error(); } - Py_BEGIN_ALLOW_THREADS do { + Py_BEGIN_ALLOW_THREADS timeout = internal_select(s, 1); n = -1; - if (timeout) - break; + if (!timeout) { #ifdef __VMS - n = sendsegmented(s->sock_fd, buf, len, flags); + n = sendsegmented(s->sock_fd, buf, len, flags); #else - n = send(s->sock_fd, buf, len, flags); + n = send(s->sock_fd, buf, len, flags); #endif + } + Py_END_ALLOW_THREADS + if (timeout == 1) { + PyBuffer_Release(&pbuf); + PyErr_SetString(socket_timeout, "timed out"); + return NULL; + } + /* PyErr_CheckSignals() might change errno */ + saved_errno = errno; + /* We must run our signal handlers before looping again. + send() can return a successful partial write when it is + interrupted, so we can't restrict ourselves to EINTR. */ + if (PyErr_CheckSignals()) { + PyBuffer_Release(&pbuf); + return NULL; + } if (n < 0) { -#ifdef EINTR - /* We must handle EINTR here as there is no way for - * the caller to know how much was sent otherwise. */ - if (errno == EINTR) { - /* Run signal handlers. If an exception was - * raised, abort and leave this socket in - * an unknown state. */ - if (PyErr_CheckSignals()) - return NULL; + /* If interrupted, try again */ + if (saved_errno == EINTR) continue; - } -#endif - break; + else + break; } buf += n; len -= n; } while (len > 0); - Py_END_ALLOW_THREADS PyBuffer_Release(&pbuf); - if (timeout == 1) { - PyErr_SetString(socket_timeout, "timed out"); - return NULL; - } if (n < 0) return s->errorhandler();