[3.5] bpo-30320, bpo-25277: backport test_eintr enhancements from master to 3.5 ...

* bpo-30320: test_eintr now uses pthread_sigmask() (#1523)

Rewrite sigwaitinfo() and sigtimedwait() unit tests for EINTR using
pthread_sigmask() to fix a race condition between the child and the
parent process.

Remove the pipe which was used as a weak workaround against the race
condition.

sigtimedwait() is now tested with a child process sending a signal
instead of testing the timeout feature which is more unstable
(especially regarding to clock resolution depending on the platform).
(cherry picked from commit 211a392cc15f9a7b1b8ce65d8f6c9f8237d1b77f)

* test_eintr: Fix ResourceWarning warnings

(cherry picked from commit c50cccfcc3b3a9ef3fe7a78b7e7271930dc24aee)

* test_eintr: remove unused import

* bpo-25277: Add a watchdog to test_eintr

Set a timeout of 10 minutes in test_eintr using faulthandler.

diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py
index 81b6277a0401ff9748255c1f56836c5e4e3957b9..ad68bd799fa7cb3e9a4480c6fe4d3137326ae4d2 100644 (file)
--- a/Lib/test/eintrdata/eintr_tester.py
+++ b/Lib/test/eintrdata/eintr_tester.py
@@ -9,7 +9,7 @@ sub-second periodicity (contrarily to signal()).
 """
 
 import contextlib
-import io
+import faulthandler
 import os
 import select
 import signal
@@ -50,6 +50,10 @@ class EINTRBaseTest(unittest.TestCase):
         signal.setitimer(signal.ITIMER_REAL, cls.signal_delay,
                          cls.signal_period)
 
+        # Issue #25277: Use faulthandler to try to debug a hang on FreeBSD
+        if hasattr(faulthandler, 'dump_traceback_later'):
+            faulthandler.dump_traceback_later(10 * 60, exit=True)
+
     @classmethod
     def stop_alarm(cls):
         signal.setitimer(signal.ITIMER_REAL, 0, 0)
@@ -58,6 +62,8 @@ class EINTRBaseTest(unittest.TestCase):
     def tearDownClass(cls):
         cls.stop_alarm()
         signal.signal(signal.SIGALRM, cls.orig_handler)
+        if hasattr(faulthandler, 'cancel_dump_traceback_later'):
+            faulthandler.cancel_dump_traceback_later()
 
     def subprocess(self, *args, **kw):
         cmd_args = (sys.executable, '-c') + args
@@ -77,6 +83,9 @@ class OSEINTRTest(EINTRBaseTest):
         processes = [self.new_sleep_process() for _ in range(num)]
         for _ in range(num):
             wait_func()
+        # Call the Popen method to avoid a ResourceWarning
+        for proc in processes:
+            proc.wait()
 
     def test_wait(self):
         self._test_wait_multiple(os.wait)
@@ -88,6 +97,8 @@ class OSEINTRTest(EINTRBaseTest):
     def _test_wait_single(self, wait_func):
         proc = self.new_sleep_process()
         wait_func(proc.pid)
+        # Call the Popen method to avoid a ResourceWarning
+        proc.wait()
 
     def test_waitpid(self):
         self._test_wait_single(lambda pid: os.waitpid(pid, 0))
@@ -358,59 +369,55 @@ class TimeEINTRTest(EINTRBaseTest):
 
 
 @unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")
+# bpo-30320: Need pthread_sigmask() to block the signal, otherwise the test
+# is vulnerable to a race condition between the child and the parent processes.
+@unittest.skipUnless(hasattr(signal, 'pthread_sigmask'),
+                     'need signal.pthread_sigmask()')
 class SignalEINTRTest(EINTRBaseTest):
     """ EINTR tests for the signal module. """
 
-    @unittest.skipUnless(hasattr(signal, 'sigtimedwait'),
-                         'need signal.sigtimedwait()')
-    def test_sigtimedwait(self):
-        t0 = time.monotonic()
-        signal.sigtimedwait([signal.SIGUSR1], self.sleep_time)
-        dt = time.monotonic() - t0
-        self.assertGreaterEqual(dt, self.sleep_time)
-
-    @unittest.skipUnless(hasattr(signal, 'sigwaitinfo'),
-                         'need signal.sigwaitinfo()')
-    def test_sigwaitinfo(self):
-        # Issue #25277, #25868: give a few milliseconds to the parent process
-        # between os.write() and signal.sigwaitinfo() to works around a race
-        # condition
-        self.sleep_time = 0.100
-
+    def check_sigwait(self, wait_func):
         signum = signal.SIGUSR1
         pid = os.getpid()
 
         old_handler = signal.signal(signum, lambda *args: None)
         self.addCleanup(signal.signal, signum, old_handler)
 
-        rpipe, wpipe = os.pipe()
-
         code = '\n'.join((
             'import os, time',
             'pid = %s' % os.getpid(),
             'signum = %s' % int(signum),
             'sleep_time = %r' % self.sleep_time,
-            'rpipe = %r' % rpipe,
-            'os.read(rpipe, 1)',
-            'os.close(rpipe)',
             'time.sleep(sleep_time)',
             'os.kill(pid, signum)',
         ))
 
+        old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, [signum])
+        self.addCleanup(signal.pthread_sigmask, signal.SIG_UNBLOCK, [signum])
+
         t0 = time.monotonic()
-        proc = self.subprocess(code, pass_fds=(rpipe,))
-        os.close(rpipe)
+        proc = self.subprocess(code)
         with kill_on_error(proc):
-            # sync child-parent
-            os.write(wpipe, b'x')
-            os.close(wpipe)
+            wait_func(signum)
+            dt = time.monotonic() - t0
+
+        self.assertEqual(proc.wait(), 0)
 
-            # parent
+    @unittest.skipUnless(hasattr(signal, 'sigwaitinfo'),
+                         'need signal.sigwaitinfo()')
+    def test_sigwaitinfo(self):
+        def wait_func(signum):
             signal.sigwaitinfo([signum])
-            dt = time.monotonic() - t0
-            self.assertEqual(proc.wait(), 0)
 
-        self.assertGreaterEqual(dt, self.sleep_time)
+        self.check_sigwait(wait_func)
+
+    @unittest.skipUnless(hasattr(signal, 'sigtimedwait'),
+                         'need signal.sigwaitinfo()')
+    def test_sigtimedwait(self):
+        def wait_func(signum):
+            signal.sigtimedwait([signum], 120.0)
+
+        self.check_sigwait(wait_func)
 
 
 @unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")