Skip to content

Commit 9ec20a1

Browse files
committed
πŸ› fix(subprocess): drain pipes after killing timed-out process
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.
1 parent 427f817 commit 9ec20a1

2 files changed

Lines changed: 3 additions & 1 deletion

File tree

β€Žsrc/python_discovery/_cached_py_info.pyβ€Ž

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ def _run_subprocess(
210210
code = process.returncode
211211
except TimeoutExpired:
212212
process.kill()
213+
process.communicate()
213214
out, err, code = "", "timed out", -1
214215
except OSError as os_error:
215216
out, err, code = "", os_error.strerror, os_error.errno

β€Žtests/test_cached_py_info.pyβ€Ž

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,14 @@ def test_run_subprocess_with_cookies(mocker: MockerFixture) -> None:
113113

114114
def test_run_subprocess_timeout(mocker: MockerFixture) -> None:
115115
mock_process = MagicMock()
116-
mock_process.communicate.side_effect = TimeoutExpired(cmd="python", timeout=30)
116+
mock_process.communicate.side_effect = [TimeoutExpired(cmd="python", timeout=30), ("", "")]
117117
mocker.patch("python_discovery._cached_py_info.Popen", return_value=mock_process)
118118
failure, result = _run_subprocess(PythonInfo, sys.executable, dict(os.environ))
119119
assert failure is not None
120120
assert "timed out" in str(failure)
121121
assert result is None
122122
mock_process.kill.assert_called_once()
123+
assert mock_process.communicate.call_count == 2
123124

124125

125126
def test_run_subprocess_nonzero_exit(mocker: MockerFixture) -> None:

0 commit comments

Comments
Β (0)