From 115171086a5ded14a2bc2519e7f774a755e7ab04 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 29 Jul 2014 23:31:34 +0200 Subject: [PATCH] Issue #22018: On Windows, signal.set_wakeup_fd() now also supports sockets. A side effect is that Python depends to the WinSock library. --- Doc/c-api/exceptions.rst | 11 ++- Doc/library/signal.rst | 3 + Lib/test/test_signal.py | 107 ++++++++++++++++++++++ Misc/NEWS | 3 + Modules/signalmodule.c | 183 +++++++++++++++++++++++++++++++++---- PCbuild/pythoncore.vcxproj | 16 ++-- 6 files changed, 294 insertions(+), 29 deletions(-) diff --git a/Doc/c-api/exceptions.rst b/Doc/c-api/exceptions.rst index 33b44390d24..b0b1e43b05a 100644 --- a/Doc/c-api/exceptions.rst +++ b/Doc/c-api/exceptions.rst @@ -443,13 +443,18 @@ in various ways. There is a separate error indicator for each thread. .. c:function:: int PySignal_SetWakeupFd(int fd) - This utility function specifies a file descriptor to which a ``'\0'`` byte will - be written whenever a signal is received. It returns the previous such file - descriptor. The value ``-1`` disables the feature; this is the initial state. + This utility function specifies a file descriptor to which the signal number + is written as a single byte whenever a signal is received. *fd* must be + non-blocking. It returns the previous such file descriptor. + + The value ``-1`` disables the feature; this is the initial state. This is equivalent to :func:`signal.set_wakeup_fd` in Python, but without any error checking. *fd* should be a valid file descriptor. The function should only be called from the main thread. + .. versionchanged:: 3.5 + On Windows, the function now also supports socket handles. + .. c:function:: PyObject* PyErr_NewException(char *name, PyObject *base, PyObject *dict) diff --git a/Doc/library/signal.rst b/Doc/library/signal.rst index a97ce66ef52..ed616b2e95e 100644 --- a/Doc/library/signal.rst +++ b/Doc/library/signal.rst @@ -318,6 +318,9 @@ The :mod:`signal` module defines the following functions: attempting to call it from other threads will cause a :exc:`ValueError` exception to be raised. + .. versionchanged:: 3.5 + On Windows, the function now also supports socket handles. + .. function:: siginterrupt(signalnum, flag) diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index caca0be999d..04cd342553e 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -6,6 +6,7 @@ import gc import pickle import select import signal +import socket import struct import subprocess import traceback @@ -255,6 +256,13 @@ class WakeupFDTests(unittest.TestCase): self.assertRaises((ValueError, OSError), signal.set_wakeup_fd, fd) + def test_invalid_socket(self): + sock = socket.socket() + fd = sock.fileno() + sock.close() + self.assertRaises((ValueError, OSError), + signal.set_wakeup_fd, fd) + def test_set_wakeup_fd_result(self): r1, w1 = os.pipe() self.addCleanup(os.close, r1) @@ -268,6 +276,20 @@ class WakeupFDTests(unittest.TestCase): self.assertEqual(signal.set_wakeup_fd(-1), w2) self.assertEqual(signal.set_wakeup_fd(-1), -1) + def test_set_wakeup_fd_socket_result(self): + sock1 = socket.socket() + self.addCleanup(sock1.close) + fd1 = sock1.fileno() + + sock2 = socket.socket() + self.addCleanup(sock2.close) + fd2 = sock2.fileno() + + signal.set_wakeup_fd(fd1) + self.assertIs(signal.set_wakeup_fd(fd2), fd1) + self.assertIs(signal.set_wakeup_fd(-1), fd2) + self.assertIs(signal.set_wakeup_fd(-1), -1) + @unittest.skipIf(sys.platform == "win32", "Not valid on Windows") class WakeupSignalTests(unittest.TestCase): @@ -435,6 +457,90 @@ class WakeupSignalTests(unittest.TestCase): """, signal.SIGUSR1, signal.SIGUSR2, ordered=False) +@unittest.skipUnless(hasattr(socket, 'socketpair'), 'need socket.socketpair') +class WakeupSocketSignalTests(unittest.TestCase): + + @unittest.skipIf(_testcapi is None, 'need _testcapi') + def test_socket(self): + # use a subprocess to have only one thread + code = """if 1: + import signal + import socket + import struct + import _testcapi + + signum = signal.SIGINT + signals = (signum,) + + def handler(signum, frame): + pass + + signal.signal(signum, handler) + + read, write = socket.socketpair() + read.setblocking(False) + write.setblocking(False) + signal.set_wakeup_fd(write.fileno()) + + _testcapi.raise_signal(signum) + + data = read.recv(1) + if not data: + raise Exception("no signum written") + raised = struct.unpack('B', data) + if raised != signals: + raise Exception("%r != %r" % (raised, signals)) + + read.close() + write.close() + """ + + assert_python_ok('-c', code) + + @unittest.skipIf(_testcapi is None, 'need _testcapi') + def test_send_error(self): + # Use a subprocess to have only one thread. + if os.name == 'nt': + action = 'send' + else: + action = 'write' + code = """if 1: + import errno + import signal + import socket + import sys + import time + import _testcapi + from test.support import captured_stderr + + signum = signal.SIGINT + + def handler(signum, frame): + pass + + signal.signal(signum, handler) + + read, write = socket.socketpair() + read.setblocking(False) + write.setblocking(False) + + signal.set_wakeup_fd(write.fileno()) + + # Close sockets: send() will fail + read.close() + write.close() + + with captured_stderr() as err: + _testcapi.raise_signal(signum) + + err = err.getvalue() + if ('Exception ignored when trying to {action} to the signal wakeup fd' + not in err): + raise AssertionError(err) + """.format(action=action) + assert_python_ok('-c', code) + + @unittest.skipIf(sys.platform == "win32", "Not valid on Windows") class SiginterruptTest(unittest.TestCase): @@ -984,6 +1090,7 @@ def test_main(): try: support.run_unittest(GenericTests, PosixTests, InterProcessSignalTests, WakeupFDTests, WakeupSignalTests, + WakeupSocketSignalTests, SiginterruptTest, ItimerTest, WindowsSignalTests, PendingSignalsTests) finally: diff --git a/Misc/NEWS b/Misc/NEWS index 2ac15a73433..479fb9325a8 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -113,6 +113,9 @@ Core and Builtins Library ------- +- Issue #22018: On Windows, signal.set_wakeup_fd() now also supports sockets. + A side effect is that Python depends to the WinSock library. + - Issue #22054: Add os.get_blocking() and os.set_blocking() functions to get and set the blocking mode of a file descriptor (False if the O_NONBLOCK flag is set, True otherwise). These functions are not available on Windows. diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index c4f01216444..61b3330d3fa 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -7,6 +7,9 @@ #ifndef MS_WINDOWS #include "posixmodule.h" #endif +#ifdef MS_WINDOWS +#include "socketmodule.h" /* needed for SOCKET_T */ +#endif #ifdef MS_WINDOWS #include @@ -87,7 +90,20 @@ static volatile struct { PyObject *func; } Handlers[NSIG]; +#ifdef MS_WINDOWS +#define INVALID_FD ((SOCKET_T)-1) + +static volatile struct { + SOCKET_T fd; + int use_send; + int send_err_set; + int send_errno; + int send_win_error; +} wakeup = {INVALID_FD, 0, 0}; +#else +#define INVALID_FD (-1) static volatile sig_atomic_t wakeup_fd = -1; +#endif /* Speed up sigcheck() when none tripped */ static volatile sig_atomic_t is_tripped = 0; @@ -172,7 +188,7 @@ checksignals_witharg(void * unused) } static int -report_wakeup_error(void *data) +report_wakeup_write_error(void *data) { int save_errno = errno; errno = (int) (Py_intptr_t) data; @@ -184,25 +200,86 @@ report_wakeup_error(void *data) return 0; } +#ifdef MS_WINDOWS +static int +report_wakeup_send_error(void* Py_UNUSED(data)) +{ + PyObject *res; + + if (wakeup.send_win_error) { + /* PyErr_SetExcFromWindowsErr() invokes FormatMessage() which + recognizes the error codes used by both GetLastError() and + WSAGetLastError */ + res = PyErr_SetExcFromWindowsErr(PyExc_OSError, wakeup.send_win_error); + } + else { + errno = wakeup.send_errno; + res = PyErr_SetFromErrno(PyExc_OSError); + } + + assert(res == NULL); + wakeup.send_err_set = 0; + + PySys_WriteStderr("Exception ignored when trying to send to the " + "signal wakeup fd:\n"); + PyErr_WriteUnraisable(NULL); + + return 0; +} +#endif /* MS_WINDOWS */ + static void trip_signal(int sig_num) { unsigned char byte; - int rc = 0; + int fd; + Py_ssize_t rc; Handlers[sig_num].tripped = 1; - if (wakeup_fd != -1) { + +#ifdef MS_WINDOWS + fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int); +#else + fd = wakeup_fd; +#endif + + if (fd != INVALID_FD) { byte = (unsigned char)sig_num; - while ((rc = write(wakeup_fd, &byte, 1)) == -1 && errno == EINTR); - if (rc == -1) - Py_AddPendingCall(report_wakeup_error, (void *) (Py_intptr_t) errno); +#ifdef MS_WINDOWS + if (wakeup.use_send) { + do { + rc = send(fd, &byte, 1, 0); + } while (rc < 0 && errno == EINTR); + + /* we only have a storage for one error in the wakeup structure */ + if (rc < 0 && !wakeup.send_err_set) { + wakeup.send_err_set = 1; + wakeup.send_errno = errno; + wakeup.send_win_error = GetLastError(); + Py_AddPendingCall(report_wakeup_send_error, NULL); + } + } + else +#endif + { + byte = (unsigned char)sig_num; + do { + rc = write(fd, &byte, 1); + } while (rc < 0 && errno == EINTR); + + if (rc < 0) { + Py_AddPendingCall(report_wakeup_write_error, + (void *)(Py_intptr_t)errno); + } + } + } + + if (!is_tripped) { + /* Set is_tripped after setting .tripped, as it gets + cleared in PyErr_CheckSignals() before .tripped. */ + is_tripped = 1; + Py_AddPendingCall(checksignals_witharg, NULL); } - if (is_tripped) - return; - /* Set is_tripped after setting .tripped, as it gets - cleared in PyErr_CheckSignals() before .tripped. */ - is_tripped = 1; - Py_AddPendingCall(checksignals_witharg, NULL); } static void @@ -426,10 +503,29 @@ signal_siginterrupt(PyObject *self, PyObject *args) static PyObject * signal_set_wakeup_fd(PyObject *self, PyObject *args) { - struct stat buf; +#ifdef MS_WINDOWS + PyObject *fdobj; + SOCKET_T fd, old_fd; + int res; + int res_size = sizeof res; + PyObject *mod; + struct stat st; + int is_socket; + + if (!PyArg_ParseTuple(args, "O:set_wakeup_fd", &fdobj)) + return NULL; + + fd = PyLong_AsSocket_t(fdobj); + if (fd == (SOCKET_T)(-1) && PyErr_Occurred()) + return NULL; +#else int fd, old_fd; + struct stat st; + if (!PyArg_ParseTuple(args, "i:set_wakeup_fd", &fd)) return NULL; +#endif + #ifdef WITH_THREAD if (PyThread_get_thread_ident() != main_thread) { PyErr_SetString(PyExc_ValueError, @@ -438,28 +534,72 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args) } #endif +#ifdef MS_WINDOWS + is_socket = 0; + if (fd != INVALID_FD) { + /* Import the _socket module to call WSAStartup() */ + mod = PyImport_ImportModuleNoBlock("_socket"); + if (mod == NULL) + return NULL; + Py_DECREF(mod); + + /* test the socket */ + if (getsockopt(fd, SOL_SOCKET, SO_ERROR, + (char *)&res, &res_size) != 0) { + int err = WSAGetLastError(); + if (err != WSAENOTSOCK) { + PyErr_SetExcFromWindowsErr(PyExc_OSError, err); + return NULL; + } + + if (!_PyVerify_fd(fd)) { + PyErr_SetString(PyExc_ValueError, "invalid fd"); + return NULL; + } + + if (fstat(fd, &st) != 0) { + PyErr_SetFromErrno(PyExc_OSError); + return NULL; + } + } + else + is_socket = 1; + } + + old_fd = wakeup.fd; + wakeup.fd = fd; + wakeup.use_send = is_socket; + + if (old_fd != INVALID_FD) + return PyLong_FromSocket_t(old_fd); + else + return PyLong_FromLong(-1); +#else if (fd != -1) { if (!_PyVerify_fd(fd)) { PyErr_SetString(PyExc_ValueError, "invalid fd"); return NULL; } - if (fstat(fd, &buf) != 0) - return PyErr_SetFromErrno(PyExc_OSError); + if (fstat(fd, &st) != 0) { + PyErr_SetFromErrno(PyExc_OSError); + return NULL; + } } old_fd = wakeup_fd; wakeup_fd = fd; return PyLong_FromLong(old_fd); +#endif } PyDoc_STRVAR(set_wakeup_fd_doc, "set_wakeup_fd(fd) -> fd\n\ \n\ -Sets the fd to be written to (with '\\0') when a signal\n\ +Sets the fd to be written to (with the signal number) when a signal\n\ comes in. A library can use this to wakeup select or poll.\n\ -The previous fd is returned.\n\ +The previous fd or -1 is returned.\n\ \n\ The fd must be non-blocking."); @@ -467,10 +607,17 @@ The fd must be non-blocking."); int PySignal_SetWakeupFd(int fd) { - int old_fd = wakeup_fd; + int old_fd; if (fd < 0) fd = -1; + +#ifdef MS_WINDOWS + old_fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int); + wakeup.fd = fd; +#else + old_fd = wakeup_fd; wakeup_fd = fd; +#endif return old_fd; } diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 6021cd74cbf..1ad16fb8222 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -176,7 +176,7 @@ "$(SolutionDir)make_buildinfo.exe" Release "$(IntDir)\" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) $(OutDir)$(PyDllName).dll libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 @@ -212,7 +212,7 @@ IF %ERRORLEVEL% NEQ 0 ( "$(SolutionDir)make_buildinfo.exe" Release "$(IntDir)\" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 @@ -247,7 +247,7 @@ IF %ERRORLEVEL% NEQ 0 ( "$(SolutionDir)make_buildinfo.exe" Debug "$(IntDir)" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 @@ -285,7 +285,7 @@ IF %ERRORLEVEL% NEQ 0 ( "$(SolutionDir)make_buildinfo.exe" Debug "$(IntDir)" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 @@ -317,7 +317,7 @@ IF %ERRORLEVEL% NEQ 0 ( "$(SolutionDir)make_buildinfo.exe" Release "$(IntDir)\" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) $(OutDir)$(PyDllName).dll libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 @@ -353,7 +353,7 @@ IF %ERRORLEVEL% NEQ 0 ( "$(SolutionDir)make_buildinfo.exe" Release "$(IntDir)\" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 MachineX64 @@ -386,7 +386,7 @@ IF %ERRORLEVEL% NEQ 0 ( "$(SolutionDir)make_buildinfo.exe" Release "$(IntDir)\" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) $(OutDir)$(PyDllName).dll libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 @@ -422,7 +422,7 @@ IF %ERRORLEVEL% NEQ 0 ( "$(SolutionDir)make_buildinfo.exe" Release "$(IntDir)\" - $(IntDir)getbuildinfo.o;%(AdditionalDependencies) + $(IntDir)getbuildinfo.o;ws2_32.lib;%(AdditionalDependencies) libc;%(IgnoreSpecificDefaultLibraries) 0x1e000000 MachineX64