mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(update): use the no-kill pid probe, not os.kill(pid, 0)
The Windows-footgun linter caught a real bug in the new update lock. On Windows os.kill(pid, 0) is not a no-op: CPython routes sig=0 to GenerateConsoleCtrlEvent, which sends Ctrl+C to the target's entire console process group (bpo-14484). The liveness probe would have killed the very updater it was asking about -- and any sibling sharing that console. Delegate to gateway.status._pid_exists, the project's existing no-kill probe, which uses psutil (OpenProcess/GetExitCodeProcess on Windows) and also reports zombies as dead. Any pid we cannot evaluate still counts as dead so a corrupt marker cannot wedge the lock.
This commit is contained in:
parent
6a444ebec7
commit
37519b4eeb
1 changed files with 15 additions and 17 deletions
|
|
@ -72,29 +72,27 @@ def update_marker_path() -> Path:
|
|||
def _pid_alive(pid: int) -> bool:
|
||||
"""True when a process with ``pid`` currently exists.
|
||||
|
||||
``os.kill(pid, 0)`` is POSIX-only in spirit but CPython implements the
|
||||
signal-0 existence probe on Windows too. ``PermissionError`` means the pid
|
||||
exists but is owned by another user — still alive for our purposes.
|
||||
Delegates to :func:`gateway.status._pid_exists`, the project's existing
|
||||
no-kill probe. Do NOT hand-roll this with ``os.kill(pid, 0)``: on Windows
|
||||
that is not a no-op — CPython routes ``sig=0`` to
|
||||
``GenerateConsoleCtrlEvent``, which Ctrl+C's the target's whole console
|
||||
process group (bpo-14484). A liveness check that killed the updater it was
|
||||
asking about would be a spectacular way to fix a concurrency bug.
|
||||
|
||||
A value too large for the platform's ``pid_t`` raises ``OverflowError``
|
||||
(not ``OSError``), so a corrupt marker would otherwise crash every update
|
||||
that reads it. Treat any unusable pid as dead: the marker is garbage and
|
||||
must not wedge the lock.
|
||||
Any pid we cannot evaluate counts as dead: a corrupt marker must not wedge
|
||||
the lock forever.
|
||||
"""
|
||||
if pid <= 0:
|
||||
return False
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except ProcessLookupError:
|
||||
from gateway.status import _pid_exists
|
||||
|
||||
return bool(_pid_exists(pid))
|
||||
except Exception as exc:
|
||||
# Import failure or an unusable pid (e.g. larger than the platform's
|
||||
# pid_t). Treat the marker as stale rather than blocking updates.
|
||||
logger.debug("Could not probe pid %s: %s", pid, exc)
|
||||
return False
|
||||
except PermissionError:
|
||||
return True
|
||||
except (OverflowError, ValueError):
|
||||
return False
|
||||
except OSError:
|
||||
# Unexpected errno: assume alive rather than stomping a live updater.
|
||||
return True
|
||||
return True
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue