From de856d97edc0bc887696bab53e6a5f37898b5bb9 Mon Sep 17 00:00:00 2001 From: Artur Jamro Date: Wed, 12 Nov 2025 19:35:06 +0100 Subject: [PATCH 1/5] gh-141473: Fix subprocess.Popen.communicate to send input to stdin --- Lib/subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 79251bd5310223..4955f77b60381f 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -2105,7 +2105,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) From 767b951b97d1a83ee2cda9c74997f4905397de5a Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 27 Nov 2025 20:37:23 +0000 Subject: [PATCH 2/5] Docs: Clarify that `input` is one time only on `communicate()` --- Doc/library/subprocess.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 1aade881745f21..43da804b62beb1 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 From 9bb75167294a876773dc09384ca4a9218e2c4a3e Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 27 Nov 2025 20:38:05 +0000 Subject: [PATCH 3/5] NEWS entry --- .../Library/2025-11-27-20-16-38.gh-issue-141473.Wq4xVN.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-11-27-20-16-38.gh-issue-141473.Wq4xVN.rst 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 76a262292e270bb7a91579994664b56c99bbd576 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 27 Nov 2025 21:36:35 +0000 Subject: [PATCH 4/5] Add a regression test. --- Lib/test/test_subprocess.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index f0e350c71f60ea..e063e829f9cd71 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1643,6 +1643,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{'#'*subprocess._PIPE_BUF}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): From 1bc972ce3c1f1fbb76fbafac0b2dffc0290215f2 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 27 Nov 2025 22:07:38 +0000 Subject: [PATCH 5/5] fix test on windows --- 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 e063e829f9cd71..9792d7c8983cd3 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1658,7 +1658,7 @@ def test_post_timeout_communicate_sends_input(self): text=True, ) as proc: try: - data = f"spam{'#'*subprocess._PIPE_BUF}beans" + data = f"spam{'#'*4096}beans" proc.communicate( input=data, timeout=0,