Issue #23715: signal.sigwaitinfo() and signal.sigtimedwait() are now retried

when interrupted by a signal not in the *sigset* parameter, if the signal
handler does not raise an exception. signal.sigtimedwait() recomputes the
timeout with a monotonic clock when it is retried.

Remove test_signal.test_sigwaitinfo_interrupted() because sigwaitinfo() doesn't
raise InterruptedError anymore if it is interrupted by a signal not in its
sigset parameter.

diff --git a/Doc/library/signal.rst b/Doc/library/signal.rst
index ed616b2e95e98ed4e3c7b342d05fc1bd319e48b6..0156415ad93e928ea5c04016da7d790a881a518f 100644 (file)
--- a/Doc/library/signal.rst
+++ b/Doc/library/signal.rst
@@ -408,6 +408,11 @@ The :mod:`signal` module defines the following functions:
 
    .. versionadded:: 3.3
 
+   .. versionchanged:: 3.5
+      The function is now retried if interrupted by a signal not in *sigset*
+      and the signal handler does not raise an exception (see :pep:`475` for
+      the rationale).
+
 
 .. function:: sigtimedwait(sigset, timeout)
 
@@ -422,6 +427,11 @@ The :mod:`signal` module defines the following functions:
 
    .. versionadded:: 3.3
 
+   .. versionchanged:: 3.5
+      The function is now retried with the recomputed timeout if interrupted by
+      a signal not in *sigset* and the signal handler does not raise an
+      exception (see :pep:`475` for the rationale).
+
 
 .. _signal-example:
 

diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py
index 64db2e55dc3c2487327a2b5d473a63b28c2b81dc..2d2876ee13543805167467ad788d5e6269394bf9 100644 (file)
--- a/Lib/test/eintrdata/eintr_tester.py
+++ b/Lib/test/eintrdata/eintr_tester.py
@@ -264,11 +264,47 @@ class TimeEINTRTest(EINTRBaseTest):
         self.assertGreaterEqual(dt, self.sleep_time)
 
 
+@unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")
+class SignalEINTRTest(EINTRBaseTest):
+    """ EINTR tests for the signal module. """
+
+    def test_sigtimedwait(self):
+        t0 = time.monotonic()
+        signal.sigtimedwait([], self.sleep_time)
+        dt = time.monotonic() - t0
+        self.assertGreaterEqual(dt, self.sleep_time)
+
+    def test_sigwaitinfo(self):
+        signum = signal.SIGUSR1
+        pid = os.getpid()
+
+        old_handler = signal.signal(signum, lambda *args: None)
+        self.addCleanup(signal.signal, signum, old_handler)
+
+        t0 = time.monotonic()
+        child_pid = os.fork()
+        if child_pid == 0:
+            # child
+            try:
+                self._sleep()
+                os.kill(pid, signum)
+            finally:
+                os._exit(0)
+        else:
+            # parent
+            signal.sigwaitinfo([signum])
+            dt = time.monotonic() - t0
+            os.waitpid(child_pid, 0)
+
+        self.assertGreaterEqual(dt, self.sleep_time)
+
+
 def test_main():
     support.run_unittest(
         OSEINTRTest,
         SocketEINTRTest,
-        TimeEINTRTest)
+        TimeEINTRTest,
+        SignalEINTRTest)
 
 
 if __name__ == "__main__":

diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py
index 4e7cbe225d1e1830f3af4aaba1609a657e8fc29d..8718eae689c39ce739ba69c1e663ad11f5b67132 100644 (file)
--- a/Lib/test/test_signal.py
+++ b/Lib/test/test_signal.py
@@ -936,35 +936,6 @@ class PendingSignalsTests(unittest.TestCase):
         signum = signal.SIGALRM
         self.assertRaises(ValueError, signal.sigtimedwait, [signum], -1.0)
 
