diff --git a/tests/tools/test_local_setsid_descendant_sweep.py b/tests/tools/test_local_setsid_descendant_sweep.py new file mode 100644 index 0000000000..8ae35d68f7 --- /dev/null +++ b/tests/tools/test_local_setsid_descendant_sweep.py @@ -0,0 +1,148 @@ +"""Regression tests for #85125 Phase 4b (terminal flavor of the #71148 class). + +LocalEnvironment._kill_process kills the process GROUP (SIGTERM -> wait -> +SIGKILL). A descendant that called ``setsid`` escapes the group and survives +the group-kill — the local sibling of issue #84967. The fix snapshots the +descendant set via psutil BEFORE the first signal (children reparent to init +after the parent dies, so a later parent walk finds nothing — same rationale +as agent/deadline.py kill_process_tree) and sweeps any snapshotted survivor +outside the (now-dead) group with SIGKILL afterwards. +""" + +import os +import signal +import textwrap +import time +from types import SimpleNamespace + +import pytest + +from tools.environments.local import LocalEnvironment + + +@pytest.fixture(autouse=True) +def _isolate_hermes_home(tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "logs").mkdir(exist_ok=True) + + +def _pid_alive(pid: int) -> bool: + try: + os.kill(pid, 0) + return True + except ProcessLookupError: + return False + except PermissionError: + return True + + +def _wait_for_pid_exit(pid: int, timeout: float = 10.0) -> bool: + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if not _pid_alive(pid): + return True + time.sleep(0.1) + return not _pid_alive(pid) + + +@pytest.mark.live_system_guard_bypass +def test_timeout_kill_reaps_setsid_grandchild(tmp_path): + """A grandchild that setsid's out of the group must not survive the + timeout kill path.""" + pytest.importorskip("psutil") + + pid_file = tmp_path / "grandchild.pid" + script = textwrap.dedent( + """ + import os, sys, time + pid = os.fork() + if pid == 0: + os.setsid() # escape the command's process group/session + with open(sys.argv[1], "w") as f: + f.write(str(os.getpid())) + time.sleep(30) + os._exit(0) + time.sleep(30) + """ + ).strip() + + env = LocalEnvironment(cwd=str(tmp_path)) + try: + import sys as _sys + + cmd = f"{_sys.executable} -c {_sh_quote(script)} {_sh_quote(str(pid_file))}" + result = env.execute(cmd, timeout=3) + + # The command must have hit the timeout/kill path. + assert "timed out" in result.get("output", "").lower() or result.get( + "returncode" + ) not in (0,), f"expected timeout, got: {result!r}" + + # The grandchild wrote its pid before the kill. + deadline = time.monotonic() + 5.0 + while time.monotonic() < deadline and not pid_file.exists(): + time.sleep(0.05) + assert pid_file.exists(), "grandchild never wrote its pid file" + grandchild_pid = int(pid_file.read_text().strip()) + + assert _wait_for_pid_exit(grandchild_pid), ( + f"setsid grandchild {grandchild_pid} SURVIVED the timeout " + f"group-kill — the #84967/#71148 orphan class (terminal flavor). " + f"_kill_process must sweep snapshotted descendants outside the " + f"group after the group-kill." + ) + finally: + # Belt and braces: never leak the sleeper into the test host. + try: + if pid_file.exists(): + os.kill(int(pid_file.read_text().strip()), signal.SIGKILL) + except (OSError, ValueError): + pass + try: + env.cleanup() + except Exception: + pass + + +def _sh_quote(s: str) -> str: + import shlex + + return shlex.quote(s) + + +def test_kill_process_survives_psutil_snapshot_failure(monkeypatch): + """A broken psutil snapshot must never break the kill path — the + group-kill escalation still runs to completion.""" + psutil = pytest.importorskip("psutil") + + env = object.__new__(LocalEnvironment) + proc = SimpleNamespace( + pid=12345, + _hermes_pgid=67890, + poll=lambda: 0, + wait=lambda timeout=None: 0, + kill=lambda: None, + ) + killpg_calls = [] + + def fake_getpgid(_pid): + return 67890 + + def fake_killpg(pgid, sig): + killpg_calls.append((pgid, sig)) + if sig == 0: + raise ProcessLookupError # group is gone after the first signal + + def boom(*_a, **_k): + raise RuntimeError("psutil exploded") + + monkeypatch.setattr(os, "getpgid", fake_getpgid) + monkeypatch.setattr(os, "killpg", fake_killpg) + monkeypatch.setattr(psutil, "Process", boom) + + env._kill_process(proc) # must not raise + + # SIGTERM was delivered to the group and the alive-probe ran: the + # escalation path completed despite the snapshot failure. + assert killpg_calls[0] == (67890, signal.SIGTERM) + assert (67890, 0) in killpg_calls diff --git a/tools/environments/local.py b/tools/environments/local.py index 07cda5b0e3..90cef0362e 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1907,27 +1907,72 @@ class LocalEnvironment(BaseEnvironment): if pgid is None: raise + # Snapshot the descendant set BEFORE the first signal: once + # the wrapper dies its children reparent to init and a parent + # walk finds nothing (same rationale as agent/deadline.py + # kill_process_tree). A descendant that called ``setsid`` + # escapes the process group entirely and would survive the + # group-kill below — the #71148 class, terminal flavor + # (issue #84967's local sibling). The snapshot must never + # break the kill path, so any failure just yields an empty + # sweep set. + descendants: list = [] + try: + import psutil + + descendants = psutil.Process(proc.pid).children(recursive=True) + except Exception: + descendants = [] + + def _sweep_escaped_descendants() -> None: + """SIGKILL snapshotted survivors outside the (dead) group. + + Runs after the TERM→KILL group escalation so in-group + members keep their SIGTERM grace window; only escapees + (own setsid sessions) are force-killed. psutil's + identity-aware Process means recycled PIDs are skipped. + + POSIX-only: reached solely from the non-_IS_WINDOWS + branch above (the win32 path returns earlier). + """ + for child in descendants: + try: + if not child.is_running(): + continue + try: + if os.getpgid(child.pid) == pgid: + continue # group-kill already covers it + except (ProcessLookupError, PermissionError, OSError): + pass + child.kill() + except Exception: + continue + try: os.killpg(pgid, signal.SIGTERM) # windows-footgun: ok — POSIX process-group SIGTERM (guarded by _IS_WINDOWS above) except ProcessLookupError: + _sweep_escaped_descendants() return # Wait on the process group, not just the shell wrapper. Under # load the wrapper can exit before grandchildren do; returning # at that point leaves orphaned process-group members behind. if _wait_for_group_exit(pgid, 1.0): + _sweep_escaped_descendants() return try: # POSIX-only: _IS_WINDOWS is handled by the outer branch. os.killpg(pgid, signal.SIGKILL) # windows-footgun: ok — POSIX process-group SIGKILL except ProcessLookupError: + _sweep_escaped_descendants() return _wait_for_group_exit(pgid, 2.0) try: proc.wait(timeout=0.2) except (subprocess.TimeoutExpired, OSError): pass + _sweep_escaped_descendants() except (ProcessLookupError, PermissionError, OSError): try: proc.kill()