refactor(hermes_cli/update): compact update_lock docstrings and marker parsing
This commit is contained in:
Teknium
+47
-82
@@ -1,12 +1,9 @@
|
||||
"""Cross-process mutual exclusion for in-flight Hermes updates.
|
||||
|
||||
Until now only the Tauri updater published an "update in progress" marker (``UpdateMarkerGuard`` in
|
||||
``apps/bootstrap-installer/src-tauri/src/update.rs``), and only the Electron desktop consumed it
|
||||
(``electron/update-marker.ts``, to gate local backend startup).
|
||||
|
||||
This module makes that same marker the single lock for **all** update entrypoints instead of adding
|
||||
a fourth mechanism. Format and location are unchanged and remain byte-compatible with the Rust and
|
||||
Electron readers:
|
||||
The marker file the Tauri updater writes (``UpdateMarkerGuard`` in
|
||||
``apps/bootstrap-installer/src-tauri/src/update.rs``) and the Electron desktop reads
|
||||
(``electron/update-marker.ts``) is the single lock for **all** update entrypoints.
|
||||
Format and location are byte-compatible with both readers.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -14,42 +11,37 @@ from __future__ import annotations
|
||||
import logging
|
||||
import os
|
||||
import time
|
||||
from contextlib import suppress
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Keep in sync with UPDATE_MARKER_MAX_AGE_MS in
|
||||
# apps/desktop/electron/update-marker.ts — the same marker is read by both, and
|
||||
# a shorter ceiling here would let Python steal a lock Electron still considers
|
||||
# live. A full update (git pull + uv sync + desktop rebuild) is minutes.
|
||||
# Keep in sync with UPDATE_MARKER_MAX_AGE_MS in apps/desktop/electron/update-marker.ts:
|
||||
# a shorter ceiling here would let Python steal a lock Electron still considers live.
|
||||
# A full update (git pull + uv sync + desktop rebuild) is minutes.
|
||||
UPDATE_MARKER_MAX_AGE_SECONDS = 20 * 60
|
||||
|
||||
MARKER_NAME = ".hermes-update-in-progress"
|
||||
|
||||
# Set by an orchestrating updater (the Tauri `hermes-setup --update` flow) to
|
||||
# its own pid before spawning `hermes update` as a child stage. The parent
|
||||
# holds the marker for its whole run, so without this the child refuses its
|
||||
# own parent's lock and the GUI update can never complete. See update_child_env
|
||||
# in apps/bootstrap-installer/src-tauri/src/update.rs — keep the name in sync.
|
||||
# Set by an orchestrating updater (Tauri `hermes-setup --update`) to its own pid before
|
||||
# spawning `hermes update` as a child stage; the parent holds the marker for its whole run,
|
||||
# so without this the child would refuse its own parent's lock. Keep in sync with
|
||||
# update_child_env in apps/bootstrap-installer/src-tauri/src/update.rs.
|
||||
HANDOFF_PID_ENV = "HERMES_UPDATE_HANDOFF_PID"
|
||||
|
||||
# Exit code meaning "another updater/instance owns this install right now".
|
||||
# Already the de-facto contract: the Windows shim + venv-holder guards in
|
||||
# _cmd_update_impl exit 2, and the Tauri updater matches on it
|
||||
# (UPDATE_EXIT_CONCURRENT in apps/bootstrap-installer/src-tauri/src/update.rs)
|
||||
# to show "Hermes is still running" instead of a generic failure. Naming it
|
||||
# here keeps the concurrent-update refusal on that same understood contract.
|
||||
# Exit code meaning "another updater/instance owns this install right now" — the same
|
||||
# contract as the Windows shim / venv-holder guards in _cmd_update_impl, matched by the
|
||||
# Tauri updater (UPDATE_EXIT_CONCURRENT in update.rs) to show "Hermes is still running".
|
||||
UPDATE_EXIT_CONCURRENT = 2
|
||||
|
||||
|
||||
def update_marker_path() -> Path:
|
||||
"""Path of the shared update marker.
|
||||
|
||||
Uses the *process* Hermes home (never the context-local profile override): the Rust updater
|
||||
resolves ``$HERMES_HOME`` or the platform default, and the desktop pins that same value into the
|
||||
updater's env. A profile-scoped path here would put the lock somewhere the other two owners
|
||||
never look.
|
||||
Uses the *process* Hermes home (never the context-local profile override): the Rust
|
||||
updater resolves ``$HERMES_HOME`` or the platform default and the desktop pins that same
|
||||
value into the updater's env, so a profile-scoped path would be one the other owners never look at.
|
||||
"""
|
||||
from hermes_constants import get_process_hermes_home
|
||||
|
||||
@@ -59,12 +51,10 @@ def update_marker_path() -> Path:
|
||||
def _pid_alive(pid: int) -> bool:
|
||||
"""True when a process with ``pid`` currently exists.
|
||||
|
||||
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).
|
||||
|
||||
Any pid we cannot evaluate counts as dead: a corrupt marker must not wedge the lock forever.
|
||||
Delegates to :func:`gateway.status._pid_exists`. Do NOT hand-roll ``os.kill(pid, 0)``: on
|
||||
Windows CPython routes ``sig=0`` to ``GenerateConsoleCtrlEvent``, which Ctrl+C's the
|
||||
target's whole console process group (bpo-14484). Any pid we cannot evaluate counts as
|
||||
dead so a corrupt marker never wedges the lock.
|
||||
"""
|
||||
if pid <= 0:
|
||||
return False
|
||||
@@ -73,37 +63,26 @@ def _pid_alive(pid: int) -> bool:
|
||||
|
||||
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
|
||||
|
||||
|
||||
def _handoff_pid() -> int | None:
|
||||
"""Pid of the orchestrating updater that spawned us, if any.
|
||||
|
||||
Read from :data:`HANDOFF_PID_ENV`. Malformed values count as absent — a broken handoff must fall
|
||||
back to the normal refusal, never crash.
|
||||
"""
|
||||
raw = os.environ.get(HANDOFF_PID_ENV, "").strip()
|
||||
if not raw:
|
||||
return None
|
||||
"""Pid of the orchestrating updater that spawned us (:data:`HANDOFF_PID_ENV`); malformed
|
||||
values count as absent so a broken handoff falls back to the normal refusal."""
|
||||
try:
|
||||
pid = int(raw)
|
||||
pid = int(os.environ.get(HANDOFF_PID_ENV, "").strip())
|
||||
except ValueError:
|
||||
return None
|
||||
return pid if pid > 0 else None
|
||||
|
||||
|
||||
def _is_ancestor_pid(pid: int) -> bool:
|
||||
"""True when ``pid`` is a live ancestor (parent chain) of this process.
|
||||
"""True when ``pid`` is a live ancestor of this process.
|
||||
|
||||
The orchestrating updater spawns ``hermes update`` as a (grand)child, so a live marker owned by
|
||||
one of our ancestors can only be the claim we are already running under — an unrelated
|
||||
concurrent updater is never in our parent chain.
|
||||
|
||||
Never includes our own pid, and any failure counts as "not an ancestor": an unprovable ancestry
|
||||
must fall back to the normal refusal.
|
||||
The orchestrating updater spawns ``hermes update`` as a (grand)child, so a live marker
|
||||
owned by an ancestor can only be the claim we already run under — an unrelated concurrent
|
||||
updater is never in our parent chain. Never our own pid; any failure is "not an ancestor".
|
||||
"""
|
||||
if pid <= 0:
|
||||
return False
|
||||
@@ -128,16 +107,14 @@ def read_live_update(*, path: Path | None = None) -> UpdateHolder | None:
|
||||
"""Return the live update holding the lock, or ``None``.
|
||||
|
||||
Mirrors ``readLiveUpdateMarker`` in ``electron/update-marker.ts``: absent, unreadable,
|
||||
malformed, dead-pid, and past-the-ceiling all mean "no live update", and a stale marker file is
|
||||
deleted so it can't strand future runs. Never raises.
|
||||
malformed, dead-pid, and past-the-ceiling all mean "no live update", and a stale marker
|
||||
file is deleted so it can't strand future runs. Never raises.
|
||||
"""
|
||||
marker = path or update_marker_path()
|
||||
try:
|
||||
raw = marker.read_text(encoding="utf-8")
|
||||
lines = marker.read_text(encoding="utf-8").splitlines()
|
||||
except OSError:
|
||||
return None # absent or unreadable => no live update
|
||||
|
||||
lines = raw.splitlines()
|
||||
return None
|
||||
try:
|
||||
pid = int(lines[0].strip())
|
||||
except (IndexError, ValueError):
|
||||
@@ -149,12 +126,9 @@ def read_live_update(*, path: Path | None = None) -> UpdateHolder | None:
|
||||
|
||||
age = time.time() - started_at
|
||||
if not _pid_alive(pid) or age > UPDATE_MARKER_MAX_AGE_SECONDS:
|
||||
try:
|
||||
with suppress(OSError):
|
||||
marker.unlink()
|
||||
except OSError:
|
||||
pass
|
||||
return None
|
||||
|
||||
return UpdateHolder(pid=pid, age_seconds=age)
|
||||
|
||||
|
||||
@@ -175,10 +149,10 @@ def describe_holder(holder: UpdateHolder) -> str:
|
||||
class UpdateLock:
|
||||
"""Context manager owning the shared update marker for this process.
|
||||
|
||||
``acquired`` is False when another live update holds it; callers decide between hard refusal
|
||||
(CLI/dashboard) and waiting. Release only removes the marker when *we* still own it, so a marker
|
||||
rewritten by a handoff partner (the Tauri updater writes its own pid) is never deleted from
|
||||
under its new owner.
|
||||
``acquired`` is False when another live update holds it; callers decide between hard
|
||||
refusal (CLI/dashboard) and waiting. Release only removes the marker when *we* still own
|
||||
it, so a marker rewritten by a handoff partner (the Tauri updater writes its own pid) is
|
||||
never deleted from under its new owner.
|
||||
"""
|
||||
|
||||
def __init__(self, *, path: Path | None = None) -> None:
|
||||
@@ -189,10 +163,9 @@ class UpdateLock:
|
||||
def acquire(self) -> bool:
|
||||
"""Claim the lock. Returns False (and sets ``holder``) if it's taken.
|
||||
|
||||
A live holder whose pid matches :data:`HANDOFF_PID_ENV` — or is an ancestor of ours — is our
|
||||
own orchestrating parent (e.g. the Tauri updater staging ``hermes update``): run under ITS
|
||||
claim and leave its marker untouched on release. The ancestry path covers staged updaters
|
||||
older than the env-var export.
|
||||
A live holder whose pid matches :data:`HANDOFF_PID_ENV` — or is an ancestor of ours —
|
||||
is our own orchestrating parent: run under ITS claim and leave its marker untouched on
|
||||
release. The ancestry path covers staged updaters older than the env-var export.
|
||||
"""
|
||||
existing = read_live_update(path=self.path)
|
||||
if existing is not None:
|
||||
@@ -202,13 +175,10 @@ class UpdateLock:
|
||||
return False
|
||||
try:
|
||||
self.path.parent.mkdir(parents=True, exist_ok=True)
|
||||
self.path.write_text(
|
||||
f"{os.getpid()}\n{int(time.time())}\n", encoding="utf-8"
|
||||
)
|
||||
self.path.write_text(f"{os.getpid()}\n{int(time.time())}\n", encoding="utf-8")
|
||||
except OSError as exc:
|
||||
# Best-effort, exactly like the Rust guard: an unwritable marker
|
||||
# must not block the update itself (that would be a worse failure
|
||||
# than the race it prevents). Degrade to the pre-lock behavior.
|
||||
# Best-effort, like the Rust guard: an unwritable marker must not block the
|
||||
# update itself (worse than the race it prevents). Degrade to pre-lock behavior.
|
||||
logger.debug("Could not write update marker %s: %s", self.path, exc)
|
||||
return True
|
||||
self.acquired = True
|
||||
@@ -220,18 +190,13 @@ class UpdateLock:
|
||||
return
|
||||
self.acquired = False
|
||||
try:
|
||||
raw = self.path.read_text(encoding="utf-8")
|
||||
owner = int(raw.splitlines()[0].strip())
|
||||
owner = int(self.path.read_text(encoding="utf-8").splitlines()[0].strip())
|
||||
except (OSError, IndexError, ValueError):
|
||||
return
|
||||
if owner != os.getpid():
|
||||
# A handoff partner took ownership (e.g. the Tauri updater wrote
|
||||
# its own pid). Leave it alone — it's still a live update.
|
||||
return
|
||||
try:
|
||||
return # a handoff partner took ownership — still a live update
|
||||
with suppress(OSError):
|
||||
self.path.unlink()
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
def __enter__(self) -> "UpdateLock":
|
||||
self.acquire()
|
||||
|
||||
Reference in New Issue
Block a user