mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 13:26:40 +00:00
fix(tests): the test runner kills detached processes a test leaves behind
`_kill_tree` SIGKILLs the per-file pytest process group, which misses any child that left the group with `start_new_session=True` (an auto-started `gateway run`, a setsid server). Those are reparented to init and run forever: one host accumulated 215 orphan gateways (~52 GB RSS, up to 21 h old), and every `hermes update` listed each as "another Hermes home — left alone". After every attempt the runner now SIGKILLs any process whose environment still points into that attempt's unique temp root (PYTEST_DEBUG_TEMPROOT / TMPDIR, or a HOME / HERMES_HOME a fixture created under it). The root is unique per attempt, so the match names only that attempt's processes. Linux-only (/proc); the process-group kill stays the fallback elsewhere.
This commit is contained in:
1 parent
ffcd3621a9
commit
3b0dc776b6
2 files changed
+68
No files matched your search
@@ -460,6 +460,36 @@ def _kill_tree(proc: "subprocess.Popen", pgid: int | None = None) -> None:
|
||||
pass
|
||||
|
||||
|
||||
def _kill_detached_leftovers(temproot: str) -> None:
|
||||
"""SIGKILL every process whose environment still points into this attempt's temp root.
|
||||
|
||||
``_kill_tree`` reaches the attempt's process group only. A detached child
|
||||
(``start_new_session=True``: an auto-started ``gateway run``, a ``setsid``
|
||||
server) leaves that group, is reparented to init when its parent exits, and
|
||||
used to run forever: 215 orphan gateways (~52 GB RSS) piled up on one host.
|
||||
Every descendant carries the attempt's unique temproot in its environment
|
||||
(``PYTEST_DEBUG_TEMPROOT``/``TMPDIR``, or a ``HOME``/``HERMES_HOME`` a fixture
|
||||
made under it), so it names exactly this attempt's processes and nothing else.
|
||||
Linux-only (``/proc``); elsewhere the process-group kill is all there is.
|
||||
"""
|
||||
if not os.path.isdir("/proc"):
|
||||
return
|
||||
import signal as _signal
|
||||
|
||||
root = os.fsencode(temproot)
|
||||
for entry in os.listdir("/proc"):
|
||||
if not entry.isdigit() or int(entry) == os.getpid():
|
||||
continue
|
||||
try:
|
||||
with open(f"/proc/{entry}/environ", "rb") as fh:
|
||||
values = [item.partition(b"=")[2] for item in fh.read().split(b"\0")]
|
||||
if not any(v == root or v.startswith(root + b"/") for v in values):
|
||||
continue
|
||||
os.kill(int(entry), _signal.SIGKILL) # windows-footgun: ok — /proc exists only on Linux
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _effective_file_timeout(
|
||||
file: Path,
|
||||
repo_root: Path,
|
||||
@@ -669,6 +699,7 @@ def _run_one_file_once(
|
||||
|
||||
output += "\n"
|
||||
finally:
|
||||
_kill_detached_leftovers(temproot)
|
||||
# Delete the temp root for this attempt. Nothing reads it after the
|
||||
# subprocess exits. More than 3000 of them fill the disk of the
|
||||
# runner over one suite. Permission fixtures leave read-only dirs
|
||||
|
||||
@@ -234,6 +234,43 @@ def test_grandchild_leak_is_killed_by_runner(tmp_path: Path) -> None:
|
||||
)
|
||||
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
@pytest.mark.live_system_guard_bypass
|
||||
def test_detached_grandchild_is_killed_by_runner(tmp_path: Path) -> None:
|
||||
"""A child in its OWN session (an auto-started ``gateway run``) escapes the process-group
|
||||
kill; the runner still finds it by the attempt's temp root in its environment."""
|
||||
repo_root = _probe_root(tmp_path)
|
||||
probe_dir = tmp_path / "probe"
|
||||
probe_dir.mkdir()
|
||||
handoff = tmp_path / "detached.json"
|
||||
(probe_dir / "test_probe_detached.py").write_text(textwrap.dedent(f"""
|
||||
import json, os, subprocess, sys
|
||||
from pathlib import Path
|
||||
|
||||
def test_spawns_detached_child_and_walks_away():
|
||||
child = subprocess.Popen(
|
||||
[sys.executable, "-c",
|
||||
"import signal, time; signal.signal(signal.SIGTERM, signal.SIG_IGN); time.sleep(600)"],
|
||||
start_new_session=True,
|
||||
)
|
||||
Path({str(handoff)!r}).write_text(json.dumps({{"pid": child.pid}}), encoding="utf-8")
|
||||
""").lstrip(), encoding="utf-8")
|
||||
proc = subprocess.run(
|
||||
[sys.executable, str(repo_root / "scripts" / "run_tests_parallel.py"),
|
||||
"--paths", str(probe_dir), "-j", "1", "--file-timeout", "30"],
|
||||
cwd=probe_dir, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
|
||||
encoding="utf-8", errors="replace", timeout=60,
|
||||
)
|
||||
pid = json.loads(handoff.read_text(encoding="utf-8-sig"))["pid"]
|
||||
deadline = time.monotonic() + 5.0
|
||||
while _pid_alive(pid) and time.monotonic() < deadline:
|
||||
time.sleep(0.05)
|
||||
if _pid_alive(pid):
|
||||
os.kill(pid, 9)
|
||||
pytest.fail(f"detached child {pid} survived the runner; output:\n{proc.stdout}")
|
||||
assert proc.returncode == 0, proc.stdout
|
||||
|
||||
# ── Bare pytest-flag passthrough ─────────────────────────────────────────────
|
||||
#
|
||||
# The runner routes any token starting with ``-`` that isn't one of its own
|
||||
|
||||
Reference in new issue
Block a user