diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 79251bd5310223..84ff169ddb9a95 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -2020,11 +2020,17 @@ def _try_wait(self, wait_flags): try: (pid, sts) = os.waitpid(self.pid, wait_flags) except ChildProcessError: - # This happens if SIGCLD is set to be ignored or waiting + # This happens if SIGCHLD is set to be ignored or waiting # for child processes has otherwise been disabled for our - # process. This child is dead, we can't get the status. - pid = self.pid - sts = 0 + # process. This child is dead, we can't get the status. + if signal.getsignal(signal.SIGCHLD) == signal.SIG_IGN: + # SIGCHLD was ignored; for compatibility return a zero + # status for this child. + pid = self.pid + sts = 0 + return (pid, sts) + + raise ProcessLookupError(f'Process {self.pid} is already waited on externally') return (pid, sts) diff --git a/Lib/test/_test_eintr.py b/Lib/test/_test_eintr.py index 4a050792df73c4..7822139575c59b 100644 --- a/Lib/test/_test_eintr.py +++ b/Lib/test/_test_eintr.py @@ -101,7 +101,11 @@ def _test_wait_multiple(self, wait_func): wait_func() # Call the Popen method to avoid a ResourceWarning for proc in processes: - proc.wait() + try: + proc.wait() + except ProcessLookupError: + # Process already exited, which is expected but not always raised + pass def test_wait(self): self._test_wait_multiple(os.wait) @@ -114,7 +118,11 @@ 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() + try: + proc.wait() + except ProcessLookupError: + # Process already exited, which is expected but not always raised + pass def test_waitpid(self): self._test_wait_single(lambda pid: os.waitpid(pid, 0)) diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 3a17c169c34f12..474903d7b04174 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -607,7 +607,12 @@ async def kill_running(): # kill the process (but asyncio is not notified immediately) proc.kill() - proc.wait() + try: + proc.wait() + except ProcessLookupError: + # Process already exited, which is expected but not always + # raised + pass proc.kill = mock.Mock() proc_returncode = proc.poll() diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index f0e350c71f60ea..681e97f61a5045 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3361,6 +3361,26 @@ def test_stopped(self): self.assertEqual(returncode, -3) + def test_wait_after_external_wait(self): + """Test behavior when process is already waited on externally; issue96863.""" + # Create a process with non-zero exit code + proc = subprocess.Popen([sys.executable, '-c', 'import sys; sys.exit(1)']) + + # Wait for the process externally using os.wait() + pid, status = os.wait() + self.assertEqual(pid, proc.pid) + self.assertEqual(os.WEXITSTATUS(status), 1) + + # Now calling proc.wait() should raise ProcessLookupError + # instead of returning 0 (the old buggy behavior) + with self.assertRaises(ProcessLookupError) as cm: + proc.wait() + + # Verify the exception contains proper information + self.assertIn(f'{proc.pid} is already waited on externally', + str(cm.exception)) + proc.kill() + def test_send_signal_race(self): # bpo-38630: send_signal() must poll the process exit status to reduce # the risk of sending the signal to the wrong process. diff --git a/Misc/NEWS.d/next/Library/2025-08-15-14-06-33.gh-issue-96863.rUk3aJ.rst b/Misc/NEWS.d/next/Library/2025-08-15-14-06-33.gh-issue-96863.rUk3aJ.rst new file mode 100644 index 00000000000000..569aab29e77223 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-08-15-14-06-33.gh-issue-96863.rUk3aJ.rst @@ -0,0 +1,3 @@ +Fix :meth:`subprocess.Popen.wait` returning zero always when process already +waited on externally. Now raises :exc:`ProcessLookupError` instead of losing +exit status. Preserves ``SIGCHLD=SIG_IGN`` compatibility.