fix(tools): tear down a child in the caller's own process group by PID, never killpg it
A spawner that skipped setsid (the Darwin gateway's posix_spawn shim) leaves tool children in the gateway's process group, and _kill_process_group_posix then killpg()s the gateway itself — launchd logs 'Killed: 9' and KeepAlive respawns it. Branch on pgid == os.getpgrp() as a plain if/else (not a raised PermissionError routed into the EPERM handler) and signal the wrapper plus its snapshotted descendants by PID; the EPERM fallback shares that helper. Fixes #107029
This commit is contained in:
@@ -146,3 +146,29 @@ def test_kill_process_survives_psutil_snapshot_failure(monkeypatch):
|
||||
# escalation path completed despite the snapshot failure.
|
||||
assert killpg_calls[0] == (67890, signal.SIGTERM)
|
||||
assert (67890, 0) in killpg_calls
|
||||
|
||||
|
||||
def test_kill_process_never_killpgs_the_callers_own_group(monkeypatch):
|
||||
"""A spawner that skipped ``setsid`` leaves the child in OUR process group (the Darwin
|
||||
gateway's posix_spawn shim, #107029): ``killpg`` there would SIGKILL the gateway itself.
|
||||
Teardown must go by PID and never signal the group."""
|
||||
pytest.importorskip("psutil")
|
||||
|
||||
env = object.__new__(LocalEnvironment)
|
||||
killed = []
|
||||
proc = SimpleNamespace(
|
||||
pid=12345,
|
||||
poll=lambda: 0,
|
||||
wait=lambda timeout=None: 0,
|
||||
kill=lambda: killed.append(12345),
|
||||
)
|
||||
killpg_calls = []
|
||||
|
||||
monkeypatch.setattr(os, "getpgid", lambda _pid: 67890)
|
||||
monkeypatch.setattr(os, "getpgrp", lambda: 67890)
|
||||
monkeypatch.setattr(os, "killpg", lambda pgid, sig: killpg_calls.append((pgid, sig)))
|
||||
|
||||
env._kill_process(proc)
|
||||
|
||||
assert killpg_calls == []
|
||||
assert killed == [12345]
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
"""Local execution environment — spawn-per-call with session snapshot."""
|
||||
|
||||
import contextlib
|
||||
import errno
|
||||
import logging
|
||||
import ntpath
|
||||
import os
|
||||
@@ -792,31 +791,37 @@ def _kill_process_group_posix(proc) -> None:
|
||||
descendants = psutil.Process(proc.pid).children(recursive=True)
|
||||
except Exception:
|
||||
descendants = []
|
||||
try:
|
||||
if pgid == os.getpgrp():
|
||||
# The child shares OUR group (a spawner that skipped setsid): killpg would
|
||||
# signal the caller itself — the gateway on Darwin (#107029). Tear down by PID.
|
||||
raise PermissionError(errno.EPERM, "child shares the caller's process group")
|
||||
os.killpg(pgid, signal.SIGTERM) # windows-footgun: ok — POSIX only (see _IS_WINDOWS gate in caller)
|
||||
if not _wait_for_group_exit(proc, pgid, 1.0):
|
||||
os.killpg(pgid, signal.SIGKILL) # windows-footgun: ok — POSIX only (see _IS_WINDOWS gate in caller)
|
||||
_wait_for_group_exit(proc, pgid, 2.0)
|
||||
with contextlib.suppress(subprocess.TimeoutExpired, OSError):
|
||||
proc.wait(timeout=0.2)
|
||||
except ProcessLookupError:
|
||||
pass
|
||||
except PermissionError:
|
||||
# macOS answers killpg with EPERM (not ESRCH) once the group's only members are
|
||||
# unreaped zombies — rg exiting between the caller's poll() and the TERM after the
|
||||
# drain hit its limit (#116855). Nothing group-wide is signalable, and the error
|
||||
# must not escape: the caller still owns the output it drained. Signal the known
|
||||
# PIDs instead so a live child (a group we may not signal) cannot outlive us.
|
||||
for target in (proc, *descendants):
|
||||
with contextlib.suppress(Exception):
|
||||
target.kill()
|
||||
if pgid == os.getpgrp():
|
||||
# The child shares OUR group (a spawner that skipped setsid — the Darwin gateway's
|
||||
# posix_spawn shim, #107029): killpg would signal the caller itself. Tear down by PID.
|
||||
_kill_known_pids(proc, descendants)
|
||||
else:
|
||||
try:
|
||||
os.killpg(pgid, signal.SIGTERM) # windows-footgun: ok — POSIX only (see _IS_WINDOWS gate in caller)
|
||||
if not _wait_for_group_exit(proc, pgid, 1.0):
|
||||
os.killpg(pgid, signal.SIGKILL) # windows-footgun: ok — POSIX only (see _IS_WINDOWS gate in caller)
|
||||
_wait_for_group_exit(proc, pgid, 2.0)
|
||||
with contextlib.suppress(subprocess.TimeoutExpired, OSError):
|
||||
proc.wait(timeout=0.2)
|
||||
except ProcessLookupError:
|
||||
pass
|
||||
except PermissionError:
|
||||
# macOS answers killpg with EPERM (not ESRCH) once the group's only members are
|
||||
# unreaped zombies — rg exiting between the caller's poll() and the TERM after the
|
||||
# drain hit its limit (#116855). Nothing group-wide is signalable, and the error
|
||||
# must not escape: the caller still owns the output it drained. Signal the known
|
||||
# PIDs instead so a live child (a group we may not signal) cannot outlive us.
|
||||
_kill_known_pids(proc, descendants)
|
||||
_sweep_escaped_descendants(descendants, pgid)
|
||||
|
||||
|
||||
def _kill_known_pids(proc, descendants) -> None:
|
||||
"""KILL the wrapper and its snapshotted descendants by PID (idempotent on zombies)."""
|
||||
for target in (proc, *descendants):
|
||||
with contextlib.suppress(Exception):
|
||||
target.kill()
|
||||
|
||||
|
||||
def _kill_process_windows(proc) -> None:
|
||||
"""Identity-checked terminate (start time guards against PID reuse), else kill."""
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user