From 4bdc075a9bcadd46b607ee51ea2a090b002958a1 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 9 Aug 2026 09:47:39 +0300 Subject: [PATCH] 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. --- Doc/library/audit_events.rst | 5 ++ Lib/subprocess.py | 72 ++++++++++++++++--- Lib/test/test_subprocess.py | 70 ++++++++++++++++++ ...8-09-02-30-00.gh-issue-87512.sptimeout.rst | 5 ++ Modules/_winapi.c | 60 ++++++++++++++++ Modules/clinic/_winapi.c.h | 67 ++++++++++++++++- 6 files changed, 270 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-08-09-02-30-00.gh-issue-87512.sptimeout.rst diff --git a/Doc/library/audit_events.rst b/Doc/library/audit_events.rst index 73a580920246315..a5d7c1ccc7f98e2 100644 --- a/Doc/library/audit_events.rst +++ b/Doc/library/audit_events.rst @@ -41,6 +41,8 @@ public API of CPython: +----------------------------+-------------------------------------------+ | _winapi.OpenProcess | ``process_id``, ``desired_access`` | +----------------------------+-------------------------------------------+ +| _winapi.OpenThread | ``thread_id``, ``desired_access`` | ++----------------------------+-------------------------------------------+ | _winapi.TerminateProcess | ``handle``, ``exit_code`` | +----------------------------+-------------------------------------------+ | _posixsubprocess.fork_exec | ``exec_list``, ``args``, ``env`` | @@ -50,3 +52,6 @@ public API of CPython: .. versionadded:: 3.14 The ``_posixsubprocess.fork_exec`` internal audit event. + +.. versionadded:: next + The ``_winapi.OpenThread`` internal audit event. diff --git a/Lib/subprocess.py b/Lib/subprocess.py index a14fede00c391c9..4e5a5b5d64f590a 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -672,11 +672,13 @@ def run(*popenargs, except TimeoutExpired as exc: process.kill() if _mswindows: - # Windows accumulates the output in a single blocking - # read() call run on child threads, with the timeout - # being done in a join() on those threads. communicate() - # _after_ kill() is required to collect that and add it - # to the exception. + # Windows accumulates the output on child threads, with the + # timeout being done in a join() on those threads. + # communicate() _after_ kill() is required to collect that + # and add it to the exception. The pipes can be inherited + # by other processes, so cancel pending reads to not wait + # for them (gh-87512). + process._cancel_io() exc.stdout, exc.stderr = process.communicate() else: # POSIX _communicate already populated the output so @@ -1277,6 +1279,8 @@ def __enter__(self): return self def __exit__(self, exc_type, value, traceback): + if _mswindows: + self._cancel_io() if self.stdout: self.stdout.close() if self.stderr: @@ -1776,8 +1780,46 @@ def _wait(self, timeout): return self.returncode + def _cancel_io(self): + # The communication threads can be blocked in a synchronous read + # or write on a pipe which is not closed by the child process. + # Closing such pipe blocks too, so cancel the I/O first. + for name in ('stdout_thread', 'stderr_thread', '_stdin_thread'): + thread = getattr(self, name, None) + if thread is None or not thread.is_alive(): + continue + try: + handle = _winapi.OpenThread(_winapi.THREAD_TERMINATE, + False, thread.ident) + except OSError: + continue + try: + _winapi.CancelSynchronousIo(handle) + except OSError: + # ERROR_NOT_FOUND if there is no pending I/O. + pass + finally: + _winapi.CloseHandle(handle) + + def _readerthread(self, fh, buffer): - buffer.append(fh.read()) + # Read what is available, or block for a single byte, so that + # the read can be canceled and does not wait for the pipe to be + # closed by all processes which inherited it (gh-87512). + handle = msvcrt.get_osfhandle(fh.fileno()) + while True: + try: + size = _winapi.PeekNamedPipe(handle)[0] or 1 + data = _winapi.ReadFile(handle, size)[0] + except BrokenPipeError: + break + except OSError as exc: + if exc.winerror == _winapi.ERROR_OPERATION_ABORTED: + break + raise + if not data: + break + buffer.append(data) fh.close() @@ -1845,8 +1887,22 @@ def _communicate(self, input, endtime, orig_timeout): self.stderr.close() # All data exchanged. Translate lists into strings. - stdout = stdout[0] if stdout else None - stderr = stderr[0] if stderr else None + if stdout is not None: + stdout = b''.join(stdout) + if stderr is not None: + stderr = b''.join(stderr) + + # Translate newlines, if requested. + # This also turns bytes into strings. + if self.text_mode: + if stdout is not None: + stdout = self._translate_newlines(stdout, + self.stdout.encoding, + self.stdout.errors) + if stderr is not None: + stderr = self._translate_newlines(stderr, + self.stderr.encoding, + self.stderr.errors) return (stdout, stderr) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index d1840e97d0f2f7c..4529300ec30465a 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3982,6 +3982,76 @@ def test_kill_dead(self): def test_terminate_dead(self): self._kill_dead_process('terminate') + # gh-87512: the pipes can be inherited by a process which outlives the + # child process, e.g. with shell=True. Reading them should not wait for + # that process, neither in communicate() nor when closing them. + + def _inherited_pipe_cmd(self, sentinel): + # Run the code in a grandchild process which lives while the + # sentinel file exists, but not longer than SHORT_TIMEOUT. + code = ("import os, sys, time; " + "print('spam'); sys.stdout.flush(); " + "print('eggs', file=sys.stderr); sys.stderr.flush(); " + "[time.sleep(0.05) for _ in range(%s) " + "if os.path.exists(%a)]" + % (int(support.SHORT_TIMEOUT / 0.05), sentinel)) + cmd = '"%s" -c "%s"' % (sys.executable, code) + return cmd + + def test_run_timeout_inherited_pipe(self): + with os_helper.temp_dir() as dirname: + sentinel = os.path.join(dirname, 'sentinel') + open(sentinel, 'wb').close() + cmd = self._inherited_pipe_cmd(sentinel) + try: + start = time.monotonic() + with self.assertRaises(subprocess.TimeoutExpired) as cm: + subprocess.run(cmd, shell=True, capture_output=True, + timeout=0.5) + self.assertLess(time.monotonic() - start, support.LOOPBACK_TIMEOUT) + # The output written before the timeout is preserved. + self.assertEqual(cm.exception.stdout, b'spam\r\n') + self.assertEqual(cm.exception.stderr, b'eggs\r\n') + finally: + os.unlink(sentinel) + + def test_run_timeout_inherited_pipe_text(self): + with os_helper.temp_dir() as dirname: + sentinel = os.path.join(dirname, 'sentinel') + open(sentinel, 'wb').close() + cmd = self._inherited_pipe_cmd(sentinel) + try: + start = time.monotonic() + with self.assertRaises(subprocess.TimeoutExpired) as cm: + subprocess.run(cmd, shell=True, capture_output=True, + text=True, timeout=0.5) + self.assertLess(time.monotonic() - start, + support.LOOPBACK_TIMEOUT) + self.assertEqual(cm.exception.stdout, 'spam\n') + self.assertEqual(cm.exception.stderr, 'eggs\n') + finally: + os.unlink(sentinel) + + def test_exit_inherited_pipe(self): + # Closing the pipes should not block on a pending read. + with os_helper.temp_dir() as dirname: + sentinel = os.path.join(dirname, 'sentinel') + open(sentinel, 'wb').close() + cmd = self._inherited_pipe_cmd(sentinel) + try: + proc = subprocess.Popen(cmd, shell=True, + stdout=subprocess.PIPE) + with self.assertRaises(subprocess.TimeoutExpired): + proc.communicate(timeout=0.5) + proc.kill() # kills the shell, not the grandchild + start = time.monotonic() + with proc: + pass + self.assertLess(time.monotonic() - start, support.LOOPBACK_TIMEOUT) + finally: + os.unlink(sentinel) + + class MiscTests(unittest.TestCase): class RecordingPopen(subprocess.Popen): diff --git a/Misc/NEWS.d/next/Library/2026-08-09-02-30-00.gh-issue-87512.sptimeout.rst b/Misc/NEWS.d/next/Library/2026-08-09-02-30-00.gh-issue-87512.sptimeout.rst new file mode 100644 index 000000000000000..5ab97f9cb06c586 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-08-09-02-30-00.gh-issue-87512.sptimeout.rst @@ -0,0 +1,5 @@ +Fix :func:`subprocess.run` on Windows: the *timeout* was ignored if the pipes +were inherited by a process which outlives the child process (for example +with ``shell=True``). The output read before the timeout is now set on the +:exc:`~subprocess.TimeoutExpired` exception, as on other platforms. Closing +:class:`subprocess.Popen` no longer blocks in such case either. diff --git a/Modules/_winapi.c b/Modules/_winapi.c index a649d84a7925a04..9595c3145df41df 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1933,6 +1933,63 @@ _winapi_OpenProcess_impl(PyObject *module, DWORD desired_access, return handle; } +/*[clinic input] +_winapi.OpenThread -> HANDLE + + desired_access: DWORD + inherit_handle: BOOL + thread_id: DWORD + / +[clinic start generated code]*/ + +static HANDLE +_winapi_OpenThread_impl(PyObject *module, DWORD desired_access, + BOOL inherit_handle, DWORD thread_id) +/*[clinic end generated code: output=4eac45e975925ec0 input=ee54a437060f9b59]*/ +{ + HANDLE handle; + + if (PySys_Audit("_winapi.OpenThread", "kk", + thread_id, desired_access) < 0) { + return INVALID_HANDLE_VALUE; + } + + Py_BEGIN_ALLOW_THREADS + handle = OpenThread(desired_access, inherit_handle, thread_id); + Py_END_ALLOW_THREADS + if (handle == NULL) { + PyErr_SetFromWindowsErr(0); + handle = INVALID_HANDLE_VALUE; + } + + return handle; +} + +/*[clinic input] +_winapi.CancelSynchronousIo + + thread: HANDLE + / + +Cancel pending synchronous I/O issued by the specified thread. +[clinic start generated code]*/ + +static PyObject * +_winapi_CancelSynchronousIo_impl(PyObject *module, HANDLE thread) +/*[clinic end generated code: output=a15680598ffc526b input=40cdf5a637ed95da]*/ +{ + BOOL result; + + Py_BEGIN_ALLOW_THREADS + result = CancelSynchronousIo(thread); + Py_END_ALLOW_THREADS + + if (!result) { + return PyErr_SetFromWindowsErr(0); + } + Py_RETURN_NONE; +} + /*[clinic input] _winapi.PeekNamedPipe @@ -3212,6 +3269,8 @@ static PyMethodDef winapi_functions[] = { _WINAPI_RESETEVENT_METHODDEF _WINAPI_SETEVENT_METHODDEF _WINAPI_SETNAMEDPIPEHANDLESTATE_METHODDEF + _WINAPI_OPENTHREAD_METHODDEF + _WINAPI_CANCELSYNCHRONOUSIO_METHODDEF _WINAPI_TERMINATEPROCESS_METHODDEF _WINAPI_UNMAPVIEWOFFILE_METHODDEF _WINAPI_VIRTUALQUERYSIZE_METHODDEF @@ -3323,6 +3382,7 @@ static int winapi_exec(PyObject *m) WINAPI_CONSTANT(F_DWORD, PIPE_WAIT); WINAPI_CONSTANT(F_DWORD, PROCESS_ALL_ACCESS); WINAPI_CONSTANT(F_DWORD, SYNCHRONIZE); + WINAPI_CONSTANT(F_DWORD, THREAD_TERMINATE); WINAPI_CONSTANT(F_DWORD, PROCESS_DUP_HANDLE); WINAPI_CONSTANT(F_DWORD, PROCESS_QUERY_LIMITED_INFORMATION); WINAPI_CONSTANT(F_DWORD, SEC_COMMIT); diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index 031a0783aef60bb..a0512986048e7d4 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -1282,6 +1282,71 @@ _winapi_OpenProcess(PyObject *module, PyObject *const *args, Py_ssize_t nargs) return return_value; } +PyDoc_STRVAR(_winapi_OpenThread__doc__, +"OpenThread($module, desired_access, inherit_handle, thread_id, /)\n" +"--\n" +"\n"); + +#define _WINAPI_OPENTHREAD_METHODDEF \ + {"OpenThread", _PyCFunction_CAST(_winapi_OpenThread), METH_FASTCALL, _winapi_OpenThread__doc__}, + +static HANDLE +_winapi_OpenThread_impl(PyObject *module, DWORD desired_access, + BOOL inherit_handle, DWORD thread_id); + +static PyObject * +_winapi_OpenThread(PyObject *module, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + DWORD desired_access; + BOOL inherit_handle; + DWORD thread_id; + HANDLE _return_value; + + if (!_PyArg_ParseStack(args, nargs, "kik:OpenThread", + &desired_access, &inherit_handle, &thread_id)) { + goto exit; + } + _return_value = _winapi_OpenThread_impl(module, desired_access, inherit_handle, thread_id); + if ((_return_value == INVALID_HANDLE_VALUE) && PyErr_Occurred()) { + goto exit; + } + if (_return_value == NULL) { + Py_RETURN_NONE; + } + return_value = HANDLE_TO_PYNUM(_return_value); + +exit: + return return_value; +} + +PyDoc_STRVAR(_winapi_CancelSynchronousIo__doc__, +"CancelSynchronousIo($module, thread, /)\n" +"--\n" +"\n" +"Cancel pending synchronous I/O issued by the specified thread."); + +#define _WINAPI_CANCELSYNCHRONOUSIO_METHODDEF \ + {"CancelSynchronousIo", (PyCFunction)_winapi_CancelSynchronousIo, METH_O, _winapi_CancelSynchronousIo__doc__}, + +static PyObject * +_winapi_CancelSynchronousIo_impl(PyObject *module, HANDLE thread); + +static PyObject * +_winapi_CancelSynchronousIo(PyObject *module, PyObject *arg) +{ + PyObject *return_value = NULL; + HANDLE thread; + + if (!PyArg_Parse(arg, "" F_HANDLE ":CancelSynchronousIo", &thread)) { + goto exit; + } + return_value = _winapi_CancelSynchronousIo_impl(module, thread); + +exit: + return return_value; +} + PyDoc_STRVAR(_winapi_PeekNamedPipe__doc__, "PeekNamedPipe($module, handle, size=0, /)\n" "--\n" @@ -2379,4 +2444,4 @@ _winapi_GetTickCount64(PyObject *module, PyObject *Py_UNUSED(ignored)) #ifndef _WINAPI_GETSHORTPATHNAME_METHODDEF #define _WINAPI_GETSHORTPATHNAME_METHODDEF #endif /* !defined(_WINAPI_GETSHORTPATHNAME_METHODDEF) */ -/*[clinic end generated code: output=713a8ce97185b017 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=6fcc80edf80151b9 input=a9049054013a1b77]*/