fix(sessions): never reap an own lease younger than the vouch snapshot
Follow-up on the #101437 salvage (#101415). `_own_live_lease_ids()` snapshots the server's session records under `_sessions_lock`, but the registry file lock is taken afterwards, and a sibling session attaches its lease to its record only after `try_acquire_active_session` has already written the entry. A finalize racing that acquire would read the brand-new lease as an orphan and drop it. Entries this process wrote inside a 30 s grace window are kept regardless of the snapshot; real orphans are minutes old.
This commit is contained in:
@@ -757,6 +757,15 @@ def transfer_active_session(
|
||||
return updated
|
||||
|
||||
|
||||
# A lease this process wrote in the last few seconds may not be in the
|
||||
# caller's ``own_live_lease_ids`` yet: ``try_acquire_active_session`` writes
|
||||
# the registry entry under the file lock and the server attaches the lease to
|
||||
# its session record only after that returns. A concurrent finalize that
|
||||
# snapshotted its live ids in between would otherwise read the brand-new lease
|
||||
# as an orphan and drop it. Real orphans are minutes old (#101415).
|
||||
_SELF_ORPHAN_GRACE_SECONDS = 30.0
|
||||
|
||||
|
||||
def _drop_self_orphans(
|
||||
entries: list[dict[str, Any]], own_live_lease_ids: set[str] | None
|
||||
) -> list[dict[str, Any]]:
|
||||
@@ -764,11 +773,13 @@ def _drop_self_orphans(
|
||||
if own_live_lease_ids is None:
|
||||
return entries
|
||||
pid = os.getpid()
|
||||
cutoff = time.time() - _SELF_ORPHAN_GRACE_SECONDS
|
||||
return [
|
||||
entry
|
||||
for entry in entries
|
||||
if entry.get("pid") != pid
|
||||
or str(entry.get("lease_id") or "") in own_live_lease_ids
|
||||
or (_optional_float(entry.get("started_at")) or 0.0) > cutoff
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -12,6 +12,17 @@ import pytest
|
||||
from hermes_cli import active_sessions
|
||||
|
||||
|
||||
|
||||
def _backdate_leases(*homes, age_seconds=600.0):
|
||||
"""Age every lease in the given registries past the self-orphan grace."""
|
||||
for home in homes:
|
||||
state_path = active_sessions._state_path(home)
|
||||
entries = active_sessions._read_entries(state_path)
|
||||
for entry in entries:
|
||||
entry["started_at"] = time.time() - age_seconds
|
||||
active_sessions._write_entries(state_path, entries)
|
||||
|
||||
|
||||
def test_resolve_max_concurrent_sessions_values(caplog):
|
||||
assert active_sessions.resolve_max_concurrent_sessions({}) is None
|
||||
assert active_sessions.resolve_max_concurrent_sessions({"max_concurrent_sessions": None}) is None
|
||||
@@ -164,6 +175,7 @@ def test_release_orphaned_leases_reclaims_only_unowned_own_pid_entries(tmp_path,
|
||||
+ [{"lease_id": "elsewhere", "session_id": "other", "surface": "cli", "pid": os.getpid() }],
|
||||
)
|
||||
|
||||
_backdate_leases(tmp_path / ".hermes")
|
||||
assert active_sessions.release_orphaned_leases({kept.lease_id, "elsewhere"}) == 1
|
||||
assert sorted(
|
||||
entry["session_id"]
|
||||
@@ -192,6 +204,10 @@ def test_release_orphaned_leases_sweeps_profile_runtime_registries(
|
||||
assert root_lease is not None and root_error is None
|
||||
assert profile_lease is not None and profile_error is None
|
||||
|
||||
# A lease written seconds ago is never an orphan: a sibling finalize that
|
||||
# snapshotted its live ids before this acquire must not reap it (#101415).
|
||||
assert active_sessions.release_orphaned_leases(set()) == 0
|
||||
_backdate_leases(root, profile)
|
||||
assert active_sessions.release_orphaned_leases(set()) == 2
|
||||
assert active_sessions.active_session_registry_snapshot(root) == []
|
||||
assert active_sessions.active_session_registry_snapshot(profile) == []
|
||||
@@ -603,3 +619,30 @@ def test_release_wins_against_transfer_waiting_on_same_lease_lock(
|
||||
assert lease.released is True
|
||||
assert active_sessions.active_session_registry_snapshot() == []
|
||||
|
||||
|
||||
|
||||
def test_liveness_guard_keeps_a_just_acquired_own_lease_it_cannot_vouch_for(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
"""Race in #101415's fix: the finalizing session snapshots its live lease
|
||||
ids, then a sibling session acquires a lease before the registry lock is
|
||||
taken. That lease is absent from the snapshot but is not an orphan."""
|
||||
home = tmp_path / ".hermes"
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
fresh, error = active_sessions.try_acquire_active_session(
|
||||
session_id="fresh", surface="desktop", config={}, registry_home=home
|
||||
)
|
||||
assert fresh is not None and error is None
|
||||
|
||||
with active_sessions.active_session_liveness_guard(
|
||||
"fresh", registry_home=home, own_live_lease_ids=set()
|
||||
) as active:
|
||||
assert active is True
|
||||
assert [e["lease_id"] for e in active_sessions.active_session_registry_snapshot(home)] == [fresh.lease_id]
|
||||
|
||||
_backdate_leases(home)
|
||||
with active_sessions.active_session_liveness_guard(
|
||||
"fresh", registry_home=home, own_live_lease_ids=set()
|
||||
) as active:
|
||||
assert active is False
|
||||
assert active_sessions.active_session_registry_snapshot(home) == []
|
||||
|
||||
@@ -305,6 +305,11 @@ def test_automatic_cleanup_reclaims_own_orphan_lease_not_treated_as_sibling(
|
||||
profile_home=profile_home,
|
||||
)
|
||||
assert owner_lease is not None and message is None
|
||||
# The owner vanished minutes ago; a lease written seconds ago is still
|
||||
# inside the self-orphan grace window and must be left alone.
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.active_sessions._SELF_ORPHAN_GRACE_SECONDS", 0.0
|
||||
)
|
||||
ended: list[tuple[str, str]] = []
|
||||
|
||||
class _FakeDB:
|
||||
|
||||
Reference in New Issue
Block a user