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.
diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 3ef0850..ccb9cd0 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -16,6 +16,7 @@
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
@@ -655,6 +656,42 @@
# have a reverse entry yet
# socket.gethostbyaddr('испытание.python.org')
+ 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 b3af982..2fb3b95 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -68,6 +68,11 @@
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 6ebb9d9..cd56ee3 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -2568,7 +2568,7 @@
{
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))
@@ -2581,42 +2581,44 @@
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();