Skip to content

Commit 526d7a8

Browse files
mrowqagpshead
andauthored
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. --------- Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent d2d2e92 commit 526d7a8

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
@@ -2105,7 +2105,7 @@ def _communicate(self, input, endtime, orig_timeout):
21052105
input_view = memoryview(self._input)
21062106

21072107
with _PopenSelector() as selector:
2108-
if self.stdin and input:
2108+
if self.stdin and not self.stdin.closed and self._input:
21092109
selector.register(self.stdin, selectors.EVENT_WRITE)
21102110
if self.stdout and not self.stdout.closed:
21112111
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
@@ -1643,6 +1643,40 @@ def test_wait_negative_timeout(self):
16431643

16441644
self.assertEqual(proc.wait(), 0)
16451645

1646+
def test_post_timeout_communicate_sends_input(self):
1647+
"""GH-141473 regression test; the stdin pipe must close"""
1648+
with subprocess.Popen(
1649+
[sys.executable, "-uc", """\
1650+
import sys
1651+
while c := sys.stdin.read(512):
1652+
sys.stdout.write(c)
1653+
print()
1654+
"""],
1655+
stdin=subprocess.PIPE,
1656+
stdout=subprocess.PIPE,
1657+
stderr=subprocess.PIPE,
1658+
text=True,
1659+
) as proc:
1660+
try:
1661+
data = f"spam{'#'*4096}beans"
1662+
proc.communicate(
1663+
input=data,
1664+
timeout=0,
1665+
)
1666+
except subprocess.TimeoutExpired as exc:
1667+
pass
1668+
# Prior to the bugfix, this would hang as the stdin
1669+
# pipe to the child had not been closed.
1670+
try:
1671+
stdout, stderr = proc.communicate(timeout=15)
1672+
except subprocess.TimeoutExpired as exc:
1673+
self.fail("communicate() hung waiting on child process that should have seen its stdin pipe close and exit")
1674+
self.assertEqual(
1675+
proc.returncode, 0,
1676+
msg=f"STDERR:\n{stderr}\nSTDOUT:\n{stdout}")
1677+
self.assertStartsWith(stdout, "spam")
1678+
self.assertIn("beans", stdout)
1679+
16461680

16471681
class RunFuncTestCase(BaseTestCase):
16481682
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)