Skip to content

Commit 4bdc075

Browse files
serhiy-storchakaserhiy-storchaka
committed
gh-87512: Fix ignored timeout in subprocess.run() on Windows
The pipes can be inherited by processes which outlive the child process, so reading them blocked until all of them exited, ignoring the timeout. The output is now read in chunks and the pending I/O is canceled, so the output read before the timeout is set on the TimeoutExpired exception, as on other platforms. Add _winapi.OpenThread(), _winapi.CancelSynchronousIo() and _winapi.THREAD_TERMINATE.
1 parent 998b890 commit 4bdc075

6 files changed

Lines changed: 270 additions & 9 deletions

File tree

‎Doc/library/audit_events.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ public API of CPython:
4141
+----------------------------+-------------------------------------------+
4242
| _winapi.OpenProcess | ``process_id``, ``desired_access`` |
4343
+----------------------------+-------------------------------------------+
44+
| _winapi.OpenThread | ``thread_id``, ``desired_access`` |
45+
+----------------------------+-------------------------------------------+
4446
| _winapi.TerminateProcess | ``handle``, ``exit_code`` |
4547
+----------------------------+-------------------------------------------+
4648
| _posixsubprocess.fork_exec | ``exec_list``, ``args``, ``env`` |
@@ -50,3 +52,6 @@ public API of CPython:
5052

5153
.. versionadded:: 3.14
5254
The ``_posixsubprocess.fork_exec`` internal audit event.
55+
56+
.. versionadded:: next
57+
The ``_winapi.OpenThread`` internal audit event.

