hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(update): the Node build stays in the caller's process group and leaves no descendant behind

Browse Source 
What: the POSIX build launcher (_OWN_GROUP -> _REAP_TREE) no longer starts node in a session of
its own. Node stays in the caller's process group; the launcher holds the checkout lock fd and,
once node exits, kills every remaining descendant: on Linux it is a child subreaper
(PR_SET_CHILD_SUBREAPER) and SIGKILLs + reaps its own children until none is left; elsewhere
(macOS) it records node's descendant tree from ps while node runs and kills the recorded
processes whose start time still matches. SIGINT/SIGTERM/SIGHUP are forwarded to node (killed
10 s later if still running), then the descendants are killed the same way.

Why: start_new_session=True moved node out of the caller's group, so every existing group kill
(update_completion's killpg on Ctrl-C, Desktop's kill(-pid), killHandoffTree) reached only the
launcher; node was orphaned and its npm/esbuild descendants kept writing after the checkout lock
was free (review L1, probe review/lock/custody/p1: grandchild wrote after abort, lock free).
The old killpg after p.wait() also signalled a reaped leader's group (pid-reuse window, review
L6): the launcher now signals only its own unreaped children (Linux) or start-time-verified
pids (ps walk), never a group.

Tests (RED on bbfa23b137d, GREEN here): a group kill of the caller-visible group stops node and
its child; a detached (setsid) grandchild outliving node is killed before the build returns, on
the subreaper and the ps-walk paths; SIGTERM to the launcher exits 143 and leaves no writer.
This commit is contained in:
Teknium
2026-10-05 01:43:53 -07:00
parent 8de219355f
commit ab8a00d2be
3 changed files with 208 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
hermes_cli/source_build.py
View File

