bpo-35426: Eliminate race condition in test_interprocess_signal (GH-11087)

pablogsal · web-flow · commit 2ab2afd38708 · 2018-12-11T11:32:12.000Z
The test only except SIGUSR1Exception inside wait_signal(), but the signal can be sent during subprocess_send_signal() call.
diff --git a/Lib/test/signalinterproctester.py b/Lib/test/signalinterproctester.py
@@ -21,25 +21,19 @@ def sigusr1_handler(self, signum, frame):
         self.got_signals['SIGUSR1'] += 1
         raise SIGUSR1Exception
 
-    def wait_signal(self, child, signame, exc_class=None):
-        try:
-            if child is not None:
-                # This wait should be interrupted by exc_class
-                # (if set)
-                child.wait()
-
-            timeout = 10.0
-            deadline = time.monotonic() + timeout
-
-            while time.monotonic() < deadline:
-                if self.got_signals[signame]:
-                    return
-                signal.pause()
-        except BaseException as exc:
-            if exc_class is not None and isinstance(exc, exc_class):
-                # got the expected exception
+    def wait_signal(self, child, signame):
+        if child is not None:
+            # This wait should be interrupted by exc_class
+            # (if set)
+            child.wait()
+
+        timeout = 10.0
+        deadline = time.monotonic() + timeout
+
+        while time.monotonic() < deadline:
+            if self.got_signals[signame]:
                 return
-            raise
+            signal.pause()
 
         self.fail('signal %s not received after %s seconds'
                   % (signame, timeout))
@@ -65,8 +59,9 @@ def test_interprocess_signal(self):
         self.assertEqual(self.got_signals, {'SIGHUP': 1, 'SIGUSR1': 0,
                                             'SIGALRM': 0})
 
-        with self.subprocess_send_signal(pid, "SIGUSR1") as child:
-            self.wait_signal(child, 'SIGUSR1', SIGUSR1Exception)
+        with self.assertRaises(SIGUSR1Exception):
+            with self.subprocess_send_signal(pid, "SIGUSR1") as child:
+                self.wait_signal(child, 'SIGUSR1')
         self.assertEqual(self.got_signals, {'SIGHUP': 1, 'SIGUSR1': 1,
                                             'SIGALRM': 0})
 
@@ -75,8 +70,9 @@ def test_interprocess_signal(self):
             child.wait()
 
         try:
-            signal.alarm(1)
-            self.wait_signal(None, 'SIGALRM', KeyboardInterrupt)
+            with self.assertRaises(KeyboardInterrupt):
+                signal.alarm(1)
+                self.wait_signal(None, 'SIGALRM')
             self.assertEqual(self.got_signals, {'SIGHUP': 1, 'SIGUSR1': 1,
                                                 'SIGALRM': 0})
         finally:
