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

@@ -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

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)

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

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):

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()