‎Lib/subprocess.py‎

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -672,11 +672,13 @@ def run(*popenargs,
672672
except TimeoutExpired as exc:
673673
process.kill()
674674
if _mswindows:
675-
# Windows accumulates the output in a single blocking
676-
# read() call run on child threads, with the timeout
677-
# being done in a join() on those threads. communicate()
678-
# _after_ kill() is required to collect that and add it
679-
# to the exception.
675+
# Windows accumulates the output on child threads, with the
676+
# timeout being done in a join() on those threads.
677+
# communicate() _after_ kill() is required to collect that
678+
# and add it to the exception. The pipes can be inherited
679+
# by other processes, so cancel pending reads to not wait
680+
# for them (gh-87512).
681+
process._cancel_io()
680682
exc.stdout, exc.stderr = process.communicate()
681683
else:
682684
# POSIX _communicate already populated the output so
@@ -1277,6 +1279,8 @@ def __enter__(self):
12771279
return self
12781280

12791281
def __exit__(self, exc_type, value, traceback):
1282+
if _mswindows:
1283+
self._cancel_io()
12801284
if self.stdout:
12811285
self.stdout.close()
12821286
if self.stderr:
@@ -1776,8 +1780,46 @@ def _wait(self, timeout):
17761780
return self.returncode
17771781

17781782

1783+
def _cancel_io(self):
1784+
# The communication threads can be blocked in a synchronous read
1785+
# or write on a pipe which is not closed by the child process.
1786+
# Closing such pipe blocks too, so cancel the I/O first.
1787+
for name in ('stdout_thread', 'stderr_thread', '_stdin_thread'):
1788+
thread = getattr(self, name, None)
1789+
if thread is None or not thread.is_alive():
1790+
continue
1791+
try:
1792+
handle = _winapi.OpenThread(_winapi.THREAD_TERMINATE,
1793+
False, thread.ident)
1794+
except OSError:
1795+
continue
1796+
try:
1797+
_winapi.CancelSynchronousIo(handle)
1798+
except OSError:
1799+
# ERROR_NOT_FOUND if there is no pending I/O.
1800+
pass
1801+
finally:
1802+
_winapi.CloseHandle(handle)
1803+
1804+
17791805
def _readerthread(self, fh, buffer):
1780-
buffer.append(fh.read())
1806+
# Read what is available, or block for a single byte, so that
1807+
# the read can be canceled and does not wait for the pipe to be
1808+
# closed by all processes which inherited it (gh-87512).
1809+
handle = msvcrt.get_osfhandle(fh.fileno())
1810+
while True:
1811+
try:
1812+
size = _winapi.PeekNamedPipe(handle)[0] or 1
1813+
data = _winapi.ReadFile(handle, size)[0]
1814+
except BrokenPipeError:
1815+
break
1816+
except OSError as exc:
1817+
if exc.winerror == _winapi.ERROR_OPERATION_ABORTED:
1818+
break
1819+
raise
1820+
if not data:
1821+
break
1822+
buffer.append(data)
17811823
fh.close()
17821824

17831825

@@ -1845,8 +1887,22 @@ def _communicate(self, input, endtime, orig_timeout):
18451887
self.stderr.close()
18461888

18471889
# All data exchanged. Translate lists into strings.
1848-
stdout = stdout[0] if stdout else None
1849-
stderr = stderr[0] if stderr else None
1890+
if stdout is not None:
1891+
stdout = b''.join(stdout)
1892+
if stderr is not None:
1893+
stderr = b''.join(stderr)
1894+
1895+
# Translate newlines, if requested.
1896+
# This also turns bytes into strings.
1897+
if self.text_mode:
1898+
if stdout is not None:
1899+
stdout = self._translate_newlines(stdout,
1900+
self.stdout.encoding,
1901+
self.stdout.errors)
1902+
if stderr is not None:
1903+
stderr = self._translate_newlines(stderr,
1904+
self.stderr.encoding,
1905+
self.stderr.errors)
18501906

18511907
return (stdout, stderr)
18521908

‎Lib/test/test_subprocess.py‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3982,6 +3982,76 @@ def test_kill_dead(self):
39823982
def test_terminate_dead(self):
39833983
self._kill_dead_process('terminate')
39843984

3985+
# gh-87512: the pipes can be inherited by a process which outlives the
3986+
# child process, e.g. with shell=True. Reading them should not wait for
3987+
# that process, neither in communicate() nor when closing them.
3988+
3989+
def _inherited_pipe_cmd(self, sentinel):
3990+
# Run the code in a grandchild process which lives while the
3991+
# sentinel file exists, but not longer than SHORT_TIMEOUT.
3992+
code = ("import os, sys, time; "
3993+
"print('spam'); sys.stdout.flush(); "
3994+
"print('eggs', file=sys.stderr); sys.stderr.flush(); "
3995+
"[time.sleep(0.05) for _ in range(%s) "
3996+
"if os.path.exists(%a)]"
3997+
% (int(support.SHORT_TIMEOUT / 0.05), sentinel))
3998+
cmd = '"%s" -c "%s"' % (sys.executable, code)
3999+
return cmd
4000+
4001+
def test_run_timeout_inherited_pipe(self):
4002+
with os_helper.temp_dir() as dirname:
4003+
sentinel = os.path.join(dirname, 'sentinel')
4004+
open(sentinel, 'wb').close()
4005+
cmd = self._inherited_pipe_cmd(sentinel)
4006+
try:
4007+
start = time.monotonic()
4008+
with self.assertRaises(subprocess.TimeoutExpired) as cm:
4009+
subprocess.run(cmd, shell=True, capture_output=True,
4010+
timeout=0.5)
4011+
self.assertLess(time.monotonic() - start, support.LOOPBACK_TIMEOUT)
4012+
# The output written before the timeout is preserved.
4013+
self.assertEqual(cm.exception.stdout, b'spam\r\n')
4014+
self.assertEqual(cm.exception.stderr, b'eggs\r\n')
4015+
finally:
4016+
os.unlink(sentinel)
4017+
4018+
def test_run_timeout_inherited_pipe_text(self):
4019+
with os_helper.temp_dir() as dirname:
4020+
sentinel = os.path.join(dirname, 'sentinel')
4021+
open(sentinel, 'wb').close()
4022+
cmd = self._inherited_pipe_cmd(sentinel)
4023+
try:
4024+
start = time.monotonic()
4025+
with self.assertRaises(subprocess.TimeoutExpired) as cm:
4026+
subprocess.run(cmd, shell=True, capture_output=True,
4027+
text=True, timeout=0.5)
4028+
self.assertLess(time.monotonic() - start,
4029+
support.LOOPBACK_TIMEOUT)
4030+
self.assertEqual(cm.exception.stdout, 'spam\n')
4031+
self.assertEqual(cm.exception.stderr, 'eggs\n')
4032+
finally:
4033+
os.unlink(sentinel)
4034+
4035+
def test_exit_inherited_pipe(self):
4036+
# Closing the pipes should not block on a pending read.
4037+
with os_helper.temp_dir() as dirname:
4038+
sentinel = os.path.join(dirname, 'sentinel')
4039+
open(sentinel, 'wb').close()
4040+
cmd = self._inherited_pipe_cmd(sentinel)
4041+
try:
4042+
proc = subprocess.Popen(cmd, shell=True,
4043+
stdout=subprocess.PIPE)
4044+
with self.assertRaises(subprocess.TimeoutExpired):
4045+
proc.communicate(timeout=0.5)
4046+
proc.kill() # kills the shell, not the grandchild
4047+
start = time.monotonic()
4048+
with proc:
4049+
pass
4050+
self.assertLess(time.monotonic() - start, support.LOOPBACK_TIMEOUT)
4051+
finally:
4052+
os.unlink(sentinel)
4053+
4054+
39854055
class MiscTests(unittest.TestCase):
39864056

39874057
class RecordingPopen(subprocess.Popen):

‎Misc/NEWS.d/next/Library/2026-08-09-02-30-00.gh-issue-87512.sptimeout.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix :func:`subprocess.run` on Windows: the *timeout* was ignored if the pipes
2+
were inherited by a process which outlives the child process (for example
3+
with ``shell=True``). The output read before the timeout is now set on the
4+
:exc:`~subprocess.TimeoutExpired` exception, as on other platforms. Closing
5+
:class:`subprocess.Popen` no longer blocks in such case either.

‎Modules/_winapi.c‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1933,6 +1933,63 @@ _winapi_OpenProcess_impl(PyObject *module, DWORD desired_access,
19331933
return handle;
19341934
}
19351935

1936+
/*[clinic input]
1937+
_winapi.OpenThread -> HANDLE
1938+
1939+
desired_access: DWORD
1940+
inherit_handle: BOOL
1941+
thread_id: DWORD
1942+
/
1943+
[clinic start generated code]*/
1944+
1945+
static HANDLE
1946+
_winapi_OpenThread_impl(PyObject *module, DWORD desired_access,
1947+
BOOL inherit_handle, DWORD thread_id)
1948+
/*[clinic end generated code: output=4eac45e975925ec0 input=ee54a437060f9b59]*/
1949+
{
1950+
HANDLE handle;
1951+
1952+
if (PySys_Audit("_winapi.OpenThread", "kk",
1953+
thread_id, desired_access) < 0) {
1954+
return INVALID_HANDLE_VALUE;
1955+
}
1956+
1957+
Py_BEGIN_ALLOW_THREADS
1958+
handle = OpenThread(desired_access, inherit_handle, thread_id);
1959+
Py_END_ALLOW_THREADS
1960+
if (handle == NULL) {
1961+
PyErr_SetFromWindowsErr(0);
1962+
handle = INVALID_HANDLE_VALUE;
1963+
}
1964+
1965+
return handle;
1966+
}
1967+
1968+
/*[clinic input]
1969+
_winapi.CancelSynchronousIo
1970+
1971+
thread: HANDLE
1972+
/
1973+
1974+
Cancel pending synchronous I/O issued by the specified thread.
1975+
[clinic start generated code]*/
1976+
1977+
static PyObject *
1978+
_winapi_CancelSynchronousIo_impl(PyObject *module, HANDLE thread)
1979+
/*[clinic end generated code: output=a15680598ffc526b input=40cdf5a637ed95da]*/
1980+
{
1981+
BOOL result;
1982+
1983+
Py_BEGIN_ALLOW_THREADS
1984+
result = CancelSynchronousIo(thread);
1985+
Py_END_ALLOW_THREADS
1986+
1987+
if (!result) {
1988+
return PyErr_SetFromWindowsErr(0);
1989+
}
1990+
Py_RETURN_NONE;
1991+
}
1992+
19361993
/*[clinic input]
19371994
_winapi.PeekNamedPipe
19381995
@@ -3212,6 +3269,8 @@ static PyMethodDef winapi_functions[] = {
32123269
_WINAPI_RESETEVENT_METHODDEF
32133270
_WINAPI_SETEVENT_METHODDEF
32143271
_WINAPI_SETNAMEDPIPEHANDLESTATE_METHODDEF
3272+
_WINAPI_OPENTHREAD_METHODDEF
3273+
_WINAPI_CANCELSYNCHRONOUSIO_METHODDEF
32153274
_WINAPI_TERMINATEPROCESS_METHODDEF
32163275
_WINAPI_UNMAPVIEWOFFILE_METHODDEF
32173276
_WINAPI_VIRTUALQUERYSIZE_METHODDEF
@@ -3323,6 +3382,7 @@ static int winapi_exec(PyObject *m)
33233382
WINAPI_CONSTANT(F_DWORD, PIPE_WAIT);
33243383
WINAPI_CONSTANT(F_DWORD, PROCESS_ALL_ACCESS);
33253384
WINAPI_CONSTANT(F_DWORD, SYNCHRONIZE);
3385+
WINAPI_CONSTANT(F_DWORD, THREAD_TERMINATE);
33263386
WINAPI_CONSTANT(F_DWORD, PROCESS_DUP_HANDLE);
33273387
WINAPI_CONSTANT(F_DWORD, PROCESS_QUERY_LIMITED_INFORMATION);
33283388
WINAPI_CONSTANT(F_DWORD, SEC_COMMIT);

‎Modules/clinic/_winapi.c.h‎

Lines changed: 66 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)