From 08ae02f11e2d34b56859481171bc447ec45d5585 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 27 Sep 2010 18:14:43 +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 | 41 ++++++++++++++++++++++++++++------------- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 2415d539ab..ea83cd1b43 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -17,6 +17,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 @@ -607,6 +608,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) + + class BasicTCPTest(SocketConnectedTest): def __init__(self, methodName='runTest'): diff --git a/Misc/NEWS b/Misc/NEWS index 06848bf1b6..8d41ef3221 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -121,6 +121,11 @@ C-API 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 #9936: Fixed executable lines' search in the trace module. - Issue #9928: Properly initialize the types exported by the bz2 module. diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 652900811f..ce594dc9dd 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2553,7 +2553,7 @@ sock_sendall(PySocketSockObject *s, PyObject *args) { char *buf; Py_ssize_t len, n = -1; - int flags = 0, timeout; + int flags = 0, timeout, saved_errno; Py_buffer pbuf; if (!PyArg_ParseTuple(args, "y*|i:sendall", &pbuf, &flags)) @@ -2566,29 +2566,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 - if (n < 0) - break; + } + 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) { + /* If interrupted, try again */ + if (saved_errno == EINTR) + continue; + 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(); -- 2.40.0