diff --git a/tests/tools/test_local_setsid_descendant_sweep.py b/tests/tools/test_local_setsid_descendant_sweep.py index 8ae35d68f7..56157611f8 100644 --- a/tests/tools/test_local_setsid_descendant_sweep.py +++ b/tests/tools/test_local_setsid_descendant_sweep.py @@ -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] diff --git a/tools/environments/local.py b/tools/environments/local.py index d944f66b75..3cdeb36f28 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -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: