Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Doc/library/subprocess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 34 additions & 0 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.assertTrue(stdout.startswith("spam"), msg=stdout)
self.assertIn("beans", stdout)


class RunFuncTestCase(BaseTestCase):
def run_python(self, code, **kwargs):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Loading