From 9ec20a180c2f5c3a8847b7433fd3e56de0f31252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Mon, 9 Mar 2026 11:14:47 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix(subprocess):=20drain=20pipes?= =?UTF-8?q?=20after=20killing=20timed-out=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, process.kill() terminates the process but doesn't close inherited pipe handles. The reader threads spawned by communicate() stay blocked on fh.read() indefinitely, preventing the caller from ever returning. This was observed in tox CI where the 5s timeout fired and killed the process, yet the test still hung. Calling communicate() after kill() joins the reader threads and closes the pipes, as required by the Python subprocess docs. --- src/python_discovery/_cached_py_info.py | 1 + tests/test_cached_py_info.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/python_discovery/_cached_py_info.py b/src/python_discovery/_cached_py_info.py index 29f9462..5a7d35d 100644 --- a/src/python_discovery/_cached_py_info.py +++ b/src/python_discovery/_cached_py_info.py @@ -210,6 +210,7 @@ def _run_subprocess( code = process.returncode except TimeoutExpired: process.kill() + process.communicate() out, err, code = "", "timed out", -1 except OSError as os_error: out, err, code = "", os_error.strerror, os_error.errno diff --git a/tests/test_cached_py_info.py b/tests/test_cached_py_info.py index 06bbc83..55d65b8 100644 --- a/tests/test_cached_py_info.py +++ b/tests/test_cached_py_info.py @@ -113,13 +113,14 @@ def test_run_subprocess_with_cookies(mocker: MockerFixture) -> None: def test_run_subprocess_timeout(mocker: MockerFixture) -> None: mock_process = MagicMock() - mock_process.communicate.side_effect = TimeoutExpired(cmd="python", timeout=30) + mock_process.communicate.side_effect = [TimeoutExpired(cmd="python", timeout=30), ("", "")] mocker.patch("python_discovery._cached_py_info.Popen", return_value=mock_process) failure, result = _run_subprocess(PythonInfo, sys.executable, dict(os.environ)) assert failure is not None assert "timed out" in str(failure) assert result is None mock_process.kill.assert_called_once() + assert mock_process.communicate.call_count == 2 def test_run_subprocess_nonzero_exit(mocker: MockerFixture) -> None: