From dab10f4f5b52c6de1aac3a8b5dc87d2eb0223a6c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 10 May 2017 14:13:37 +0200 Subject: [PATCH] [3.5] bpo-30320, bpo-25277: backport test_eintr enhancements from master to 3.5 (#1532) * 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. --- Lib/test/eintrdata/eintr_tester.py | 69 ++++++++++++++++-------------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py index 81b6277a0401..ad68bd799fa7 100644 --- 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()") -- 2.47.3