From 897f197fc707d9db324105298734921fb50ebb1e Mon Sep 17 00:00:00 2001 From: Artur Jamro Date: Sat, 29 Nov 2025 03:04:52 +0100 Subject: [PATCH 1/2] gh-141473: Fix subprocess.Popen.communicate to send input to stdin upon a subsequent post-timeout call (GH-141477) * gh-141473: Fix subprocess.Popen.communicate to send input to stdin * Docs: Clarify that `input` is one time only on `communicate()` * NEWS entry * Add a regression test. --------- (cherry picked from commit 526d7a8bb47bd8ff58c829c30384cd70cc5d0747) Co-authored-by: Artur Jamro Co-authored-by: Gregory P. Smith --- Doc/library/subprocess.rst | 4 ++- Lib/subprocess.py | 2 +- Lib/test/test_subprocess.py | 34 +++++++++++++++++++ ...-11-27-20-16-38.gh-issue-141473.Wq4xVN.rst | 4 +++ 4 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-11-27-20-16-38.gh-issue-141473.Wq4xVN.rst diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 4390f00fe7cd55..45dfd9ed025813 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -831,7 +831,9 @@ Instances of the :class:`Popen` class have the following methods: If the process does not terminate after *timeout* seconds, a :exc:`TimeoutExpired` exception will be raised. Catching this exception and - retrying communication will not lose any output. + retrying communication will not lose any output. Supplying *input* to a + subsequent post-timeout :meth:`communicate` call is in undefined behavior + and may become an error in the future. The child process is not killed if the timeout expires, so in order to cleanup properly a well-behaved application should kill the child process and diff --git a/Lib/subprocess.py b/Lib/subprocess.py index ffc3fdf6afb5c7..8bf06f111a26e6 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -2108,7 +2108,7 @@ def _communicate(self, input, endtime, orig_timeout): input_view = memoryview(self._input) with _PopenSelector() as selector: - if self.stdin and input: + if self.stdin and not self.stdin.closed and self._input: selector.register(self.stdin, selectors.EVENT_WRITE) if self.stdout and not self.stdout.closed: selector.register(self.stdout, selectors.EVENT_READ) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 8b2f48b0fc0119..6a5d626663e2f6 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1642,6 +1642,40 @@ def test_wait_negative_timeout(self): self.assertEqual(proc.wait(), 0) + def test_post_timeout_communicate_sends_input(self): + """GH-141473 regression test; the stdin pipe must close""" + with subprocess.Popen( + [sys.executable, "-uc", """\ +import sys +while c := sys.stdin.read(512): + sys.stdout.write(c) +print() +"""], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) as proc: + try: + data = f"spam{'#'*4096}beans" + proc.communicate( + input=data, + timeout=0, + ) + except subprocess.TimeoutExpired as exc: + pass + # Prior to the bugfix, this would hang as the stdin + # pipe to the child had not been closed. + try: + stdout, stderr = proc.communicate(timeout=15) + except subprocess.TimeoutExpired as exc: + self.fail("communicate() hung waiting on child process that should have seen its stdin pipe close and exit") + self.assertEqual( + proc.returncode, 0, + msg=f"STDERR:\n{stderr}\nSTDOUT:\n{stdout}") + self.assertStartsWith(stdout, "spam") + self.assertIn("beans", stdout) + class RunFuncTestCase(BaseTestCase): def run_python(self, code, **kwargs): diff --git a/Misc/NEWS.d/next/Library/2025-11-27-20-16-38.gh-issue-141473.Wq4xVN.rst b/Misc/NEWS.d/next/Library/2025-11-27-20-16-38.gh-issue-141473.Wq4xVN.rst new file mode 100644 index 00000000000000..f6aa592cefda35 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-11-27-20-16-38.gh-issue-141473.Wq4xVN.rst @@ -0,0 +1,4 @@ +When :meth:`subprocess.Popen.communicate` was called with *input* and a +*timeout* and is called for a second time after a +:exc:`~subprocess.TimeoutExpired` exception before the process has died, it +should no longer hang. From f32202f529b12d45425aeca97b477fc7a412abaf Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 29 Nov 2025 06:29:18 +0000 Subject: [PATCH 2/2] no assertStartsWith --- Lib/test/test_subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 6a5d626663e2f6..f1ab1220f874e3 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1673,7 +1673,7 @@ def test_post_timeout_communicate_sends_input(self): self.assertEqual( proc.returncode, 0, msg=f"STDERR:\n{stderr}\nSTDOUT:\n{stdout}") - self.assertStartsWith(stdout, "spam") + self.assertTrue(stdout.startswith("spam"), msg=stdout) self.assertIn("beans", stdout)