Handle rare case where poll() says we're done, but waitpid() doesn't

Author: giampaolo

Lib/subprocess.py

@@ -2122,6 +2122,9 @@ def _wait(self, timeout):
                 return self.returncode
 
             if timeout is not None:
+                started = _time()
+                endtime = started + timeout
+
                 # Try efficient wait first.
                 if self._wait_pidfd(timeout) or self._wait_kqueue(timeout):
                     # Process is gone. At this point os.waitpid(pid, 0)
@@ -2136,11 +2139,17 @@ def _wait(self, timeout):
                         assert pid == self.pid or pid == 0
                         if pid == self.pid:
                             self._handle_exitstatus(sts)
-                        return self.returncode
+                            return self.returncode
+                        # os.waitpid(pid, WNOHANG) returned 0 instead
+                        # of our PID, meaning PID has not yet exited,
+                        # even though poll() / kqueue() said so. Very
+                        # rare and mostly theoretical. Fallback to busy
+                        # polling.
+                        elapsed = _time() - started
+                        endtime -= elapsed
 
                 # Enter a busy loop if we have a timeout.  This busy loop was
                 # cribbed from Lib/threading.py in Thread.wait() at r71065.
-                endtime = _time() + timeout
                 delay = 0.0005 # 500 us -> initial delay of 1 ms
                 while True:
                     if self._waitpid_lock.acquire(False):

Lib/test/test_subprocess.py

@@ -4091,8 +4091,8 @@ def test_broken_pipe_cleanup(self):
 
 
 class FastWaitTestCase(BaseTestCase):
-    """Tests for efficient (pidfd_open / kqueue) process waiting in
-    subprocess.Popen.wait().
+    """Tests for efficient (pidfd_open() + poll() / kqueue()) process
+    waiting in subprocess.Popen.wait().
     """
     CAN_USE_PIDFD_OPEN = hasattr(os, "pidfd_open")
     CAN_USE_KQUEUE = subprocess._CAN_USE_KQUEUE
@@ -4164,6 +4164,36 @@ def test_pidfd_open_race(self):
     def test_kqueue_race(self):
         self.assert_wait_race_condition("select.kqueue", select.kqueue)
 
+    def assert_notification_without_immediate_reap(self, patch_target):
+        # Verify fallback to busy polling when poll() / kqueue()
+        # succeeds, but waitpid(pid, WNOHANG) returns (0, 0).
+        def waitpid_wrapper(pid, flags):
+            nonlocal ncalls
+            ncalls += 1
+            if ncalls == 1:
+                return (0, 0)
+            return real_waitpid(pid, flags)
+
+        ncalls = 0
+        real_waitpid = os.waitpid
+        with mock.patch.object(subprocess.Popen, patch_target, return_value=True) as m1:
+            with mock.patch("os.waitpid", side_effect=waitpid_wrapper) as m2:
+                p = subprocess.Popen([sys.executable,
+                                      "-c", "import time; time.sleep(0.3)"])
+                with self.assertRaises(subprocess.TimeoutExpired):
+                    p.wait(timeout=0.0001)
+                self.assertEqual(p.wait(timeout=support.SHORT_TIMEOUT), 0)
+        assert m1.called
+        assert m2.called
+
+    @unittest.skipIf(not CAN_USE_PIDFD_OPEN, reason="LINUX only")
+    def test_poll_notification_without_immediate_reap(self):
+        self.assert_notification_without_immediate_reap("_wait_pidfd")
+
+    @unittest.skipIf(not CAN_USE_KQUEUE, reason="macOS / BSD only")
+    def test_kqueue_notification_without_immediate_reap(self):
+        self.assert_notification_without_immediate_reap("_wait_kqueue")
+
 
 if __name__ == "__main__":
     unittest.main()