已关闭
fix: 补充子进程超时、修复部署器 fd 泄漏与 CLI 资源清理 #555
zhangzj创建于 9月23日关闭于 29 天前
fix: 补充子进程超时、修复部署器 fd 泄漏与 CLI 资源清理 #555
已关闭
共 7 个文件变更+323-51
| @@ -7,10 +7,12 @@ | |||
| 7 | """ | 7 | """ |
| 8 | 8 | ||
| 9 | import asyncio | 9 | import asyncio |
| 10 | +import atexit | ||
| 10 | import json | 11 | import json |
| 11 | import os | 12 | import os |
| 12 | import re | 13 | import re |
| 13 | import subprocess | 14 | import subprocess |
| 15 | +import weakref | ||
| 14 | from pathlib import Path | 16 | from pathlib import Path |
| 15 | 17 | ||
| 16 | import click | 18 | import click |
| @@ -92,6 +94,26 @@ def _build_project(project_dir: Path) -> Path: | |||
| 92 | return whl_path | 94 | return whl_path |
| 93 | 95 | ||
| 94 | 96 | ||
| 97 | +_shutdown_managers: "weakref.WeakSet" = weakref.WeakSet() | ||
| 98 | + | ||
| 99 | + | ||
| 100 | +def _shutdown_manager(manager) -> None: | ||
| 101 | + """幂等地关闭 DeploymentManager。 | ||
| 102 | + | ||
| 103 | + ``result_callback`` 仅在命令成功时触发,命令抛异常时不会执行, | ||
| 104 | + 因此额外通过 ``atexit`` 兜底,保证数据库连接等资源始终被释放。 | ||
| 105 | + 去重以 manager 实例为粒度,避免同一进程内多次调用 CLI 时 | ||
| 106 | + 后续 manager 因模块级标记而被跳过。 | ||
| 107 | + """ | ||
| 108 | + if manager in _shutdown_managers: | ||
| 109 | + return | ||
| 110 | + _shutdown_managers.add(manager) | ||
| 111 | + try: | ||
| 112 | + asyncio.run(manager.shutdown()) | ||
| 113 | + except Exception as exc: | ||
| 114 | + click.echo(f"Failed to shutdown manager: {exc}", err=True) | ||
| 115 | + | ||
| 116 | + | ||
| 95 | @click.group() | 117 | @click.group() |
| 96 | @click.pass_context | 118 | @click.pass_context |
| 97 | def cli(ctx): | 119 | def cli(ctx): |
| @@ -106,6 +128,7 @@ def cli(ctx): | |||
| 106 | 128 | ||
| 107 | # 初始化 manager | 129 | # 初始化 manager |
| 108 | asyncio.run(manager.initialize()) | 130 | asyncio.run(manager.initialize()) |
| 131 | + atexit.register(_shutdown_manager, manager) | ||
| 109 | 132 | ||
| 110 | 133 | ||
| 111 | @cli.result_callback() | 134 | @cli.result_callback() |
| @@ -113,7 +136,7 @@ def cli(ctx): | |||
| 113 | def cleanup(ctx, result, **kwargs): | 136 | def cleanup(ctx, result, **kwargs): |
| 114 | """清理资源""" | 137 | """清理资源""" |
| 115 | if "manager" in ctx.obj: | 138 | if "manager" in ctx.obj: |
| 116 | - asyncio.run(ctx.obj["manager"].shutdown()) | 139 | + _shutdown_manager(ctx.obj["manager"]) |
| 117 | 140 | ||
| 118 | 141 | ||
| 119 | # ==================== Agent 命令 ==================== | 142 | # ==================== Agent 命令 ==================== |
| @@ -0,0 +1,29 @@ | |||
| 1 | +# coding: utf-8 | ||
| 2 | +# Copyright (c) Huawei Technologies Co., Ltd. 2026-2026. All rights reserved | ||
| 3 | + | ||
| 4 | +from unittest import TestCase | ||
| 5 | +from unittest import mock | ||
| 6 | + | ||
| 7 | +from openjiuwen_runtime.cli.main import _shutdown_manager | ||
| 8 | + | ||
| 9 | + | ||
| 10 | +class _FakeManager: | ||
| 11 | + def __init__(self): | ||
| 12 | + self.shutdown = mock.AsyncMock() | ||
| 13 | + | ||
| 14 | + | ||
| 15 | +class TestShutdownManagerIdempotency(TestCase): | ||
| 16 | + """_shutdown_manager 应以 manager 实例为粒度幂等,而非模块级一次性标记。""" | ||
| 17 | + | ||
| 18 | + def test_each_manager_shutdown_once(self): | ||
| 19 | + first = _FakeManager() | ||
| 20 | + second = _FakeManager() | ||
| 21 | + | ||
| 22 | + _shutdown_manager(first) | ||
| 23 | + _shutdown_manager(first) | ||
| 24 | + | ||
| 25 | + self.assertEqual(first.shutdown.await_count, 1) | ||
| 26 | + | ||
| 27 | + # 同一进程内再次调用 CLI 产生的 manager 仍应被关闭 | ||
| 28 | + _shutdown_manager(second) | ||
| 29 | + self.assertEqual(second.shutdown.await_count, 1) | ||
| @@ -41,12 +41,16 @@ async def package_python_to_whl( | |||
| 41 | if not setup_py.exists() and not pyproject_toml.exists(): | 41 | if not setup_py.exists() and not pyproject_toml.exists(): |
| 42 | raise ValueError(f"No setup.py or pyproject.toml found in {source_dir}") | 42 | raise ValueError(f"No setup.py or pyproject.toml found in {source_dir}") |
| 43 | 43 | ||
| 44 | - result = subprocess.run( | 44 | + try: |
| 45 | - [sys.executable, "-m", "build", "--wheel", "--outdir", str(output_path)], | 45 | + result = subprocess.run( |
| 46 | - cwd=str(source_path), | 46 | + [sys.executable, "-m", "build", "--wheel", "--outdir", str(output_path)], |
| 47 | - capture_output=True, | 47 | + cwd=str(source_path), |
| 48 | - text=True, | 48 | + capture_output=True, |
| 49 | - ) | 49 | + text=True, |
| 50 | + timeout=600, | ||
| 51 | + ) | ||
| 52 | + except subprocess.TimeoutExpired as e: | ||
| 53 | + raise RuntimeError(f"Build timed out after {e.timeout}s") from e | ||
| 50 | 54 | ||
| 51 | if result.returncode != 0: | 55 | if result.returncode != 0: |
| 52 | raise RuntimeError(f"Build failed: {result.stderr}") | 56 | raise RuntimeError(f"Build failed: {result.stderr}") |
| @@ -81,8 +85,11 @@ def create_virtualenv(venv_path: str, python_version: Optional[str] = None) -> s | |||
| 81 | 85 | ||
| 82 | cmd = [sys.executable, "-m", "venv", str(venv_path)] | 86 | cmd = [sys.executable, "-m", "venv", str(venv_path)] |
| 83 | 87 | ||
| 84 | - result = subprocess.run(cmd, capture_output=True, text=True) | 88 | + try: |
| 85 | - | 89 | + result = subprocess.run(cmd, capture_output=True, text=True, timeout=300) |
| 90 | + except subprocess.TimeoutExpired as e: | ||
| 91 | + raise RuntimeError(f"Failed to create virtualenv: timed out after {e.timeout}s") from e | ||
| 92 | + | ||
| 86 | if result.returncode != 0: | 93 | if result.returncode != 0: |
| 87 | raise RuntimeError(f"Failed to create virtualenv: {result.stderr}") | 94 | raise RuntimeError(f"Failed to create virtualenv: {result.stderr}") |
| 88 | 95 | ||
| @@ -118,12 +125,16 @@ def install_package(venv_path: str, package: str) -> bool: | |||
| 118 | """ | 125 | """ |
| 119 | pip_path = get_pip_path(venv_path) | 126 | pip_path = get_pip_path(venv_path) |
| 120 | 127 | ||
| 121 | - result = subprocess.run( | 128 | + try: |
| 122 | - [pip_path, "install", package], | 129 | + result = subprocess.run( |
| 123 | - capture_output=True, | 130 | + [pip_path, "install", package], |
| 124 | - text=True, | 131 | + capture_output=True, |
| 125 | - ) | 132 | + text=True, |
| 126 | - | 133 | + timeout=600, |
| 134 | + ) | ||
| 135 | + except subprocess.TimeoutExpired: | ||
| 136 | + return False | ||
| 137 | + | ||
| 127 | return result.returncode == 0 | 138 | return result.returncode == 0 |
| 128 | 139 | ||
| 129 | 140 | ||
| @@ -140,10 +151,14 @@ def uninstall_package(venv_path: str, package: str) -> bool: | |||
| 140 | """ | 151 | """ |
| 141 | pip_path = get_pip_path(venv_path) | 152 | pip_path = get_pip_path(venv_path) |
| 142 | 153 | ||
| 143 | - result = subprocess.run( | 154 | + try: |
| 144 | - [pip_path, "uninstall", "-y", package], | 155 | + result = subprocess.run( |
| 145 | - capture_output=True, | 156 | + [pip_path, "uninstall", "-y", package], |
| 146 | - text=True, | 157 | + capture_output=True, |
| 147 | - ) | 158 | + text=True, |
| 148 | - | 159 | + timeout=300, |
| 160 | + ) | ||
| 161 | + except subprocess.TimeoutExpired: | ||
| 162 | + return False | ||
| 163 | + | ||
| 149 | return result.returncode == 0 | 164 | return result.returncode == 0 |
| @@ -73,7 +73,8 @@ class VirtualEnvironmentManager: | |||
| 73 | result = subprocess.run( | 73 | result = subprocess.run( |
| 74 | [uv_exe, "venv", str(venv_path)], | 74 | [uv_exe, "venv", str(venv_path)], |
| 75 | capture_output=True, | 75 | capture_output=True, |
| 76 | - text=True | 76 | + text=True, |
| 77 | + timeout=300, | ||
| 77 | ) | 78 | ) |
| 78 | 79 | ||
| 79 | if result.returncode != 0: | 80 | if result.returncode != 0: |
| @@ -103,7 +104,8 @@ class VirtualEnvironmentManager: | |||
| 103 | ensure_result = subprocess.run( | 104 | ensure_result = subprocess.run( |
| 104 | [str(python_exe), "-m", "ensurepip", "--upgrade"], | 105 | [str(python_exe), "-m", "ensurepip", "--upgrade"], |
| 105 | capture_output=True, | 106 | capture_output=True, |
| 106 | - text=True | 107 | + text=True, |
| 108 | + timeout=300, | ||
| 107 | ) | 109 | ) |
| 108 | if ensure_result.returncode != 0: | 110 | if ensure_result.returncode != 0: |
| 109 | logger.warning( | 111 | logger.warning( |
| @@ -114,7 +116,8 @@ class VirtualEnvironmentManager: | |||
| 114 | get_pip_result = subprocess.run( | 116 | get_pip_result = subprocess.run( |
| 115 | [str(python_exe), "-m", "pip", "install", "--upgrade", "pip"], | 117 | [str(python_exe), "-m", "pip", "install", "--upgrade", "pip"], |
| 116 | capture_output=True, | 118 | capture_output=True, |
| 117 | - text=True | 119 | + text=True, |
| 120 | + timeout=300, | ||
| 118 | ) | 121 | ) |
| 119 | if get_pip_result.returncode != 0: | 122 | if get_pip_result.returncode != 0: |
| 120 | logger.error("Failed to bootstrap pip: %s", get_pip_result.stderr) | 123 | logger.error("Failed to bootstrap pip: %s", get_pip_result.stderr) |
| @@ -122,9 +125,20 @@ class VirtualEnvironmentManager: | |||
| 122 | 125 | ||
| 123 | logger.info("Virtual environment created successfully: %s", venv_path) | 126 | logger.info("Virtual environment created successfully: %s", venv_path) |
| 124 | return venv_path | 127 | return venv_path |
| 128 | + except subprocess.TimeoutExpired as e: | ||
| 129 | + logger.error("Timed out creating virtual environment: %s", venv_path) | ||
| 130 | + shutil.rmtree(venv_path, ignore_errors=True) | ||
| 131 | + raise RuntimeError( | ||
| 132 | + f"Failed to create venv: command timed out after {e.timeout}s" | ||
| 133 | + ) from e | ||
| 125 | except subprocess.CalledProcessError as e: | 134 | except subprocess.CalledProcessError as e: |
| 126 | logger.error("Failed to create virtual environment: %s", e.stderr) | 135 | logger.error("Failed to create virtual environment: %s", e.stderr) |
| 136 | + shutil.rmtree(venv_path, ignore_errors=True) | ||
| 127 | raise RuntimeError(f"Failed to create venv: {e}") from e | 137 | raise RuntimeError(f"Failed to create venv: {e}") from e |
| 138 | + except Exception: | ||
| 139 | + # 清理半成品 venv,避免下次调用因目录已存在而误判为创建成功 | ||
| 140 | + shutil.rmtree(venv_path, ignore_errors=True) | ||
| 141 | + raise | ||
| 128 | 142 | ||
| 129 | def get_python_executable(self, deployment_id: str) -> Path: | 143 | def get_python_executable(self, deployment_id: str) -> Path: |
| 130 | """ | 144 | """ |
| @@ -211,6 +225,7 @@ class VirtualEnvironmentManager: | |||
| 211 | capture_output=True, | 225 | capture_output=True, |
| 212 | text=True, | 226 | text=True, |
| 213 | encoding="utf-8", | 227 | encoding="utf-8", |
| 228 | + timeout=1800, | ||
| 214 | ) | 229 | ) |
| 215 | 230 | ||
| 216 | if result.returncode == 0: | 231 | if result.returncode == 0: |
| @@ -0,0 +1,53 @@ | |||
| 1 | +# coding: utf-8 | ||
| 2 | +# Copyright (c) Huawei Technologies Co., Ltd. 2026-2026. All rights reserved | ||
| 3 | + | ||
| 4 | +import subprocess | ||
| 5 | +import tempfile | ||
| 6 | +from pathlib import Path | ||
| 7 | +from unittest import TestCase | ||
| 8 | +from unittest import mock | ||
| 9 | + | ||
| 10 | +from openjiuwen_runtime.foundation.venv_manager import VirtualEnvironmentManager | ||
| 11 | + | ||
| 12 | +_UV_RESOLVER = "openjiuwen_runtime.foundation.venv_manager._resolve_uv_executable" | ||
| 13 | +_SUBPROCESS_RUN = "openjiuwen_runtime.foundation.venv_manager.subprocess.run" | ||
| 14 | + | ||
| 15 | + | ||
| 16 | +class TestCreateVenvFailureCleanup(TestCase): | ||
| 17 | + """create_venv 失败时必须清理半成品 venv,避免下次调用误判为已创建。""" | ||
| 18 | + | ||
| 19 | + def test_timeout_removes_partial_venv(self): | ||
| 20 | + manager = VirtualEnvironmentManager() | ||
| 21 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 22 | + venv_path = Path(tmp) / "deploy-timeout" / ".venv" | ||
| 23 | + | ||
| 24 | + def fake_run(cmd, *args, **kwargs): | ||
| 25 | + # 模拟 uv 已创建部分目录后超时 | ||
| 26 | + venv_path.mkdir(parents=True, exist_ok=True) | ||
| 27 | + raise subprocess.TimeoutExpired(cmd=cmd, timeout=300) | ||
| 28 | + | ||
| 29 | + with mock.patch.object(manager, "get_venv_path", return_value=venv_path), \ | ||
| 30 | + mock.patch(_UV_RESOLVER, return_value="uv"), \ | ||
| 31 | + mock.patch(_SUBPROCESS_RUN, side_effect=fake_run): | ||
| 32 | + with self.assertRaises(RuntimeError): | ||
| 33 | + manager.create_venv("deploy-timeout") | ||
| 34 | + | ||
| 35 | + self.assertFalse(venv_path.exists()) | ||
| 36 | + | ||
| 37 | + def test_missing_python_executable_removes_partial_venv(self): | ||
| 38 | + manager = VirtualEnvironmentManager() | ||
| 39 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 40 | + venv_path = Path(tmp) / "deploy-partial" / ".venv" | ||
| 41 | + | ||
| 42 | + def fake_run(cmd, *args, **kwargs): | ||
| 43 | + # 目录已建立但缺少解释器,随后应整体清理 | ||
| 44 | + venv_path.mkdir(parents=True, exist_ok=True) | ||
| 45 | + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") | ||
| 46 | + | ||
| 47 | + with mock.patch.object(manager, "get_venv_path", return_value=venv_path), \ | ||
| 48 | + mock.patch(_UV_RESOLVER, return_value="uv"), \ | ||
| 49 | + mock.patch(_SUBPROCESS_RUN, side_effect=fake_run): | ||
| 50 | + with self.assertRaises(RuntimeError): | ||
| 51 | + manager.create_venv("deploy-partial") | ||
| 52 | + | ||
| 53 | + self.assertFalse(venv_path.exists()) | ||
| @@ -47,7 +47,7 @@ class LocalSubprocessDeployer(Deployer[SubprocessParams]): | |||
| 47 | self.default_host = default_host | 47 | self.default_host = default_host |
| 48 | self.default_port_start = default_port_start | 48 | self.default_port_start = default_port_start |
| 49 | self.venv_manager = VirtualEnvironmentManager() | 49 | self.venv_manager = VirtualEnvironmentManager() |
| 50 | - self._processes: Dict[str, asyncio.subprocess.Process] = {} | 50 | + self._processes: Dict[str, subprocess.Popen] = {} |
| 51 | 51 | ||
| 52 | def _kill_by_pid(self, pid: int) -> bool: | 52 | def _kill_by_pid(self, pid: int) -> bool: |
| 53 | """通过 PID 终止进程(跨进程有效)""" | 53 | """通过 PID 终止进程(跨进程有效)""" |
| @@ -196,14 +196,19 @@ class LocalSubprocessDeployer(Deployer[SubprocessParams]): | |||
| 196 | log_dir.mkdir(parents=True, exist_ok=True) | 196 | log_dir.mkdir(parents=True, exist_ok=True) |
| 197 | log_file = log_dir / "agent.log" | 197 | log_file = log_dir / "agent.log" |
| 198 | log_fp = open(log_file, "a", encoding="utf-8") | 198 | log_fp = open(log_file, "a", encoding="utf-8") |
| 199 | - | 199 | + try: |
| 200 | - process = subprocess.Popen( | 200 | + process = subprocess.Popen( |
| 201 | - cmd, | 201 | + cmd, |
| 202 | - env=env, | 202 | + env=env, |
| 203 | - stdout=log_fp, | 203 | + stdout=log_fp, |
| 204 | - stderr=log_fp, | 204 | + stderr=log_fp, |
| 205 | - creationflags=creation_flags, | 205 | + creationflags=creation_flags, |
| 206 | - ) | 206 | + ) |
| 207 | + finally: | ||
| 208 | + # 子进程已继承日志句柄,父进程不再需要保留 | ||
| 209 | + log_fp.close() | ||
| 210 | + # 登记进程,供 stop/get_status 精确管理 | ||
| 211 | + self._processes[deployment_id] = process | ||
| 207 | logger.info("Agent log file: %s", log_file) | 212 | logger.info("Agent log file: %s", log_file) |
| 208 | 213 | ||
| 209 | # 6. 等待进程启动并检查状态 | 214 | # 6. 等待进程启动并检查状态 |
| @@ -211,7 +216,7 @@ class LocalSubprocessDeployer(Deployer[SubprocessParams]): | |||
| 211 | 216 | ||
| 212 | if process.poll() is not None: | 217 | if process.poll() is not None: |
| 213 | # 进程已经退出,从日志文件读取错误信息 | 218 | # 进程已经退出,从日志文件读取错误信息 |
| 214 | - log_fp.close() | 219 | + self._processes.pop(deployment_id, None) |
| 215 | error_msg = "Unknown error" | 220 | error_msg = "Unknown error" |
| 216 | try: | 221 | try: |
| 217 | error_msg = log_file.read_text(encoding="utf-8", errors="ignore").strip() or error_msg | 222 | error_msg = log_file.read_text(encoding="utf-8", errors="ignore").strip() or error_msg |
| @@ -261,22 +266,29 @@ class LocalSubprocessDeployer(Deployer[SubprocessParams]): | |||
| 261 | logger.info("Stopping subprocess: deployment_id=%s, pid=%s", deployment_id, pid) | 266 | logger.info("Stopping subprocess: deployment_id=%s, pid=%s", deployment_id, pid) |
| 262 | try: | 267 | try: |
| 263 | if deployment_id in self._processes: | 268 | if deployment_id in self._processes: |
| 264 | - process = self._processes[deployment_id] | 269 | + process = self._processes.pop(deployment_id, None) |
| 265 | - logger.debug( | 270 | + if process is not None: |
| 266 | - "Terminating process: deployment_id=%s, pid=%s", | 271 | + logger.debug( |
| 267 | - deployment_id, | 272 | + "Terminating process: deployment_id=%s, pid=%s", |
| 268 | - process.pid, | ||
| 269 | - ) | ||
| 270 | - process.terminate() | ||
| 271 | - try: | ||
| 272 | - await asyncio.wait_for(process.wait(), timeout=10) | ||
| 273 | - except asyncio.TimeoutError: | ||
| 274 | - logger.warning( | ||
| 275 | - "Process did not terminate gracefully, killing: deployment_id=%s", | ||
| 276 | deployment_id, | 273 | deployment_id, |
| 274 | + process.pid, | ||
| 277 | ) | 275 | ) |
| 278 | - process.kill() | 276 | + process.terminate() |
| 279 | - del self._processes[deployment_id] | 277 | + try: |
| 278 | + await asyncio.to_thread(process.wait, 10) | ||
| 279 | + except subprocess.TimeoutExpired: | ||
| 280 | + logger.warning( | ||
| 281 | + "Process did not terminate gracefully, killing: deployment_id=%s", | ||
| 282 | + deployment_id, | ||
| 283 | + ) | ||
| 284 | + process.kill() | ||
| 285 | + try: | ||
| 286 | + await asyncio.wait_for(asyncio.to_thread(process.wait), timeout=10) | ||
| 287 | + except asyncio.TimeoutError: | ||
| 288 | + logger.warning( | ||
| 289 | + "Process still alive after kill: deployment_id=%s", | ||
| 290 | + deployment_id, | ||
| 291 | + ) | ||
| 280 | elif pid: | 292 | elif pid: |
| 281 | success = self._kill_by_pid(pid) | 293 | success = self._kill_by_pid(pid) |
| 282 | if not success: | 294 | if not success: |
| @@ -324,10 +336,10 @@ class LocalSubprocessDeployer(Deployer[SubprocessParams]): | |||
| 324 | 336 | ||
| 325 | if deployment_id in self._processes: | 337 | if deployment_id in self._processes: |
| 326 | process = self._processes[deployment_id] | 338 | process = self._processes[deployment_id] |
| 327 | - if process.returncode is None: | 339 | + # poll() 会刷新 returncode,进程自然退出时也能被正确识别 |
| 340 | + if process.poll() is None: | ||
| 328 | return DeploymentStatus.RUNNING | 341 | return DeploymentStatus.RUNNING |
| 329 | - else: | 342 | + return DeploymentStatus.STOPPED |
| 330 | - return DeploymentStatus.STOPPED | ||
| 331 | 343 | ||
| 332 | if pid: | 344 | if pid: |
| 333 | is_running = self._check_process_by_pid(pid) | 345 | is_running = self._check_process_by_pid(pid) |
| @@ -0,0 +1,125 @@ | |||
| 1 | +# coding: utf-8 | ||
| 2 | +# Copyright (c) Huawei Technologies Co., Ltd. 2026-2026. All rights reserved | ||
| 3 | + | ||
| 4 | +import asyncio | ||
| 5 | +import subprocess | ||
| 6 | +import tempfile | ||
| 7 | +from pathlib import Path | ||
| 8 | +from unittest import TestCase | ||
| 9 | + | ||
| 10 | +from openjiuwen_runtime.management.deployments.subprocess.deployer import ( | ||
| 11 | + LocalSubprocessDeployer, | ||
| 12 | +) | ||
| 13 | +from openjiuwen_runtime.management.models.enums import DeploymentStatus | ||
| 14 | + | ||
| 15 | + | ||
| 16 | +class _FakePopen: | ||
| 17 | + """模拟同步 subprocess.Popen:returncode 仅在 poll() 后才会刷新。""" | ||
| 18 | + | ||
| 19 | + def __init__(self, pid: int = 4321): | ||
| 20 | + self.pid = pid | ||
| 21 | + self.returncode = None | ||
| 22 | + self._exited = False | ||
| 23 | + self.poll_calls = 0 | ||
| 24 | + | ||
| 25 | + def exit_naturally(self) -> None: | ||
| 26 | + self._exited = True | ||
| 27 | + | ||
| 28 | + def poll(self): | ||
| 29 | + self.poll_calls += 1 | ||
| 30 | + if self._exited: | ||
| 31 | + self.returncode = 0 | ||
| 32 | + return self.returncode | ||
| 33 | + | ||
| 34 | + | ||
| 35 | +class _StoppablePopen: | ||
| 36 | + """模拟同步 Popen,记录 terminate/kill,wait 支持一次超时。""" | ||
| 37 | + | ||
| 38 | + def __init__(self, pid: int = 5678, timeout_first: bool = True): | ||
| 39 | + self.pid = pid | ||
| 40 | + self.terminated = False | ||
| 41 | + self.killed = False | ||
| 42 | + self._timeout_first = timeout_first | ||
| 43 | + self._wait_calls = 0 | ||
| 44 | + | ||
| 45 | + def terminate(self) -> None: | ||
| 46 | + self.terminated = True | ||
| 47 | + | ||
| 48 | + def kill(self) -> None: | ||
| 49 | + self.killed = True | ||
| 50 | + | ||
| 51 | + def wait(self, timeout=None) -> int: | ||
| 52 | + self._wait_calls += 1 | ||
| 53 | + if self._timeout_first and self._wait_calls == 1: | ||
| 54 | + raise subprocess.TimeoutExpired(cmd="agent", timeout=timeout) | ||
| 55 | + return 0 | ||
| 56 | + | ||
| 57 | + | ||
| 58 | +class TestGetStatusRefreshesReturncode(TestCase): | ||
| 59 | + def test_natural_exit_detected_as_stopped(self): | ||
| 60 | + deployer = LocalSubprocessDeployer() | ||
| 61 | + process = _FakePopen() | ||
| 62 | + getattr(deployer, "_processes")["dep-1"] = process | ||
| 63 | + | ||
| 64 | + self.assertEqual( | ||
| 65 | + asyncio.run(deployer.get_status("dep-1")), | ||
| 66 | + DeploymentStatus.RUNNING, | ||
| 67 | + ) | ||
| 68 | + | ||
| 69 | + # 进程自然退出后 returncode 尚未被读取过,get_status 需通过 poll() 刷新 | ||
| 70 | + process.exit_naturally() | ||
| 71 | + | ||
| 72 | + self.assertEqual( | ||
| 73 | + asyncio.run(deployer.get_status("dep-1")), | ||
| 74 | + DeploymentStatus.STOPPED, | ||
| 75 | + ) | ||
| 76 | + self.assertGreaterEqual(process.poll_calls, 2) | ||
| 77 | + | ||
| 78 | + def test_running_process_stays_running(self): | ||
| 79 | + deployer = LocalSubprocessDeployer() | ||
| 80 | + process = _FakePopen() | ||
| 81 | + getattr(deployer, "_processes")["dep-2"] = process | ||
| 82 | + | ||
| 83 | + self.assertEqual( | ||
| 84 | + asyncio.run(deployer.get_status("dep-2")), | ||
| 85 | + DeploymentStatus.RUNNING, | ||
| 86 | + ) | ||
| 87 | + | ||
| 88 | + | ||
| 89 | +class TestStopRegisteredProcess(TestCase): | ||
| 90 | + """登记同步 Popen 后 stop() 的优雅分支必须成功返回并完成清理。""" | ||
| 91 | + | ||
| 92 | + def _deployer_with(self, process) -> LocalSubprocessDeployer: | ||
| 93 | + deployer = LocalSubprocessDeployer() | ||
| 94 | + getattr(deployer, "_processes")["dep-stop"] = process | ||
| 95 | + return deployer | ||
| 96 | + | ||
| 97 | + def test_graceful_stop_succeeds_and_cleans_up(self): | ||
| 98 | + process = _StoppablePopen(timeout_first=False) | ||
| 99 | + deployer = self._deployer_with(process) | ||
| 100 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 101 | + venv_path = Path(tmp) / "venv" | ||
| 102 | + venv_path.mkdir() | ||
| 103 | + | ||
| 104 | + result = asyncio.run(deployer.stop("dep-stop", venv_path=str(venv_path))) | ||
| 105 | + | ||
| 106 | + self.assertTrue(result.success) | ||
| 107 | + self.assertTrue(process.terminated) | ||
| 108 | + self.assertFalse(process.killed) | ||
| 109 | + self.assertNotIn("dep-stop", getattr(deployer, "_processes")) | ||
| 110 | + self.assertFalse(venv_path.exists()) | ||
| 111 | + | ||
| 112 | + def test_stop_kills_when_terminate_times_out(self): | ||
| 113 | + process = _StoppablePopen(timeout_first=True) | ||
| 114 | + deployer = self._deployer_with(process) | ||
| 115 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 116 | + venv_path = Path(tmp) / "venv" | ||
| 117 | + venv_path.mkdir() | ||
| 118 | + | ||
| 119 | + result = asyncio.run(deployer.stop("dep-stop", venv_path=str(venv_path))) | ||
| 120 | + | ||
| 121 | + self.assertTrue(result.success) | ||
| 122 | + self.assertTrue(process.terminated) | ||
| 123 | + self.assertTrue(process.killed) | ||
| 124 | + self.assertNotIn("dep-stop", getattr(deployer, "_processes")) | ||
| 125 | + self.assertFalse(venv_path.exists()) | ||
🟠 High Priority
改动点(deployer.py:211)新增
self._processes[deployment_id] = process,而process来自subprocess.Popen(...)(第 200 行,标准库同步 Popen),并非asyncio.subprocess.Process(注意第 50 行注解Dict[str, asyncio.subprocess.Process]与实际存入类型不符)。契约/可达性变化:改动前
self._processes始终为空,stop()(第 268-284 行)的if deployment_id in self._processes:分支是死代码,实际走elif pid: self._kill_by_pid(pid),能正常返回 success=True。改动后同一 manager/deployer 实例(BaseDeploymentStrategy.__init__只创建一次self._deployer,deploy 与 stop 共用)内该分支变为可达。失败模式:进入该分支后第 275 行
process.terminate()之后执行await asyncio.wait_for(process.wait(), timeout=10)。Popen.wait()是阻塞方法、返回 int,先被同步求值(阻塞事件循环直到子进程退出),随后asyncio.wait_for(<int>, ...)因参数非 awaitable 抛TypeError(ensure_future 失败)。后果:TypeError被stop()外层except Exception吞掉,返回DeployResult(success=False),即使进程已被 terminate——把成功停止误报为失败;del self._processes[deployment_id]与第 290-292 行shutil.rmtree(venv_path)均被跳过,虚拟环境不被清理、登记项残留;process.wait()无超时地永久阻塞事件循环,本应生效的 10s 超时与 kill() 兜底完全失效(与 PR 描述「走 process.terminate() 优雅停止」的意图相反)。建议:在 stop() 的
deployment_id in self._processes分支中用非阻塞方式等待(run_in_executor 包装 Popen.wait 再 wait_for,或直接process.wait(timeout=10)+ 捕获 TimeoutExpired 后 kill),并把_processes注解改为Dict[str, subprocess.Popen]。