From 0d6ee3dc5a85e6c49caa2eb41660fc5c33cdfc7d Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:59:26 -0700 Subject: [PATCH] test: count a zombie grandchild as gone in the migrate deploy timeout test (#42570) * test: count a zombie grandchild as gone in the migrate deploy timeout test A SIGKILLed grandchild whose parent died in the same killpg reparents to PID 1 or the nearest subreaper and stays a zombie until reaped, and signal 0 still succeeds on a zombie, so the timeout test read it as alive wherever PID 1 is slow to reap or never does. The sibling test in tests/test_litellm/proxy/db already handled that; both now share one process_is_gone helper that reads the /proc state and reaps its own children, with unit tests for the live, reaped, unreaped, and foreign zombie shapes. * test: move the pre-commit interrupt test onto the shared zombie-aware liveness helper --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com> --- tests/_process_helpers.py | 57 ++++++++++++++++ .../test_prisma_toolchain.py | 18 ++--- tests/test_litellm/proxy/db/conftest.py | 32 ++------- tests/test_litellm/test_pre_commit_lint.py | 12 +--- tests/test_litellm/test_process_helpers.py | 65 +++++++++++++++++++ 5 files changed, 133 insertions(+), 51 deletions(-) create mode 100644 tests/_process_helpers.py create mode 100644 tests/test_litellm/test_process_helpers.py diff --git a/tests/_process_helpers.py b/tests/_process_helpers.py new file mode 100644 index 00000000000..9f80c563fa9 --- /dev/null +++ b/tests/_process_helpers.py @@ -0,0 +1,57 @@ +"""Whether a killed process is really gone, for tests that kill whole process trees. + +A SIGKILLed grandchild whose parent died in the same ``killpg`` reparents to the +nearest subreaper or PID 1, and until that ancestor reaps it the pid is a zombie +that ``os.kill(pid, 0)`` still accepts. Reading its ``/proc`` state, and reaping +it when it landed on this process, keeps a runner that is slow to reap, or never +does, from turning a dead process into a failed assertion. The reap comes after +the liveness read so a child seen dying between the two is still collected on +the next poll instead of staying this process's own zombie. +""" + +import os +import time +from pathlib import Path +from typing import Final + +POLL_INTERVAL_S: Final = 0.05 + + +def _exists(pid: int) -> bool: + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + return True + + +def _is_zombie(pid: int) -> bool: + try: + stat: Final = Path(f"/proc/{pid}/stat").read_text() + except OSError: + return False + return stat.rpartition(")")[2].split()[0] == "Z" + + +def _reap_if_ours(pid: int) -> None: + if os.name == "nt": + return + try: + os.waitpid(pid, os.WNOHANG) + except ChildProcessError: + pass + + +def _gone_now(pid: int) -> bool: + dead: Final = not _exists(pid) or _is_zombie(pid) + _reap_if_ours(pid) + return dead + + +def process_is_gone(pid: int, within_seconds: float) -> bool: + deadline: Final = time.monotonic() + within_seconds + while time.monotonic() < deadline: + if _gone_now(pid): + return True + time.sleep(POLL_INTERVAL_S) + return False diff --git a/tests/proxy_migration_tests/test_prisma_toolchain.py b/tests/proxy_migration_tests/test_prisma_toolchain.py index 4e2274cc582..556c680a84a 100644 --- a/tests/proxy_migration_tests/test_prisma_toolchain.py +++ b/tests/proxy_migration_tests/test_prisma_toolchain.py @@ -25,7 +25,6 @@ from collections.abc import Callable from pathlib import Path import pytest - from litellm_proxy_extras.prisma_toolchain import ( DEFAULT_PRISMA_COMMAND_TIMEOUT, DEFAULT_PRISMA_MIGRATE_DEPLOY_TIMEOUT, @@ -36,14 +35,16 @@ from litellm_proxy_extras.prisma_toolchain import ( heal_incomplete_nodeenv_cache, node_binary_path, prisma_bootstrap_timeout, - prisma_command_timeout, prisma_cli_available, + prisma_command_timeout, prisma_migrate_deploy_timeout, resolve_prisma_argv, run_prisma, ) from litellm_proxy_extras.utils import ProxyExtrasDBManager +from tests._process_helpers import process_is_gone + REPO_ROOT = Path(__file__).resolve().parents[2] PROXY_EXTRAS = REPO_ROOT / "litellm-proxy-extras" / "litellm_proxy_extras" @@ -280,17 +281,6 @@ def test_migrate_deploy_stops_at_its_own_timeout( assert elapsed < 30 -def _process_is_gone(pid: int, within_seconds: float) -> bool: - deadline = time.monotonic() + within_seconds - while time.monotonic() < deadline: - try: - os.kill(pid, 0) - except ProcessLookupError: - return True - time.sleep(0.05) - return False - - def test_a_timed_out_migrate_deploy_takes_its_process_tree_with_it( toolchain_env: tuple[Path, Path], monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: @@ -309,7 +299,7 @@ def test_a_timed_out_migrate_deploy_takes_its_process_tree_with_it( grandchild_pid = int(pidfile.read_text()) try: assert len(_deploy_calls(log_path)) == 2 - assert _process_is_gone(grandchild_pid, within_seconds=5) + assert process_is_gone(grandchild_pid, within_seconds=5) finally: try: os.kill(grandchild_pid, signal.SIGKILL) diff --git a/tests/test_litellm/proxy/db/conftest.py b/tests/test_litellm/proxy/db/conftest.py index bcb7794a20d..4d775142dd5 100644 --- a/tests/test_litellm/proxy/db/conftest.py +++ b/tests/test_litellm/proxy/db/conftest.py @@ -2,14 +2,15 @@ import json import os import signal import sys -import time from collections.abc import Generator from dataclasses import dataclass from pathlib import Path -from typing import Final, Optional +from typing import Optional import pytest +from tests._process_helpers import process_is_gone + DB_ENV_KEYS = ( "IAM_TOKEN_DB_AUTH", "AZURE_POSTGRESQL_AUTH", @@ -35,14 +36,6 @@ DB_ENV_KEYS = ( _db_env_snapshot_key = pytest.StashKey[dict[str, Optional[str]]]() -def _is_zombie(pid: int) -> bool: - try: - stat: Final = Path(f"/proc/{pid}/stat").read_text() - except OSError: - return False - return stat.rpartition(")")[2].split()[0] == "Z" - - def _db_env_snapshot() -> dict[str, Optional[str]]: return {key: os.environ.get(key) for key in DB_ENV_KEYS} @@ -130,24 +123,7 @@ class FakePrismaCli: return [json.loads(line) for line in self.calls_file.read_text().splitlines()] def grandchild_is_gone(self, within_seconds: float) -> bool: - pid: Final = int(self.grandchild_pidfile.read_text()) - deadline: Final = time.monotonic() + within_seconds - while time.monotonic() < deadline: - if os.name != "nt": - try: - reaped_pid, _ = os.waitpid(pid, os.WNOHANG) - if reaped_pid == pid: - return True - except ChildProcessError: - pass - try: - os.kill(pid, 0) - except ProcessLookupError: - return True - if _is_zombie(pid): - return True - time.sleep(0.05) - return False + return process_is_gone(int(self.grandchild_pidfile.read_text()), within_seconds=within_seconds) @pytest.fixture diff --git a/tests/test_litellm/test_pre_commit_lint.py b/tests/test_litellm/test_pre_commit_lint.py index e12da0833dc..2d7fa897536 100644 --- a/tests/test_litellm/test_pre_commit_lint.py +++ b/tests/test_litellm/test_pre_commit_lint.py @@ -9,6 +9,8 @@ from pathlib import Path import pytest +from tests._process_helpers import process_is_gone + ROOT = Path(__file__).resolve().parents[2] SCRIPT = ROOT / "scripts" / "pre_commit_lint.sh" WHOLE_TREE_RUFF = "run --no-sync ruff check --config ruff-tests.toml tests" @@ -343,14 +345,6 @@ def _wait_until(predicate: Callable[[], bool], timeout_seconds: float) -> bool: return predicate() -def _pid_gone(pid: int) -> bool: - try: - os.kill(pid, 0) - except ProcessLookupError: - return True - return False - - def test_interrupt_kills_background_jobs_and_removes_logs(tmp_path: Path) -> None: repo, bin_dir = _sandbox(tmp_path) hang_dir = tmp_path / "hang" @@ -372,7 +366,7 @@ def test_interrupt_kills_background_jobs_and_removes_logs(tmp_path: Path) -> Non os.killpg(proc.pid, signal.SIGINT) assert proc.wait(timeout=10) != 0 make_pid = int((hang_dir / "make.pid").read_text()) - assert _wait_until(lambda: _pid_gone(make_pid), 5) + assert process_is_gone(make_pid, within_seconds=5) assert _wait_until(lambda: not any(tmp_dir.iterdir()), 5), list(tmp_dir.iterdir()) finally: with suppress(ProcessLookupError, PermissionError): diff --git a/tests/test_litellm/test_process_helpers.py b/tests/test_litellm/test_process_helpers.py new file mode 100644 index 00000000000..00d4d07d71f --- /dev/null +++ b/tests/test_litellm/test_process_helpers.py @@ -0,0 +1,65 @@ +"""``process_is_gone`` has to say gone for every shape a killed process can take, and never for a live one.""" + +import os +import signal +import subprocess +import sys +from pathlib import Path +from typing import Final + +import pytest + +from tests._process_helpers import process_is_gone + +SLEEP_FOREVER: Final = (sys.executable, "-I", "-c", "import time; time.sleep(600)") + +LEAVE_A_ZOMBIE_BEHIND: Final = """ +import os, signal, subprocess, sys, time +grandchild = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(600)"]) +os.kill(grandchild.pid, signal.SIGKILL) +while open(f"/proc/{grandchild.pid}/stat").read().rpartition(")")[2].split()[0] != "Z": + time.sleep(0.01) +print(grandchild.pid, flush=True) +time.sleep(600) +""" + + +def test_a_live_process_is_not_gone() -> None: + child: Final = subprocess.Popen(SLEEP_FOREVER) + try: + assert not process_is_gone(child.pid, within_seconds=0.3) + finally: + child.kill() + child.wait() + + +def test_a_reaped_child_is_gone() -> None: + child: Final = subprocess.Popen(SLEEP_FOREVER) + child.kill() + child.wait() + assert process_is_gone(child.pid, within_seconds=1) + + +@pytest.mark.skipif(os.name == "nt", reason="zombies are a POSIX thing") +def test_an_unreaped_child_is_reaped_and_gone() -> None: + child: Final = subprocess.Popen(SLEEP_FOREVER) + os.kill(child.pid, signal.SIGKILL) + assert process_is_gone(child.pid, within_seconds=1) + with pytest.raises(ChildProcessError): + os.waitpid(child.pid, os.WNOHANG) + + +@pytest.mark.skipif(not Path("/proc").is_dir(), reason="needs procfs to see a zombie that is not our child") +def test_a_zombie_left_by_another_process_is_gone() -> None: + zombie_factory: Final = subprocess.Popen( + [sys.executable, "-I", "-c", LEAVE_A_ZOMBIE_BEHIND], stdout=subprocess.PIPE, text=True + ) + try: + assert zombie_factory.stdout is not None + zombie_pid: Final = int(zombie_factory.stdout.readline()) + with pytest.raises(ChildProcessError): + os.waitpid(zombie_pid, os.WNOHANG) + assert process_is_gone(zombie_pid, within_seconds=1) + finally: + zombie_factory.kill() + zombie_factory.wait()