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]*/