-    @unittest.skipUnless(hasattr(signal, 'sigwaitinfo'),
-                         'need signal.sigwaitinfo()')
-    # Issue #18238: sigwaitinfo() can be interrupted on Linux (raises
-    # InterruptedError), but not on AIX
-    @unittest.skipIf(sys.platform.startswith("aix"),
-                     'signal.sigwaitinfo() cannot be interrupted on AIX')
-    def test_sigwaitinfo_interrupted(self):
-        self.wait_helper(signal.SIGUSR1, '''
-        def test(signum):
-            import errno
-
-            hndl_called = True
-            def alarm_handler(signum, frame):
-                hndl_called = False
-
-            signal.signal(signal.SIGALRM, alarm_handler)
-            signal.alarm(1)
-            try:
-                signal.sigwaitinfo([signal.SIGUSR1])
-            except OSError as e:
-                if e.errno == errno.EINTR:
-                    if not hndl_called:
-                        raise Exception("SIGALRM handler not called")
-                else:
-                    raise Exception("Expected EINTR to be raised by sigwaitinfo")
-            else:
-                raise Exception("Expected EINTR to be raised by sigwaitinfo")
-        ''')
-
     @unittest.skipUnless(hasattr(signal, 'sigwait'),
                          'need signal.sigwait()')
     @unittest.skipUnless(hasattr(signal, 'pthread_sigmask'),

diff --git a/Misc/NEWS b/Misc/NEWS
index f3dab008fb9160197c80912eefccb3c97e0d8791..455c0c1c27a385182d7f603250aef0c5e3d273c6 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -21,6 +21,11 @@ Core and Builtins
 Library
 -------
 
+- Issue #23715: :func:`signal.sigwaitinfo` and :func:`signal.sigtimedwait` are
+  now retried when interrupted by a signal not in the *sigset* parameter, if
+  the signal handler does not raise an exception. signal.sigtimedwait()
+  recomputes the timeout with a monotonic clock when it is retried.
+
 - Issue #23001: Few functions in modules mmap, ossaudiodev, socket, ssl, and
   codecs, that accepted only read-only bytes-like object now accept writable
   bytes-like object too.

diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index 3ad8ebb8cd12fc7cbbd444bc68e0256f4d4874ef..5dba8b1fd32a07de74cc8af6833ebeb1a2a96d84 100644 (file)
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -934,6 +934,7 @@ signal_sigwaitinfo(PyObject *self, PyObject *args)
     sigset_t set;
     siginfo_t si;
     int err;
+    int async_err = 0;
 
     if (!PyArg_ParseTuple(args, "O:sigwaitinfo", &signals))
         return NULL;
@@ -941,11 +942,14 @@ signal_sigwaitinfo(PyObject *self, PyObject *args)
     if (iterable_to_sigset(signals, &set))
         return NULL;
 
-    Py_BEGIN_ALLOW_THREADS
-    err = sigwaitinfo(&set, &si);
-    Py_END_ALLOW_THREADS
+    do {
+        Py_BEGIN_ALLOW_THREADS
+        err = sigwaitinfo(&set, &si);
+        Py_END_ALLOW_THREADS
+    } while (err == -1
+             && errno == EINTR && !(async_err = PyErr_CheckSignals()));
     if (err == -1)
-        return PyErr_SetFromErrno(PyExc_OSError);
+        return (!async_err) ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
 
     return fill_siginfo(&si);
 }
@@ -962,25 +966,19 @@ Returns a struct_siginfo containing information about the signal.");
 static PyObject *
 signal_sigtimedwait(PyObject *self, PyObject *args)
 {
-    PyObject *signals, *timeout;
-    struct timespec buf;
+    PyObject *signals;
+    double timeout, frac;
+    struct timespec ts;
     sigset_t set;
     siginfo_t si;
-    time_t tv_sec;
-    long tv_nsec;
-    int err;
+    int res;
+    _PyTime_timeval deadline, monotonic;
 
-    if (!PyArg_ParseTuple(args, "OO:sigtimedwait",
+    if (!PyArg_ParseTuple(args, "Od:sigtimedwait",
                           &signals, &timeout))
         return NULL;
 
-    if (_PyTime_ObjectToTimespec(timeout, &tv_sec, &tv_nsec,
-                                 _PyTime_ROUND_DOWN) == -1)
-        return NULL;
-    buf.tv_sec = tv_sec;
-    buf.tv_nsec = tv_nsec;
-
-    if (buf.tv_sec < 0 || buf.tv_nsec < 0) {
+    if (timeout < 0) {
         PyErr_SetString(PyExc_ValueError, "timeout must be non-negative");
         return NULL;
     }
@@ -988,15 +986,38 @@ signal_sigtimedwait(PyObject *self, PyObject *args)
     if (iterable_to_sigset(signals, &set))
         return NULL;
 
-    Py_BEGIN_ALLOW_THREADS
-    err = sigtimedwait(&set, &si, &buf);
-    Py_END_ALLOW_THREADS
-    if (err == -1) {
-        if (errno == EAGAIN)
-            Py_RETURN_NONE;
-        else
-            return PyErr_SetFromErrno(PyExc_OSError);
-    }
+    _PyTime_monotonic(&deadline);
+    _PyTime_AddDouble(&deadline, timeout, _PyTime_ROUND_UP);
+
+    do {
+        frac = fmod(timeout, 1.0);
+        timeout = floor(timeout);
+        ts.tv_sec = (long)timeout;
+        ts.tv_nsec = (long)(frac*1e9);
+
+        Py_BEGIN_ALLOW_THREADS
+        res = sigtimedwait(&set, &si, &ts);
+        Py_END_ALLOW_THREADS
+
+        if (res != -1)
+            break;
+
+        if (errno != EINTR) {
+            if (errno == EAGAIN)
+                Py_RETURN_NONE;
+            else
+                return PyErr_SetFromErrno(PyExc_OSError);
+        }
+
+        /* sigtimedwait() was interrupted by a signal (EINTR) */
+        if (PyErr_CheckSignals())
+            return NULL;
+
+        _PyTime_monotonic(&monotonic);
+        timeout = _PyTime_INTERVAL(monotonic, deadline);
+        if (timeout <= 0.0)
+            break;
+    } while (1);
 
     return fill_siginfo(&si);
 }