From 5a0b652d7fd8beefc0885e9f6961433704cbea0d Mon Sep 17 00:00:00 2001 From: Devil Date: Wed, 30 Sep 2026 22:52:19 -0400 Subject: [PATCH] fix: treat zombies as dead and kill descendants across sessions on POSIX Found by CI on Ubuntu (reproduced under WSL): - pid_alive used kill(pid, 0), which succeeds for zombies, so a killed child that its parent had not reaped still counted as alive. A long-lived process (the MCP server) that spawns workers would never see them die. Reap our own finished children with a non-blocking waitpid and check /proc for others' zombies. - kill_tree only signalled the worker's process group, but agents run in their own session so a timeout can stop them; they survived a cancel. Snapshot the descendants first (they are re-parented when their parent dies) and signal them and their groups, escalating to SIGKILL. Co-Authored-By: Claude Sonnet 5.5 --- clodex/procs.py | 111 ++++++++++++++++++++++++++++------ tests/test_process_control.py | 6 ++ 2 files changed, 97 insertions(+), 20 deletions(-) diff --git a/clodex/procs.py b/clodex/procs.py index fde82fd..c52be9c 100644 --- a/clodex/procs.py +++ b/clodex/procs.py @@ -10,6 +10,7 @@ import signal import subprocess import time +from pathlib import Path def popen_isolation_kwargs() -> dict[str, object]: @@ -22,15 +23,34 @@ def popen_isolation_kwargs() -> dict[str, object]: def pid_alive(pid: int | None) -> bool: if not pid or pid <= 0: return False + pid = int(pid) if os.name == "nt": - return _windows_pid_alive(int(pid)) + return _windows_pid_alive(pid) + # A finished child stays a zombie until its parent reaps it, and kill(pid, 0) still + # succeeds for zombies. Reap our own children here; check /proc for anyone else's. try: - os.kill(int(pid), 0) + waited, _status = os.waitpid(pid, os.WNOHANG) + except OSError: + waited = 0 # not our child + if waited == pid: + return False + try: + os.kill(pid, 0) except ProcessLookupError: return False except PermissionError: return True - return True + return not _is_zombie(pid) + + +def _is_zombie(pid: int) -> bool: + try: + with open(f"/proc/{pid}/stat", "rb") as handle: + data = handle.read() + except OSError: + return False # no /proc (e.g. macOS): fall back to "alive" + # The state letter follows the last ")" because the command name may contain anything. + return data[data.rfind(b")") + 2 : data.rfind(b")") + 3] == b"Z" def _windows_pid_alive(pid: int) -> bool: @@ -55,33 +75,84 @@ def _windows_pid_alive(pid: int) -> bool: kernel32.CloseHandle(handle) +def _parent_map() -> dict[int, list[int]]: + """parent pid -> child pids, from /proc where available, else `ps`.""" + children: dict[int, list[int]] = {} + proc = Path("/proc") + if proc.is_dir(): + for entry in proc.iterdir(): + if not entry.name.isdigit(): + continue + try: + data = (entry / "stat").read_bytes() + ppid = int(data[data.rfind(b")") + 2 :].split()[1]) + except (OSError, ValueError, IndexError): + continue + children.setdefault(ppid, []).append(int(entry.name)) + if children: + return children + try: + out = subprocess.run(["ps", "-A", "-o", "pid=,ppid="], capture_output=True, text=True, check=False).stdout + except OSError: + return children + for line in out.splitlines(): + parts = line.split() + if len(parts) == 2 and parts[0].isdigit() and parts[1].isdigit(): + children.setdefault(int(parts[1]), []).append(int(parts[0])) + return children + + +def descendants(pid: int) -> list[int]: + children = _parent_map() + found: list[int] = [] + stack = [pid] + while stack: + for child in children.get(stack.pop(), []): + found.append(child) + stack.append(child) + return found + + def kill_tree(pid: int, grace: float = 3.0) -> None: - """Stop `pid` and everything it started. Best effort; never raises.""" + """Stop `pid` and everything it started. Best effort; never raises. + + A child may run in its own session (agents do, so a timeout can stop them), so the + process group alone is not enough: snapshot the descendants first, because they are + re-parented once their parent dies. + """ if not pid or pid <= 0 or pid == os.getpid(): return if os.name == "nt": subprocess.run(["taskkill", "/PID", str(pid), "/T", "/F"], capture_output=True, check=False) return - try: - group = os.getpgid(pid) - except OSError: - return - if group == os.getpgrp(): - # Not isolated (never put it in its own group): only stop that one process. + victims = [pid, *descendants(pid)] + groups: set[int] = set() + for victim in victims: try: - os.kill(pid, signal.SIGTERM) + group = os.getpgid(victim) except OSError: - pass - return - try: - os.killpg(group, signal.SIGTERM) - except OSError: - return + continue + if group != os.getpgrp(): # never signal our own group + groups.add(group) + _signal_all(victims, groups, signal.SIGTERM) deadline = time.monotonic() + grace - while time.monotonic() < deadline and pid_alive(pid): + while time.monotonic() < deadline and any(pid_alive(victim) for victim in victims): time.sleep(0.05) - if pid_alive(pid): + survivors = [victim for victim in victims if pid_alive(victim)] + if survivors: + _signal_all(survivors, groups, signal.SIGKILL) + + +def _signal_all(pids: list[int], groups: set[int], sig: int) -> None: + for group in groups: + try: + os.killpg(group, sig) + except OSError: + pass + for victim in pids: + if victim == os.getpid(): + continue try: - os.killpg(group, signal.SIGKILL) + os.kill(victim, sig) except OSError: pass diff --git a/tests/test_process_control.py b/tests/test_process_control.py index 283b04b..fe9bcdb 100644 --- a/tests/test_process_control.py +++ b/tests/test_process_control.py @@ -49,6 +49,12 @@ def test_pid_alive(self): self.assertFalse(pid_alive(0)) self.assertFalse(pid_alive(finished_pid())) + def test_exited_but_unreaped_child_is_not_alive(self): + # On POSIX an exited child stays a zombie until its parent waits; it must not count as alive. + child = subprocess.Popen([sys.executable, "-c", "pass"]) + self.addCleanup(child.wait) + self.assertTrue(wait_for(lambda: not pid_alive(child.pid), 15), "zombie child reported alive") + def test_kill_tree_stops_the_process_and_its_children(self): child_pid_file = Path(self.id() + ".pid") script = (