From 4448c08451e947ccf197742d22fa49adba71e363 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 31 Mar 2015 11:48:34 +0200 Subject: Issue #23485: select.kqueue.control() is now retried when interrupted by a signal --- Doc/library/select.rst | 6 +++++ Doc/whatsnew/3.5.rst | 3 ++- Lib/selectors.py | 6 ++--- Lib/test/eintrdata/eintr_tester.py | 15 +++++++++++-- Modules/selectmodule.c | 45 ++++++++++++++++++++++++++++---------- 5 files changed, 57 insertions(+), 18 deletions(-) diff --git a/Doc/library/select.rst b/Doc/library/select.rst index 61f3835..e6f95fd 100644 --- a/Doc/library/select.rst +++ b/Doc/library/select.rst @@ -454,6 +454,12 @@ Kqueue Objects - max_events must be 0 or a positive integer - timeout in seconds (floats possible) + .. versionchanged:: 3.5 + The function is now retried with a recomputed timeout when interrupted by + a signal, except if the signal handler raises an exception (see + :pep:`475` for the rationale), instead of raising + :exc:`InterruptedError`. + .. _kevent-objects: diff --git a/Doc/whatsnew/3.5.rst b/Doc/whatsnew/3.5.rst index 6c2d521..e5fefae 100644 --- a/Doc/whatsnew/3.5.rst +++ b/Doc/whatsnew/3.5.rst @@ -621,7 +621,8 @@ Changes in the Python API - :func:`os.open`, :func:`open` - :func:`os.read`, :func:`os.write` - - :func:`select.select`, :func:`select.poll.poll`, :func:`select.epoll.poll` + - :func:`select.select`, :func:`select.poll.poll`, :func:`select.epoll.poll`, + :func:`select.kqueue.control` - :func:`time.sleep` * Before Python 3.5, a :class:`datetime.time` object was considered to be false diff --git a/Lib/selectors.py b/Lib/selectors.py index 2a0a44c..a3f2e78 100644 --- a/Lib/selectors.py +++ b/Lib/selectors.py @@ -549,11 +549,9 @@ if hasattr(select, 'kqueue'): def select(self, timeout=None): timeout = None if timeout is None else max(timeout, 0) max_ev = len(self._fd_to_key) + kev_list = self._kqueue.control(None, max_ev, timeout) + ready = [] - try: - kev_list = self._kqueue.control(None, max_ev, timeout) - except InterruptedError: - return ready for kev in kev_list: fd = kev.ident flag = kev.filter diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py index 0df9762..4e9b992 100644 --- a/Lib/test/eintrdata/eintr_tester.py +++ b/Lib/test/eintrdata/eintr_tester.py @@ -315,8 +315,8 @@ class SelectEINTRTest(EINTRBaseTest): def test_select(self): t0 = time.monotonic() select.select([], [], [], self.sleep_time) - self.stop_alarm() dt = time.monotonic() - t0 + self.stop_alarm() self.assertGreaterEqual(dt, self.sleep_time) @unittest.skipUnless(hasattr(select, 'poll'), 'need select.poll') @@ -325,8 +325,8 @@ class SelectEINTRTest(EINTRBaseTest): t0 = time.monotonic() poller.poll(self.sleep_time * 1e3) - self.stop_alarm() dt = time.monotonic() - t0 + self.stop_alarm() self.assertGreaterEqual(dt, self.sleep_time) @unittest.skipUnless(hasattr(select, 'epoll'), 'need select.epoll') @@ -336,8 +336,19 @@ class SelectEINTRTest(EINTRBaseTest): t0 = time.monotonic() poller.poll(self.sleep_time) + dt = time.monotonic() - t0 self.stop_alarm() + self.assertGreaterEqual(dt, self.sleep_time) + + @unittest.skipUnless(hasattr(select, 'kqueue'), 'need select.kqueue') + def test_kqueue(self): + kqueue = select.kqueue() + self.addCleanup(kqueue.close) + + t0 = time.monotonic() + kqueue.control(None, 1, self.sleep_time) dt = time.monotonic() - t0 + self.stop_alarm() self.assertGreaterEqual(dt, self.sleep_time) diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index 96be5ba..452525d 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -2083,8 +2083,9 @@ kqueue_queue_control(kqueue_queue_Object *self, PyObject *args) PyObject *result = NULL; struct kevent *evl = NULL; struct kevent *chl = NULL; - struct timespec timeout; + struct timespec timeoutspec; struct timespec *ptimeoutspec; + _PyTime_t timeout, deadline = 0; if (self->kqfd < 0) return kqueue_queue_err_closed(); @@ -2103,9 +2104,7 @@ kqueue_queue_control(kqueue_queue_Object *self, PyObject *args) ptimeoutspec = NULL; } else { - _PyTime_t ts; - - if (_PyTime_FromSecondsObject(&ts, + if (_PyTime_FromSecondsObject(&timeout, otimeout, _PyTime_ROUND_CEILING) < 0) { PyErr_Format(PyExc_TypeError, "timeout argument must be an number " @@ -2114,15 +2113,15 @@ kqueue_queue_control(kqueue_queue_Object *self, PyObject *args) return NULL; } - if (_PyTime_AsTimespec(ts, &timeout) == -1) + if (_PyTime_AsTimespec(timeout, &timeoutspec) == -1) return NULL; - if (timeout.tv_sec < 0) { + if (timeoutspec.tv_sec < 0) { PyErr_SetString(PyExc_ValueError, "timeout must be positive or None"); return NULL; } - ptimeoutspec = &timeout; + ptimeoutspec = &timeoutspec; } if (ch != NULL && ch != Py_None) { @@ -2167,10 +2166,34 @@ kqueue_queue_control(kqueue_queue_Object *self, PyObject *args) } } - Py_BEGIN_ALLOW_THREADS - gotevents = kevent(self->kqfd, chl, nchanges, - evl, nevents, ptimeoutspec); - Py_END_ALLOW_THREADS + if (ptimeoutspec) + deadline = _PyTime_GetMonotonicClock() + timeout; + + do { + Py_BEGIN_ALLOW_THREADS + errno = 0; + gotevents = kevent(self->kqfd, chl, nchanges, + evl, nevents, ptimeoutspec); + Py_END_ALLOW_THREADS + + if (errno != EINTR) + break; + + /* kevent() was interrupted by a signal */ + if (PyErr_CheckSignals()) + goto error; + + if (ptimeoutspec) { + timeout = deadline - _PyTime_GetMonotonicClock(); + if (timeout < 0) { + gotevents = 0; + break; + } + if (_PyTime_AsTimespec(timeout, &timeoutspec) == -1) + goto error; + /* retry kevent() with the recomputed timeout */ + } + } while (1); if (gotevents == -1) { PyErr_SetFromErrno(PyExc_OSError); -- cgit v0.12 From 45ca48b03d08cdc9959e0577bc84b845931112f6 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 31 Mar 2015 12:10:33 +0200 Subject: Issue #23485: select.devpoll.poll() is now retried when interrupted by a signal --- Doc/library/select.rst | 6 +++ Doc/whatsnew/3.5.rst | 4 +- Lib/selectors.py | 7 ++- Lib/test/eintrdata/eintr_tester.py | 11 ++++ Modules/selectmodule.c | 106 ++++++++++++++++++++++--------------- 5 files changed, 85 insertions(+), 49 deletions(-) diff --git a/Doc/library/select.rst b/Doc/library/select.rst index e6f95fd..a62dc84 100644 --- a/Doc/library/select.rst +++ b/Doc/library/select.rst @@ -249,6 +249,12 @@ object. returning. If *timeout* is omitted, -1, or :const:`None`, the call will block until there is an event for this poll object. + .. versionchanged:: 3.5 + The function is now retried with a recomputed timeout when interrupted by + a signal, except if the signal handler raises an exception (see + :pep:`475` for the rationale), instead of raising + :exc:`InterruptedError`. + .. _epoll-objects: diff --git a/Doc/whatsnew/3.5.rst b/Doc/whatsnew/3.5.rst index e5fefae..434c0a7 100644 --- a/Doc/whatsnew/3.5.rst +++ b/Doc/whatsnew/3.5.rst @@ -619,10 +619,10 @@ Changes in the Python API instead of raising :exc:`InterruptedError` if the signal handler does not raise an exception: - - :func:`os.open`, :func:`open` + - :func:`open`, :func:`os.open`, :func:`io.open` - :func:`os.read`, :func:`os.write` - :func:`select.select`, :func:`select.poll.poll`, :func:`select.epoll.poll`, - :func:`select.kqueue.control` + :func:`select.kqueue.control`, :func:`select.devpoll.poll` - :func:`time.sleep` * Before Python 3.5, a :class:`datetime.time` object was considered to be false diff --git a/Lib/selectors.py b/Lib/selectors.py index a3f2e78..44a6150 100644 --- a/Lib/selectors.py +++ b/Lib/selectors.py @@ -479,11 +479,10 @@ if hasattr(select, 'devpoll'): # devpoll() has a resolution of 1 millisecond, round away from # zero to wait *at least* timeout seconds. timeout = math.ceil(timeout * 1e3) + + fd_event_list = self._devpoll.poll(timeout) + ready = [] - try: - fd_event_list = self._devpoll.poll(timeout) - except InterruptedError: - return ready for fd, event in fd_event_list: events = 0 if event & ~select.POLLIN: diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py index 4e9b992..f755880 100644 --- a/Lib/test/eintrdata/eintr_tester.py +++ b/Lib/test/eintrdata/eintr_tester.py @@ -351,6 +351,17 @@ class SelectEINTRTest(EINTRBaseTest): self.stop_alarm() self.assertGreaterEqual(dt, self.sleep_time) + @unittest.skipUnless(hasattr(select, 'devpoll'), 'need select.devpoll') + def test_devpoll(self): + poller = select.devpoll() + self.addCleanup(poller.close) + + t0 = time.monotonic() + poller.poll(self.sleep_time * 1e3) + dt = time.monotonic() - t0 + self.stop_alarm() + self.assertGreaterEqual(dt, self.sleep_time) + def test_main(): support.run_unittest( diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index 452525d..590aeca 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -876,40 +876,38 @@ static PyObject * devpoll_poll(devpollObject *self, PyObject *args) { struct dvpoll dvp; - PyObject *result_list = NULL, *tout = NULL; + PyObject *result_list = NULL, *timeout_obj = NULL; int poll_result, i; - long timeout; PyObject *value, *num1, *num2; + _PyTime_t timeout, ms, deadline = 0; if (self->fd_devpoll < 0) return devpoll_err_closed(); - if (!PyArg_UnpackTuple(args, "poll", 0, 1, &tout)) { + if (!PyArg_ParseTuple(args, "|O:poll", &timeout_obj)) { return NULL; } /* Check values for timeout */ - if (tout == NULL || tout == Py_None) + if (timeout_obj == NULL || timeout_obj == Py_None) { timeout = -1; - else if (!PyNumber_Check(tout)) { - PyErr_SetString(PyExc_TypeError, - "timeout must be an integer or None"); - return NULL; + ms = -1; } else { - tout = PyNumber_Long(tout); - if (!tout) - return NULL; - timeout = PyLong_AsLong(tout); - Py_DECREF(tout); - if (timeout == -1 && PyErr_Occurred()) + if (_PyTime_FromMillisecondsObject(&timeout, timeout_obj, + _PyTime_ROUND_CEILING) < 0) { + if (PyErr_ExceptionMatches(PyExc_TypeError)) { + PyErr_SetString(PyExc_TypeError, + "timeout must be an integer or None"); + } return NULL; - } + } - if ((timeout < -1) || (timeout > INT_MAX)) { - PyErr_SetString(PyExc_OverflowError, - "timeout is out of range"); - return NULL; + ms = _PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING); + if (ms < -1 || ms > INT_MAX) { + PyErr_SetString(PyExc_OverflowError, "timeout is too large"); + return NULL; + } } if (devpoll_flush(self)) @@ -917,12 +915,36 @@ devpoll_poll(devpollObject *self, PyObject *args) dvp.dp_fds = self->fds; dvp.dp_nfds = self->max_n_fds; - dvp.dp_timeout = timeout; + dvp.dp_timeout = (int)ms; + + if (timeout >= 0) + deadline = _PyTime_GetMonotonicClock() + timeout; + + do { + /* call devpoll() */ + Py_BEGIN_ALLOW_THREADS + errno = 0; + poll_result = ioctl(self->fd_devpoll, DP_POLL, &dvp); + Py_END_ALLOW_THREADS + + if (errno != EINTR) + break; - /* call devpoll() */ - Py_BEGIN_ALLOW_THREADS - poll_result = ioctl(self->fd_devpoll, DP_POLL, &dvp); - Py_END_ALLOW_THREADS + /* devpoll() was interrupted by a signal */ + if (PyErr_CheckSignals()) + return NULL; + + if (timeout >= 0) { + timeout = deadline - _PyTime_GetMonotonicClock(); + if (timeout < 0) { + poll_result = 0; + break; + } + ms = _PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING); + dvp.dp_timeout = (int)ms; + /* retry devpoll() with the recomputed timeout */ + } + } while (1); if (poll_result < 0) { PyErr_SetFromErrno(PyExc_IOError); @@ -930,28 +952,26 @@ devpoll_poll(devpollObject *self, PyObject *args) } /* build the result list */ - result_list = PyList_New(poll_result); if (!result_list) return NULL; - else { - for (i = 0; i < poll_result; i++) { - num1 = PyLong_FromLong(self->fds[i].fd); - num2 = PyLong_FromLong(self->fds[i].revents); - if ((num1 == NULL) || (num2 == NULL)) { - Py_XDECREF(num1); - Py_XDECREF(num2); - goto error; - } - value = PyTuple_Pack(2, num1, num2); - Py_DECREF(num1); - Py_DECREF(num2); - if (value == NULL) - goto error; - if ((PyList_SetItem(result_list, i, value)) == -1) { - Py_DECREF(value); - goto error; - } + + for (i = 0; i < poll_result; i++) { + num1 = PyLong_FromLong(self->fds[i].fd); + num2 = PyLong_FromLong(self->fds[i].revents); + if ((num1 == NULL) || (num2 == NULL)) { + Py_XDECREF(num1); + Py_XDECREF(num2); + goto error; + } + value = PyTuple_Pack(2, num1, num2); + Py_DECREF(num1); + Py_DECREF(num2); + if (value == NULL) + goto error; + if ((PyList_SetItem(result_list, i, value)) == -1) { + Py_DECREF(value); + goto error; } } -- cgit v0.12 From b31017331994abcc6af3ce2febfc33593c3d7fec Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 31 Mar 2015 12:08:09 +0200 Subject: Issue #23485: Enhance and update selectors doc and test_selectors Selector.select() is now retried with the recomputed timeout when interrupted by a signal. Write an unit test with a signal handler raising an exception, and a unit with a signal handler which does not raise an exception (it does nothing). --- Doc/library/selectors.rst | 6 ++++++ Lib/test/test_selectors.py | 37 ++++++++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/Doc/library/selectors.rst b/Doc/library/selectors.rst index 8bd9e1c..f6ef24b 100644 --- a/Doc/library/selectors.rst +++ b/Doc/library/selectors.rst @@ -159,6 +159,12 @@ below: timeout has elapsed if the current process receives a signal: in this case, an empty list will be returned. + .. versionchanged:: 3.5 + The selector is now retried with a recomputed timeout when interrupted + by a signal if the signal handler did not raise an exception (see + :pep:`475` for the rationale), instead of returning an empty list + of events before the timeout. + .. method:: close() Close the selector. diff --git a/Lib/test/test_selectors.py b/Lib/test/test_selectors.py index 9521481..454c17b 100644 --- a/Lib/test/test_selectors.py +++ b/Lib/test/test_selectors.py @@ -357,7 +357,35 @@ class BaseSelectorTestCase(unittest.TestCase): @unittest.skipUnless(hasattr(signal, "alarm"), "signal.alarm() required for this test") - def test_select_interrupt(self): + def test_select_interrupt_exc(self): + s = self.SELECTOR() + self.addCleanup(s.close) + + rd, wr = self.make_socketpair() + + class InterruptSelect(Exception): + pass + + def handler(*args): + raise InterruptSelect + + orig_alrm_handler = signal.signal(signal.SIGALRM, handler) + self.addCleanup(signal.signal, signal.SIGALRM, orig_alrm_handler) + self.addCleanup(signal.alarm, 0) + + signal.alarm(1) + + s.register(rd, selectors.EVENT_READ) + t = time() + # select() is interrupted by a signal which raises an exception + with self.assertRaises(InterruptSelect): + s.select(30) + # select() was interrupted before the timeout of 30 seconds + self.assertLess(time() - t, 5.0) + + @unittest.skipUnless(hasattr(signal, "alarm"), + "signal.alarm() required for this test") + def test_select_interrupt_noraise(self): s = self.SELECTOR() self.addCleanup(s.close) @@ -371,8 +399,11 @@ class BaseSelectorTestCase(unittest.TestCase): s.register(rd, selectors.EVENT_READ) t = time() - self.assertFalse(s.select(2)) - self.assertLess(time() - t, 2.5) + # select() is interrupted by a signal, but the signal handler doesn't + # raise an exception, so select() should by retries with a recomputed + # timeout + self.assertFalse(s.select(1.5)) + self.assertGreaterEqual(time() - t, 1.0) class ScalableSelectorMixIn: -- cgit v0.12