From 08ab03508ae6333ab8a15d564a6bd90906b2ef87 Mon Sep 17 00:00:00 2001 From: zfz2005 <187017388+zfz2005@users.noreply.github.com> Date: Sun, 25 Oct 2026 18:07:02 +0800 Subject: [PATCH 1/2] Clean up backend process on frontend interruption --- .../+interrupted-backend.bugfix.rst | 1 + src/pyproject_api/_via_fresh_subprocess.py | 8 ++++- tests/test_frontend.py | 35 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 docs/changelog.d/+interrupted-backend.bugfix.rst diff --git a/docs/changelog.d/+interrupted-backend.bugfix.rst b/docs/changelog.d/+interrupted-backend.bugfix.rst new file mode 100644 index 0000000..e3b668e --- /dev/null +++ b/docs/changelog.d/+interrupted-backend.bugfix.rst @@ -0,0 +1 @@ +Stop the backend process and join its output collection thread when a frontend call is interrupted. diff --git a/src/pyproject_api/_via_fresh_subprocess.py b/src/pyproject_api/_via_fresh_subprocess.py index e9a2c29..8d3c891 100644 --- a/src/pyproject_api/_via_fresh_subprocess.py +++ b/src/pyproject_api/_via_fresh_subprocess.py @@ -77,7 +77,13 @@ def _send_msg(self, cmd: str, result_file: Path, msg: str) -> Generator[Subproce cwd=self._root, env=env, ) - yield SubprocessCmdStatus(process, f"{os.linesep}{msg}{os.linesep}") + status = SubprocessCmdStatus(process, f"{os.linesep}{msg}{os.linesep}") + try: + yield status + finally: + if process.poll() is None: + process.kill() + status.join() def send_cmd(self, cmd: str, **kwargs: Any) -> tuple[Any, str, str]: """ diff --git a/tests/test_frontend.py b/tests/test_frontend.py index 8410d90..e0e9817 100644 --- a/tests/test_frontend.py +++ b/tests/test_frontend.py @@ -10,6 +10,7 @@ import pytest from packaging.requirements import Requirement +from pyproject_api import _via_fresh_subprocess from pyproject_api._frontend import BackendFailed from pyproject_api._via_fresh_subprocess import SubprocessFrontend @@ -161,6 +162,40 @@ def test_large_command_with_failed_backend(local_builder: Callable[[str], Path]) assert "RuntimeError: backend import failed" in context.value.err +@pytest.mark.parametrize("error_type", [KeyboardInterrupt, SystemExit, RuntimeError]) +def test_interrupted_frontend_cleans_up_backend( + error_type: type[BaseException], local_builder: Callable[[str], Path], mocker: pytest_mock.MockerFixture +) -> None: + root = local_builder(""" + import time + + def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): + time.sleep(60) + return "demo-1.0-py3-none-any.whl" + """) + frontend = SubprocessFrontend(*SubprocessFrontend.create_args_from_folder(root)[:-1]) + status_factory = mocker.spy(_via_fresh_subprocess, "SubprocessCmdStatus") + error = error_type("build interrupted") + mocker.patch("pyproject_api._frontend.sleep", side_effect=error) + + try: + with pytest.raises(error_type) as context: + frontend.build_wheel(root / "dist") + + assert context.value is error + status = status_factory.spy_return + assert status.process.poll() is not None + assert not status.is_alive() + assert status.process.stdin.closed + assert status.process.stdout.closed + assert status.process.stderr.closed + finally: + # Keep a regression from leaving the backend and its non-daemon thread running. + status = status_factory.spy_return + status.process.kill() + status.join(timeout=10) + + @pytest.mark.parametrize("cmd", ["build_wheel", "build_sdist"]) def test_missing_required_cmd(cmd: str, local_builder: Callable[[str], Path]) -> None: tmp_path = local_builder("") From d7e8bd5e228d677723e2d123fb43a89db85e1955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Tue, 27 Oct 2026 12:34:51 -0700 Subject: [PATCH 2/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(subprocess):?= =?UTF-8?q?=20kill=20the=20backend=20without=20polling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Popen.kill() already returns without signalling once the process has exited, so the poll() guard repeated it. The test now asserts the one observable effect, the output thread ending; closed pipes follow from communicate() returning. Name the parametrized cases and mark the new locals Final. --- src/pyproject_api/_via_fresh_subprocess.py | 7 ++--- tests/test_frontend.py | 33 +++++++++++----------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/pyproject_api/_via_fresh_subprocess.py b/src/pyproject_api/_via_fresh_subprocess.py index 8d3c891..49236d8 100644 --- a/src/pyproject_api/_via_fresh_subprocess.py +++ b/src/pyproject_api/_via_fresh_subprocess.py @@ -5,7 +5,7 @@ from contextlib import contextmanager from subprocess import PIPE, Popen # ruff:ignore[suspicious-subprocess-import] from threading import Thread -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any, Final, cast from ._frontend import CmdStatus, Frontend @@ -77,12 +77,11 @@ def _send_msg(self, cmd: str, result_file: Path, msg: str) -> Generator[Subproce cwd=self._root, env=env, ) - status = SubprocessCmdStatus(process, f"{os.linesep}{msg}{os.linesep}") + status: Final = SubprocessCmdStatus(process, f"{os.linesep}{msg}{os.linesep}") try: yield status finally: - if process.poll() is None: - process.kill() + process.kill() status.join() def send_cmd(self, cmd: str, **kwargs: Any) -> tuple[Any, str, str]: diff --git a/tests/test_frontend.py b/tests/test_frontend.py index e0e9817..5ad3475 100644 --- a/tests/test_frontend.py +++ b/tests/test_frontend.py @@ -162,36 +162,35 @@ def test_large_command_with_failed_backend(local_builder: Callable[[str], Path]) assert "RuntimeError: backend import failed" in context.value.err -@pytest.mark.parametrize("error_type", [KeyboardInterrupt, SystemExit, RuntimeError]) +@pytest.mark.parametrize( + "error_type", + [ + pytest.param(KeyboardInterrupt, id="keyboard-interrupt"), + pytest.param(SystemExit, id="system-exit"), + pytest.param(RuntimeError, id="runtime-error"), + ], +) def test_interrupted_frontend_cleans_up_backend( error_type: type[BaseException], local_builder: Callable[[str], Path], mocker: pytest_mock.MockerFixture ) -> None: - root = local_builder(""" + root: Final = local_builder(""" import time def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): time.sleep(60) return "demo-1.0-py3-none-any.whl" """) - frontend = SubprocessFrontend(*SubprocessFrontend.create_args_from_folder(root)[:-1]) - status_factory = mocker.spy(_via_fresh_subprocess, "SubprocessCmdStatus") - error = error_type("build interrupted") - mocker.patch("pyproject_api._frontend.sleep", side_effect=error) + status_factory: Final = mocker.spy(_via_fresh_subprocess, "SubprocessCmdStatus") + mocker.patch("pyproject_api._frontend.sleep", side_effect=error_type("build interrupted")) try: - with pytest.raises(error_type) as context: - frontend.build_wheel(root / "dist") - - assert context.value is error - status = status_factory.spy_return - assert status.process.poll() is not None - assert not status.is_alive() - assert status.process.stdin.closed - assert status.process.stdout.closed - assert status.process.stderr.closed + with pytest.raises(error_type): + SubprocessFrontend(*SubprocessFrontend.create_args_from_folder(root)[:-1]).build_wheel(root / "dist") + + assert not status_factory.spy_return.is_alive() finally: # Keep a regression from leaving the backend and its non-daemon thread running. - status = status_factory.spy_return + status: Final = status_factory.spy_return status.process.kill() status.join(timeout=10)