Skip to content

Commit 2f3024f

Browse files
miss-islingtonmrowqagpshead
authored
[3.13] gh-141473: Fix subprocess.Popen.communicate to send input to stdin upon a subsequent post-timeout call (GH-141477) (#142060)
* 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 526d7a8) Co-authored-by: Artur Jamro <artur.jamro@gmail.com> Co-authored-by: Gregory P. Smith <greg@krypto.org> * no assertStartsWith --------- Co-authored-by: Artur Jamro <artur.jamro@gmail.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent 704bb69 commit 2f3024f

File tree

4 files changed

+42
-2
lines changed

4 files changed

+42
-2
lines changed

Doc/library/subprocess.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,9 @@ Instances of the :class:`Popen` class have the following methods:
831831

832832
If the process does not terminate after *timeout* seconds, a
833833
:exc:`TimeoutExpired` exception will be raised. Catching this exception and
834-
retrying communication will not lose any output.
834+
retrying communication will not lose any output. Supplying *input* to a
835+
subsequent post-timeout :meth:`communicate` call is in undefined behavior
836+
and may become an error in the future.
835837

836838
The child process is not killed if the timeout expires, so in order to
837839
cleanup properly a well-behaved application should kill the child process and

Lib/subprocess.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2111,7 +2111,7 @@ def _communicate(self, input, endtime, orig_timeout):
21112111
input_view = self._input.cast("b") # byte input required
21122112

21132113
with _PopenSelector() as selector:
2114-
if self.stdin and input:
2114+
if self.stdin and not self.stdin.closed and self._input:
21152115
selector.register(self.stdin, selectors.EVENT_WRITE)
21162116
if self.stdout and not self.stdout.closed:
21172117
selector.register(self.stdout, selectors.EVENT_READ)

Lib/test/test_subprocess.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,6 +1684,40 @@ def test_wait_negative_timeout(self):
16841684

16851685
self.assertEqual(proc.wait(), 0)
16861686

1687+
def test_post_timeout_communicate_sends_input(self):
1688+
"""GH-141473 regression test; the stdin pipe must close"""
1689+
with subprocess.Popen(
1690+
[sys.executable, "-uc", """\
1691+
import sys
1692+
while c := sys.stdin.read(512):
1693+
sys.stdout.write(c)
1694+
print()
1695+
"""],
1696+
stdin=subprocess.PIPE,
1697+
stdout=subprocess.PIPE,
1698+
stderr=subprocess.PIPE,
1699+
text=True,
1700+
) as proc:
1701+
try:
1702+
data = f"spam{'#'*4096}beans"
1703+
proc.communicate(
1704+
input=data,
1705+
timeout=0,
1706+
)
1707+
except subprocess.TimeoutExpired as exc:
1708+
pass
1709+
# Prior to the bugfix, this would hang as the stdin
1710+
# pipe to the child had not been closed.
1711+
try:
1712+
stdout, stderr = proc.communicate(timeout=15)
1713+
except subprocess.TimeoutExpired as exc:
1714+
self.fail("communicate() hung waiting on child process that should have seen its stdin pipe close and exit")
1715+
self.assertEqual(
1716+
proc.returncode, 0,
1717+
msg=f"STDERR:\n{stderr}\nSTDOUT:\n{stdout}")
1718+
self.assertTrue(stdout.startswith("spam"), msg=stdout)
1719+
self.assertIn("beans", stdout)
1720+
16871721

16881722
class RunFuncTestCase(BaseTestCase):
16891723
def run_python(self, code, **kwargs):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
When :meth:`subprocess.Popen.communicate` was called with *input* and a
2+
*timeout* and is called for a second time after a
3+
:exc:`~subprocess.TimeoutExpired` exception before the process has died, it
4+
should no longer hang.

0 commit comments

Comments
 (0)