From ea5a3765fb0c1b0f38bc678fa7782aaf44fa6370 Mon Sep 17 00:00:00 2001 From: writtic Date: Fri, 15 Aug 2025 15:24:07 +0900 Subject: [PATCH 1/6] Fix subprocess.Popen.wait to raise ProcessLookupError for external waits --- Lib/subprocess.py | 14 ++++++++++---- Lib/test/test_asyncio/test_subprocess.py | 6 +++++- Lib/test/test_subprocess.py | 19 +++++++++++++++++++ ...5-08-15-14-06-33.gh-issue-96863.rUk3aJ.rst | 3 +++ 4 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-08-15-14-06-33.gh-issue-96863.rUk3aJ.rst 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_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 3a17c169c34f12..5aeae9351896d3 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -607,7 +607,11 @@ 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 + 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..1e55758f0986ce 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3361,6 +3361,25 @@ 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 + exc = cm.exception + self.assertIn('%d is already waited on externally' % proc.pid, str(exc)) + 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. From d8af14f621bc02423ae7e7f12b4ea87bce07edde Mon Sep 17 00:00:00 2001 From: writtic Date: Fri, 15 Aug 2025 15:37:23 +0900 Subject: [PATCH 2/6] Ensure ProcessLookupError is raised when waiting for killed process --- Lib/test/test_asyncio/test_subprocess.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 5aeae9351896d3..4828e8ea7a0580 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -607,11 +607,8 @@ async def kill_running(): # kill the process (but asyncio is not notified immediately) proc.kill() - try: + with self.assertRaises(ProcessLookupError): proc.wait() - except ProcessLookupError: - # Process already exited, which is expected - pass proc.kill = mock.Mock() proc_returncode = proc.poll() From cb8d8740e636fc38e9d27d74238b0726c2a37b11 Mon Sep 17 00:00:00 2001 From: writtic Date: Fri, 15 Aug 2025 15:49:24 +0900 Subject: [PATCH 3/6] =?UTF-8?q?Handle=20ProcessLookupError=20with=20a=20tr?= =?UTF-8?q?y=E2=80=93except=20block,=20since=20it=E2=80=99s=20not=20always?= =?UTF-8?q?=20raised.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Lib/test/test_asyncio/test_subprocess.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 4828e8ea7a0580..474903d7b04174 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -607,8 +607,12 @@ async def kill_running(): # kill the process (but asyncio is not notified immediately) proc.kill() - with self.assertRaises(ProcessLookupError): + try: proc.wait() + except ProcessLookupError: + # Process already exited, which is expected but not always + # raised + pass proc.kill = mock.Mock() proc_returncode = proc.poll() From a542c71a9f8255b22c836898c522ead518c7d544 Mon Sep 17 00:00:00 2001 From: writtic Date: Fri, 15 Aug 2025 16:02:36 +0900 Subject: [PATCH 4/6] Handle ProcessLookupError in wait calls for subprocesses in test_eintr --- Lib/test/_test_eintr.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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)) From f77e2eaed859f2c72e28b1f404048990ad8490db Mon Sep 17 00:00:00 2001 From: writtic Date: Fri, 15 Aug 2025 20:47:02 +0900 Subject: [PATCH 5/6] Ensure process is killed in test_wait_after_external_wait --- Lib/test/test_subprocess.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 1e55758f0986ce..d5ed75e9ed66c5 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3379,6 +3379,7 @@ def test_wait_after_external_wait(self): # Verify the exception contains proper information exc = cm.exception self.assertIn('%d is already waited on externally' % proc.pid, str(exc)) + proc.kill() def test_send_signal_race(self): # bpo-38630: send_signal() must poll the process exit status to reduce From 327f904a2ec51f707038dda4f34d2599828190ce Mon Sep 17 00:00:00 2001 From: writtic Date: Fri, 15 Aug 2025 20:47:34 +0900 Subject: [PATCH 6/6] Use f-string for exception message in test_wait_after_external_wait --- Lib/test/test_subprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index d5ed75e9ed66c5..681e97f61a5045 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3377,8 +3377,8 @@ def test_wait_after_external_wait(self): proc.wait() # Verify the exception contains proper information - exc = cm.exception - self.assertIn('%d is already waited on externally' % proc.pid, str(exc)) + self.assertIn(f'{proc.pid} is already waited on externally', + str(cm.exception)) proc.kill() def test_send_signal_race(self):