@@ -48,10 +48,10 @@ def source_build_env(base_env: dict | None = None, *, explicit: bool = False) ->
def run_in_custody(project_root: Path, command: list, label: str, **kwargs):
"""``pm.progress.run_contained`` for a build command that writes the checkout (node, npm):
it and everything it starts stay in the update's custody (POSIX: the checkout lock fd and
its own process group, killed when it exits; Windows: the owner's kill-on-close job), so the
checkout is never handed to a contender while one of them still writes. Outside an update it
is ``run_contained`` as is."""
it and everything it starts stay in the update's custody (POSIX: the checkout lock fd, the
caller's process group, and every descendant killed when it exits; Windows: the owner's
kill-on-close job), so the checkout is never handed to a contender while one of them still
writes. Outside an update it is ``run_contained`` as is."""
from pm.progress import run_contained
from hermes_cli.update_custody import contained_command

141
hermes_cli/update_custody.py
View File

@@ -477,30 +477,133 @@ def _join_launcher_python() -> str:
return sys.executable
# POSIX twin of the launcher: Node marks the donated lock fd close-on-exec, so nothing node
# starts (npm, esbuild, sh) keeps the checkout locked. The command runs as its own process group
# under this stdlib parent, which holds the fd too and, once the command exits (or this parent is
# interrupted), kills the whole group: no descendant outlives the custodial command (N13).
_OWN_GROUP = (
"import os, signal, subprocess, sys\n"
"fds = tuple(int(fd) for fd in sys.argv[1].split(','))\n"
"p = subprocess.Popen(sys.argv[2:], pass_fds=fds, start_new_session=True)\n"
"try:\n"
" code = p.wait()\n"
"finally:\n"
" try:\n"
" os.killpg(p.pid, signal.SIGKILL)\n" # windows-footgun: ok — POSIX-only launcher
" except OSError:\n"
" pass\n"
"sys.exit(code if code >= 0 else 128 - code)\n"
)
# POSIX twin of the launcher (N13, review L1/L6). Node marks the donated lock fd close-on-exec,
# so nothing node starts (npm, esbuild, sh) keeps the checkout locked: custody of them rests on
# this stdlib parent, which holds the fd until no descendant of the command is left.
# * The command stays in the CALLER's process group: every group kill that stops a build (a
# Ctrl-C'd completion child's ``killpg``, Desktop's ``kill(-pid)``) reaches this launcher,
# node and everything under it that did not start a session of its own.
# * Linux: the launcher is a child subreaper, so a descendant orphaned by node's exit (or by any
# intermediate's) is re-parented to it, never to init. Once node exits it SIGKILLs and reaps
# its children until none is left; it only signals its own unreaped children, so no pid it
# kills can have been reused (never a ``killpg`` of an already reaped leader's group).
# * Elsewhere (macOS has no subreaper) it records node's descendant tree from ``ps`` while node
# runs and, once node exits, kills every recorded process whose start time still matches,
# plus what they started. Best effort: a process born and orphaned between two samples escapes.
# * SIGINT/SIGTERM/SIGHUP are forwarded to node (a node still running 10 s later is killed); the
# descendants are then killed as above.
_REAP_TREE = r"""
import os, signal, subprocess, sys, time
KILL = signal.SIGKILL # windows-footgun: ok - POSIX-only launcher
fds = tuple(int(fd) for fd in sys.argv[1].split(','))
me = os.getpid()
reaper = False
if sys.platform.startswith('linux'):
try:
import ctypes
reaper = ctypes.CDLL(None, use_errno=True).prctl(36, 1, 0, 0, 0) == 0 # PR_SET_CHILD_SUBREAPER
except (OSError, AttributeError):
reaper = False
def children():
found = []
for entry in os.listdir('/proc'):
if not entry.isdigit():
continue
try:
with open('/proc/%s/stat' % entry, 'rb') as fh:
stat = fh.read()
if int(stat[stat.rindex(b')') + 2:].split()[1]) == me:
found.append(int(entry))
except (OSError, ValueError, IndexError):
continue
return found
def table():
try:
out = subprocess.run(['ps', '-A', '-o', 'pid=,ppid=,stat=,lstart='], stdin=subprocess.DEVNULL,
capture_output=True, text=True, errors='replace', timeout=10).stdout
except (OSError, subprocess.SubprocessError):
return {}
rows = {}
for line in out.splitlines():
parts = line.split(None, 3)
if len(parts) == 4 and parts[0].isdigit() and parts[1].isdigit() and not parts[2].startswith('Z'):
rows[int(parts[0])] = (int(parts[1]), parts[3].strip())
return rows
seen = {}
def sample(root):
rows = table()
tree = {pid for pid, start in seen.items() if rows.get(pid, (0, None))[1] == start}
if root is not None:
tree.add(root)
grew = True
while grew:
grew = False
for pid, (ppid, start) in rows.items():
if ppid in tree and pid not in tree:
tree.add(pid)
seen[pid] = start
grew = True
return [pid for pid, start in seen.items() if rows.get(pid, (0, None))[1] == start]
def kill(pids):
for pid in pids:
try:
os.kill(pid, KILL)
except OSError:
pass
p = subprocess.Popen(sys.argv[2:], pass_fds=fds)
got = []
def forward(signum, frame):
got.append(signum)
try:
p.send_signal(signum)
except OSError:
pass
for name in ('SIGINT', 'SIGTERM', 'SIGHUP'):
signal.signal(getattr(signal, name), forward)
deadline = None
while True:
try:
code = p.wait(timeout=0.5)
break
except subprocess.TimeoutExpired:
pass
if not reaper:
sample(p.pid)
if got:
deadline = deadline or time.monotonic() + 10
if time.monotonic() > deadline:
p.kill()
end = time.monotonic() + 10
while time.monotonic() < end:
live = children() if reaper else sample(None)
if not live:
break
kill(live)
while reaper:
try:
if not os.waitpid(-1, os.WNOHANG)[0]:
break
except ChildProcessError:
break
time.sleep(0.02)
sys.exit(code if code >= 0 else 128 - code)
"""
@contextlib.contextmanager
def contained_command(argv: Sequence[str], *, inherit_lock: bool = True, root=None):
"""``(argv, kwargs)`` for a checkout writer started by a runner that hides its Popen (the Node
build in ``pm.progress.run_contained``). POSIX: the lock fd (this process's, or one it
inherited for checkout ``root``), with the command's process group killed when it exits.
inherited for checkout ``root``), under a launcher that keeps the command in the caller's
process group and kills every descendant left when it exits (:data:`_REAP_TREE`).
Windows inside an update: the command runs under a launcher that joins the update's
kill-on-close job first; when the job cannot be handed over or the join is refused, the
command never runs and :class:`CustodyRefused` is raised (D2)."""
@@ -510,7 +613,7 @@ def contained_command(argv: Sequence[str], *, inherit_lock: bool = True, root=No
fds = tuple(checkout_lock_fds(root)) if inherit_lock and root is not None else _lock_fds(inherit_lock)
if fds:
argv = [sys.executable, "-I", "-S", "-c", _OWN_GROUP, ",".join(map(str, fds)), *argv]
argv = [sys.executable, "-I", "-S", "-c", _REAP_TREE, ",".join(map(str, fds)), *argv]
yield argv, ({"pass_fds": fds} if fds else {})
return
try:

82
tests/hermes_cli/test_update_custody.py
View File

@@ -362,6 +362,88 @@ def test_no_build_descendant_outlives_the_build_call(repo, tmp_path, monkeypatch
assert not late.exists(), "a build descendant kept writing after the build call returned"
def _late_writer(late: Path, *, detach: bool = False, delay: float = 2.0) -> str:
"""Python source for a build descendant that writes ``late`` after ``delay`` seconds, holding
no inherited fd and no pipe (so nothing but a kill stops it); ``detach`` starts its own session."""
detach_code = "os.setsid(); " if detach else "" # windows-footgun: ok - the callers are POSIX-only
return (f"import os, pathlib, time; {detach_code}os.closerange(3, 4096); "
f"time.sleep({delay}); pathlib.Path({str(late)!r}).touch()")
def _node_starting(writer: str, *, then: str) -> list[str]:
"""A stand-in for node: starts ``writer`` (stdio detached), then runs ``then``."""
return [sys.executable, "-c",
"import subprocess, sys, time; d = subprocess.DEVNULL; "
f"subprocess.Popen([sys.executable, '-c', {writer!r}], stdin=d, stdout=d, stderr=d); {then}"]
def _launch_in_custody(repo: Path, tmp_path: Path, command: list[str], *, platform: str | None = None,
**popen) -> subprocess.Popen:
"""Start ``command`` the way the build runner does (contained_command under a held checkout
lock); ``platform`` makes the launcher see another ``sys.platform`` (its no-subreaper path)."""
from hermes_cli.update_custody import contained_command
lock = ul.UpdateLock(path=tmp_path / "marker", install_root=repo)
assert lock.acquire()
try:
with contained_command(command, root=repo) as (argv, custody):
assert argv[3] == "-c", argv
if platform is not None:
argv[4] = f"import sys; sys.platform = {platform!r}\n" + argv[4]
return subprocess.Popen(argv, stdin=subprocess.DEVNULL, **custody, **popen)
finally:
lock.release()
@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="POSIX process groups + /proc")
def test_a_group_kill_of_the_caller_stops_the_whole_build(repo, tmp_path):
"""L1: the completion child's Ctrl-C path ``killpg``s its own group and Desktop kills the
backend's group; the build (node and what it starts) must be inside that group. A build
moved into a session of its own outlived the kill and kept writing after the lock was free."""
import signal
late_node, late_child = tmp_path / "late-node", tmp_path / "late-child"
node = _node_starting(_late_writer(late_child), then=f"time.sleep(2); __import__('pathlib').Path({str(late_node)!r}).touch()")
# The launcher leads a fresh group here, as the completion child does for its build.
launcher = _launch_in_custody(repo, tmp_path, node, start_new_session=True)
time.sleep(1)
os.killpg(launcher.pid, signal.SIGKILL) # windows-footgun: ok - Linux-only test
launcher.wait(timeout=10)
time.sleep(3)
assert not late_node.exists(), "node survived a group kill of its caller"
assert not late_child.exists(), "a build descendant survived a group kill of its caller"
@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="POSIX sessions + /proc")
@pytest.mark.parametrize("platform", [None, "darwin"], ids=["subreaper", "ps-walk"])
def test_a_detached_grandchild_outliving_node_dies_before_the_build_returns(repo, tmp_path, platform):
"""N13/L1: a descendant that left node's process group (its own session) and outlives node
is still killed before the build call returns: Linux through the subreaper, elsewhere
through the recorded descendant tree."""
late = tmp_path / "late"
node = _node_starting(_late_writer(late, detach=True), then="time.sleep(1.2)")
launcher = _launch_in_custody(repo, tmp_path, node, platform=platform)
assert launcher.wait(timeout=30) == 0
time.sleep(3)
assert not late.exists(), "a detached build descendant kept writing after the build call returned"
@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="POSIX signals + /proc")
def test_a_terminated_launcher_takes_node_and_its_descendants_with_it(repo, tmp_path):
"""L1: SIGTERM to the launcher (a service manager, the caller's polite stop) is forwarded to
node, and what node started is killed too, before the launcher exits."""
import signal
late = tmp_path / "late"
node = _node_starting(_late_writer(late), then="time.sleep(30)")
launcher = _launch_in_custody(repo, tmp_path, node, start_new_session=True)
time.sleep(1)
launcher.send_signal(signal.SIGTERM)
assert launcher.wait(timeout=20) == 128 + signal.SIGTERM
time.sleep(3)
assert not late.exists(), "a build descendant kept writing after its launcher was terminated"
@pytest.mark.skipif(not Path("/proc/self/fd").is_dir(), reason="needs /proc fd listing")
def test_the_desktop_build_runs_in_checkout_custody(repo, tmp_path, monkeypatch):
"""F04: the desktop app build (npm run build / builder) writes the checkout like the other