fix(lifecycle-ledger): a --replace handover is no longer reported as an unclean death
`detect_unclean_exit` decided "live owner mid-handover" by comparing the
sentinel's `start_time` (the ledger claim, `time.time()` seconds) with
`gateway.status.get_process_start_time(pid)` (proc clock ticks on Linux,
centiseconds elsewhere — its own docstring says it is only comparable with
itself). The two never matched, so every `--replace` takeover whose old owner
was still tearing down read as a crash and was logged/persisted as one.
The sentinel now carries the psutil `create_time` (stamped at claim since
8c3a35b69d), so the guard compares that with the live PID's create time —
same producer, same unit. A pre-stamp sentinel cannot disambiguate PID
reuse; a live PID is taken as the owner, as the psutil-silent case already
was. Test drives all three cases; it fails on the previous ledger.
Found while reviewing the start-attestation follow-ups (#110958).
This commit is contained in:
@@ -104,23 +104,31 @@ def _append_exit_diag(record: Dict[str, Any], home: Optional[Path]) -> None:
|
||||
logger.debug("Failed to append unclean-exit record", exc_info=True)
|
||||
|
||||
|
||||
def _pid_alive_with_start_time(pid: Any, start_time: Any) -> bool:
|
||||
"""True when ``pid`` is a live process matching ``start_time`` (±2s) — guards the
|
||||
``--replace`` race: a live matching owner mid-teardown is a handover, not a death."""
|
||||
def _pid_is_sentinel_owner(pid: Any, create_time: Any) -> bool:
|
||||
"""True when ``pid`` is a live process that is the sentinel's incarnation — guards the
|
||||
``--replace`` race: a live matching owner mid-teardown is a handover, not a death.
|
||||
|
||||
Identity is the psutil ``create_time`` the sentinel stamps at claim (epoch seconds, same
|
||||
producer on both sides). The sentinel's ``start_time`` is the ledger claim time and is NOT comparable with
|
||||
``gateway.status.get_process_start_time`` (proc ticks on Linux, centiseconds elsewhere) — the
|
||||
old comparison never matched, so every ``--replace`` handover read as an unclean death. A
|
||||
sentinel without ``create_time`` (pre-stamp gateway) cannot disambiguate PID reuse; a live PID
|
||||
is taken as the owner, matching the psutil-silent case below."""
|
||||
try:
|
||||
pid_int = int(pid)
|
||||
# NOT os.kill(pid, 0): on Windows that sends CTRL_C_EVENT to the target's console group.
|
||||
from gateway.status import _pid_exists, get_process_start_time
|
||||
from gateway.status import _pid_exists
|
||||
|
||||
if pid_int <= 0 or not _pid_exists(pid_int):
|
||||
return False
|
||||
except Exception:
|
||||
return False
|
||||
try:
|
||||
actual = None if start_time is None else get_process_start_time(pid_int)
|
||||
return actual is None or abs(float(actual) - float(start_time)) <= 2.0 # None: can't disambiguate PID reuse
|
||||
except Exception:
|
||||
if type(create_time) not in (int, float):
|
||||
return True
|
||||
from hermes_cli.process_identity import _process_create_time
|
||||
|
||||
actual = _process_create_time(pid_int)
|
||||
return actual is None or abs(actual - float(create_time)) <= 2.0 # None: can't disambiguate PID reuse
|
||||
|
||||
|
||||
def _suspected_oom(mem: Dict[str, Any]) -> bool:
|
||||
@@ -141,7 +149,7 @@ def detect_unclean_exit(home: Optional[Path] = None) -> Optional[Dict[str, Any]]
|
||||
sentinel = _read_json(get_lifecycle_sentinel_path(home))
|
||||
if not sentinel or sentinel.get("phase") != "running":
|
||||
return None
|
||||
if _pid_alive_with_start_time(sentinel.get("pid"), sentinel.get("start_time")):
|
||||
if _pid_is_sentinel_owner(sentinel.get("pid"), sentinel.get("create_time")):
|
||||
return None # live owner — planned takeover in flight, not a death
|
||||
evidence: Dict[str, Any] = {
|
||||
"prior_pid": sentinel.get("pid"), "prior_started_at": sentinel.get("started_at"),
|
||||
|
||||
@@ -228,3 +228,25 @@ def test_sentinel_carries_process_birth_through_exit(tmp_path: Path, monkeypatch
|
||||
exited = json.loads(get_lifecycle_sentinel_path(tmp_path).read_text(encoding="utf-8"))
|
||||
assert exited["phase"] == "exited"
|
||||
assert (exited["start_time"], exited["create_time"]) == (running["start_time"], 1234.5)
|
||||
|
||||
|
||||
def test_replace_handover_is_not_a_death_and_pid_reuse_is(tmp_path: Path, monkeypatch) -> None:
|
||||
"""The live-owner guard compares the sentinel's psutil ``create_time`` with the live PID's
|
||||
(same producer, epoch seconds). The ledger's ``start_time`` (claim time) is never compared
|
||||
with ``get_process_start_time`` (proc ticks / centiseconds): that comparison could not match,
|
||||
so a ``--replace`` handover was reported as an unclean death."""
|
||||
from gateway import lifecycle_ledger
|
||||
|
||||
monkeypatch.setattr(lifecycle_ledger, "_pid_exists", lambda pid: True, raising=False)
|
||||
monkeypatch.setattr("gateway.status._pid_exists", lambda pid: True)
|
||||
monkeypatch.setattr("hermes_cli.process_identity._process_create_time", lambda pid=None: 5000.0)
|
||||
live = {"phase": "running", "pid": 4242, "start_time": 5003.7, "started_at": "x"}
|
||||
|
||||
_write_sentinel(tmp_path, {**live, "create_time": 5000.0})
|
||||
assert detect_unclean_exit(home=tmp_path) is None # same incarnation still alive: handover
|
||||
|
||||
_write_sentinel(tmp_path, {**live, "create_time": 4000.0})
|
||||
assert detect_unclean_exit(home=tmp_path) is not None # PID reused by another process: death
|
||||
|
||||
_write_sentinel(tmp_path, live) # pre-stamp sentinel: a live PID is taken as the owner
|
||||
assert detect_unclean_exit(home=tmp_path) is None
|
||||
|
||||
Reference in New Issue
Block a user