[Python-checkins] bpo-30050: Allow disabling full buffer warnings in signal.set_wakeup_fd (#4792)
Yury Selivanov
webhook-mailer at python.org
Sun Dec 17 23:10:20 EST 2017
https://github.com/python/cpython/commit/902ab80b590e474bb2077b1fae8aac497b856d66
commit: 902ab80b590e474bb2077b1fae8aac497b856d66
branch: master
author: Nathaniel J. Smith <njs at pobox.com>
committer: Yury Selivanov <yury at magic.io>
date: 2017-12-17T23:10:18-05:00
summary:
bpo-30050: Allow disabling full buffer warnings in signal.set_wakeup_fd (#4792)
files:
A Misc/NEWS.d/next/Library/2017-12-10-23-44-56.bpo-30050.4SZ3lY.rst
M Doc/library/signal.rst
M Lib/test/test_signal.py
M Modules/signalmodule.c
diff --git a/Doc/library/signal.rst b/Doc/library/signal.rst
index 2bc39d9f133..67eaa2c6381 100644
--- a/Doc/library/signal.rst
+++ b/Doc/library/signal.rst
@@ -300,7 +300,7 @@ The :mod:`signal` module defines the following functions:
Availability: Unix.
-.. function:: set_wakeup_fd(fd)
+.. function:: set_wakeup_fd(fd, *, warn_on_full_buffer=True)
Set the wakeup file descriptor to *fd*. When a signal is received, the
signal number is written as a single byte into the fd. This can be used by
@@ -312,16 +312,36 @@ The :mod:`signal` module defines the following functions:
If not -1, *fd* must be non-blocking. It is up to the library to remove
any bytes from *fd* before calling poll or select again.
- Use for example ``struct.unpack('%uB' % len(data), data)`` to decode the
- signal numbers list.
-
When threads are enabled, this function can only be called from the main thread;
attempting to call it from other threads will cause a :exc:`ValueError`
exception to be raised.
+ There are two common ways to use this function. In both approaches,
+ you use the fd to wake up when a signal arrives, but then they
+ differ in how they determine *which* signal or signals have
+ arrived.
+
+ In the first approach, we read the data out of the fd's buffer, and
+ the byte values give you the signal numbers. This is simple, but in
+ rare cases it can run into a problem: generally the fd will have a
+ limited amount of buffer space, and if too many signals arrive too
+ quickly, then the buffer may become full, and some signals may be
+ lost. If you use this approach, then you should set
+ ``warn_on_full_buffer=True``, which will at least cause a warning
+ to be printed to stderr when signals are lost.
+
+ In the second approach, we use the wakeup fd *only* for wakeups,
+ and ignore the actual byte values. In this case, all we care about
+ is whether the fd's buffer is empty or non-empty; a full buffer
+ doesn't indicate a problem at all. If you use this approach, then
+ you should set ``warn_on_full_buffer=False``, so that your users
+ are not confused by spurious warning messages.
+
.. versionchanged:: 3.5
On Windows, the function now also supports socket handles.
+ .. versionchanged:: 3.7
+ Added ``warn_on_full_buffer`` parameter.
.. function:: siginterrupt(signalnum, flag)
diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py
index dc048e56665..f17123c3bb5 100644
--- a/Lib/test/test_signal.py
+++ b/Lib/test/test_signal.py
@@ -91,6 +91,15 @@ def test_issue9324(self):
class WakeupFDTests(unittest.TestCase):
+ def test_invalid_call(self):
+ # First parameter is positional-only
+ with self.assertRaises(TypeError):
+ signal.set_wakeup_fd(signum=signal.SIGINT)
+
+ # warn_on_full_buffer is a keyword-only parameter
+ with self.assertRaises(TypeError):
+ signal.set_wakeup_fd(signal.SIGINT, False)
+
def test_invalid_fd(self):
fd = support.make_bad_fd()
self.assertRaises((ValueError, OSError),
@@ -423,6 +432,89 @@ def handler(signum, frame):
""".format(action=action)
assert_python_ok('-c', code)
+ @unittest.skipIf(_testcapi is None, 'need _testcapi')
+ def test_warn_on_full_buffer(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
+
+ # This handler will be called, but we intentionally won't read from
+ # the wakeup fd.
+ def handler(signum, frame):
+ pass
+
+ signal.signal(signum, handler)
+
+ read, write = socket.socketpair()
+ read.setblocking(False)
+ write.setblocking(False)
+
+ # Fill the send buffer
+ try:
+ while True:
+ write.send(b"x")
+ except BlockingIOError:
+ pass
+
+ # By default, we get a warning when a signal arrives
+ signal.set_wakeup_fd(write.fileno())
+
+ 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)
+
+ # And also if warn_on_full_buffer=True
+ signal.set_wakeup_fd(write.fileno(), warn_on_full_buffer=True)
+
+ 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)
+
+ # But not if warn_on_full_buffer=False
+ signal.set_wakeup_fd(write.fileno(), warn_on_full_buffer=False)
+
+ with captured_stderr() as err:
+ _testcapi.raise_signal(signum)
+
+ err = err.getvalue()
+ if err != "":
+ raise AssertionError("got unexpected output %r" % (err,))
+
+ # And then check the default again, to make sure warn_on_full_buffer
+ # settings don't leak across calls.
+ signal.set_wakeup_fd(write.fileno())
+
+ 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):
diff --git a/Misc/NEWS.d/next/Library/2017-12-10-23-44-56.bpo-30050.4SZ3lY.rst b/Misc/NEWS.d/next/Library/2017-12-10-23-44-56.bpo-30050.4SZ3lY.rst
new file mode 100644
index 00000000000..76de12bbe80
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-12-10-23-44-56.bpo-30050.4SZ3lY.rst
@@ -0,0 +1,3 @@
+New argument warn_on_full_buffer to signal.set_wakeup_fd lets you control
+whether Python prints a warning on stderr when the wakeup fd buffer
+overflows.
diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index 1023244b626..b553eedc0f2 100644
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -100,14 +100,15 @@ static volatile struct {
static volatile struct {
SOCKET_T fd;
+ int warn_on_full_buffer;
int use_send;
- int send_err_set;
- int send_errno;
- int send_win_error;
-} wakeup = {INVALID_FD, 0, 0};
+} wakeup = {.fd = INVALID_FD, .warn_on_full_buffer = 1, .use_send = 0};
#else
#define INVALID_FD (-1)
-static volatile sig_atomic_t wakeup_fd = -1;
+static volatile struct {
+ sig_atomic_t fd;
+ int warn_on_full_buffer;
+} wakeup = {.fd = INVALID_FD, .warn_on_full_buffer = 1};
#endif
/* Speed up sigcheck() when none tripped */
@@ -210,28 +211,15 @@ report_wakeup_write_error(void *data)
#ifdef MS_WINDOWS
static int
-report_wakeup_send_error(void* Py_UNUSED(data))
+report_wakeup_send_error(void* 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;
-
+ /* PyErr_SetExcFromWindowsErr() invokes FormatMessage() which
+ recognizes the error codes used by both GetLastError() and
+ WSAGetLastError */
+ PyErr_SetExcFromWindowsErr(PyExc_OSError, (int) (intptr_t) data);
PySys_WriteStderr("Exception ignored when trying to send to the "
"signal wakeup fd:\n");
PyErr_WriteUnraisable(NULL);
-
return 0;
}
#endif /* MS_WINDOWS */
@@ -257,7 +245,7 @@ trip_signal(int sig_num)
and then set the flag, but this allowed the following sequence of events
(especially on windows, where trip_signal may run in a new thread):
- - main thread blocks on select([wakeup_fd], ...)
+ - main thread blocks on select([wakeup.fd], ...)
- signal arrives
- trip_signal writes to the wakeup fd
- the main thread wakes up
@@ -274,41 +262,43 @@ trip_signal(int sig_num)
#ifdef MS_WINDOWS
fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int);
#else
- fd = wakeup_fd;
+ fd = wakeup.fd;
#endif
if (fd != INVALID_FD) {
byte = (unsigned char)sig_num;
#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() isn't signal-safe, but we
- still use it for this exceptional case. */
- Py_AddPendingCall(report_wakeup_send_error, NULL);
+ rc = send(fd, &byte, 1, 0);
+
+ if (rc < 0) {
+ int last_error = GetLastError();
+ if (wakeup.warn_on_full_buffer ||
+ last_error != WSAEWOULDBLOCK)
+ {
+ /* Py_AddPendingCall() isn't signal-safe, but we
+ still use it for this exceptional case. */
+ Py_AddPendingCall(report_wakeup_send_error,
+ (void *)(intptr_t) last_error);
+ }
}
}
else
#endif
{
- byte = (unsigned char)sig_num;
-
/* _Py_write_noraise() retries write() if write() is interrupted by
a signal (fails with EINTR). */
rc = _Py_write_noraise(fd, &byte, 1);
if (rc < 0) {
- /* Py_AddPendingCall() isn't signal-safe, but we
- still use it for this exceptional case. */
- Py_AddPendingCall(report_wakeup_write_error,
- (void *)(intptr_t)errno);
+ if (wakeup.warn_on_full_buffer ||
+ (errno != EWOULDBLOCK && errno != EAGAIN))
+ {
+ /* Py_AddPendingCall() isn't signal-safe, but we
+ still use it for this exceptional case. */
+ Py_AddPendingCall(report_wakeup_write_error,
+ (void *)(intptr_t)errno);
+ }
}
}
}
@@ -549,9 +539,13 @@ signal_siginterrupt_impl(PyObject *module, int signalnum, int flag)
static PyObject*
-signal_set_wakeup_fd(PyObject *self, PyObject *args)
+signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds)
{
struct _Py_stat_struct status;
+ static char *kwlist[] = {
+ "", "warn_on_full_buffer", NULL,
+ };
+ int warn_on_full_buffer = 1;
#ifdef MS_WINDOWS
PyObject *fdobj;
SOCKET_T sockfd, old_sockfd;
@@ -560,7 +554,8 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args)
PyObject *mod;
int is_socket;
- if (!PyArg_ParseTuple(args, "O:set_wakeup_fd", &fdobj))
+ if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$p:set_wakeup_fd", kwlist,
+ &fdobj, &warn_on_full_buffer))
return NULL;
sockfd = PyLong_AsSocket_t(fdobj);
@@ -569,7 +564,8 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args)
#else
int fd, old_fd;
- if (!PyArg_ParseTuple(args, "i:set_wakeup_fd", &fd))
+ if (!PyArg_ParseTupleAndKeywords(args, kwds, "i|$p:set_wakeup_fd", kwlist,
+ &fd, &warn_on_full_buffer))
return NULL;
#endif
@@ -620,6 +616,7 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args)
old_sockfd = wakeup.fd;
wakeup.fd = sockfd;
+ wakeup.warn_on_full_buffer = warn_on_full_buffer;
wakeup.use_send = is_socket;
if (old_sockfd != INVALID_FD)
@@ -644,15 +641,16 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args)
}
}
- old_fd = wakeup_fd;
- wakeup_fd = fd;
+ old_fd = wakeup.fd;
+ wakeup.fd = fd;
+ wakeup.warn_on_full_buffer = warn_on_full_buffer;
return PyLong_FromLong(old_fd);
#endif
}
PyDoc_STRVAR(set_wakeup_fd_doc,
-"set_wakeup_fd(fd) -> fd\n\
+"set_wakeup_fd(fd, *, warn_on_full_buffer=True) -> fd\n\
\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\
@@ -670,11 +668,11 @@ PySignal_SetWakeupFd(int fd)
#ifdef MS_WINDOWS
old_fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int);
- wakeup.fd = fd;
#else
- old_fd = wakeup_fd;
- wakeup_fd = fd;
+ old_fd = wakeup.fd;
#endif
+ wakeup.fd = fd;
+ wakeup.warn_on_full_buffer = 1;
return old_fd;
}
@@ -1155,7 +1153,7 @@ static PyMethodDef signal_methods[] = {
SIGNAL_GETITIMER_METHODDEF
SIGNAL_SIGNAL_METHODDEF
SIGNAL_GETSIGNAL_METHODDEF
- {"set_wakeup_fd", signal_set_wakeup_fd, METH_VARARGS, set_wakeup_fd_doc},
+ {"set_wakeup_fd", (PyCFunction)signal_set_wakeup_fd, METH_VARARGS | METH_KEYWORDS, set_wakeup_fd_doc},
SIGNAL_SIGINTERRUPT_METHODDEF
SIGNAL_PAUSE_METHODDEF
SIGNAL_PTHREAD_KILL_METHODDEF
More information about the Python-checkins
mailing list