fix(tools): search_files keeps its matches when killpg is refused
On macOS a search that reaches its `limit` takes the early-stop branch that
TERMs rg's process group; rg has often already exited, and macOS answers
killpg on a zombie-only group with EPERM instead of ESRCH. The
PermissionError escaped _kill_process_group_posix, unwound to search_tool
and the drained matches were replaced by
`{"error": "[Errno 1] Operation not permitted"}` (#116855, same symptom
as #104696). The helper now treats EPERM like "nothing left to signal"
and falls back to killing the known PIDs so a live child cannot escape.
It also never killpg's the caller's own group (#107029): a child that
shares our pgid is torn down by PID.
Diagnosis credit: #116949 (@liuhao1024), redone slim.
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
This commit is contained in:
@@ -112,3 +112,16 @@ def test_kill_switch_routes_search_back_to_the_shell(tree, ops_factory, monkeypa
|
||||
assert result.total_count == 4
|
||||
assert any(c.startswith("test -e") for c in calls)
|
||||
assert any("pipefail" in c and "rg" in c for c in calls)
|
||||
|
||||
|
||||
def test_limit_hit_keeps_drained_matches_when_group_kill_is_refused(tree, ops_factory, monkeypatch):
|
||||
"""Reaching ``limit`` takes the early-stop branch that TERMs rg's group. macOS answers
|
||||
``killpg`` with EPERM (not ESRCH) for a zombie-only group; that must not surface as a tool
|
||||
error nor discard the matches already drained (#116855)."""
|
||||
import os
|
||||
|
||||
monkeypatch.setenv("HERMES_NATIVE_FILE_READ", "1")
|
||||
ops = ops_factory(tree, [])
|
||||
monkeypatch.setattr(os, "killpg", lambda pgid, sig: (_ for _ in ()).throw(PermissionError(1, "Operation not permitted")))
|
||||
result = ops.search(pattern="needle", path=str(tree), limit=2)
|
||||
assert not result.error and len(result.matches) == 2, result.to_dict()
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Local execution environment — spawn-per-call with session snapshot."""
|
||||
|
||||
import contextlib
|
||||
import errno
|
||||
import logging
|
||||
import ntpath
|
||||
import os
|
||||
@@ -792,6 +793,10 @@ def _kill_process_group_posix(proc) -> None:
|
||||
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)
|
||||
@@ -800,6 +805,15 @@ def _kill_process_group_posix(proc) -> None:
|
||||
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()
|
||||
_sweep_escaped_descendants(descendants, pgid)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user