Skip to content

Commit 4760894

Browse files
miss-islingtonmrowqagpshead
authored
[3.14] gh-141473: Fix subprocess.Popen.communicate to send input to stdin upon a subsequent post-timeout call (GH-141477) (#142059)
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>
1 parent c1d3e25 commit 4760894

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
@@ -2109,7 +2109,7 @@ def _communicate(self, input, endtime, orig_timeout):
21092109
input_view = self._input.cast("b") # byte input required
21102110

21112111
with _PopenSelector() as selector:
2112-
if self.stdin and input:
2112+
if self.stdin and not self.stdin.closed and self._input:
21132113
selector.register(self.stdin, selectors.EVENT_WRITE)
21142114
if self.stdout and not self.stdout.closed:
21152115
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
@@ -1685,6 +1685,40 @@ def test_wait_negative_timeout(self):
16851685

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

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

16891723
class RunFuncTestCase(BaseTestCase):
16901724
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)