hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(dashboard): bound the PTY helper grace and await background closes

Browse Source 
- Cap the helper grace at 2.0 s (and ignore non-finite or <= 0 env
  values) and wait only 0.5 s after SIGKILL, so a raised
  HERMES_TUI_GATEWAY_SHUTDOWN_GRACE_S cannot push close_all() past the
  backend SIGTERM -> SIGKILL budget.
- close_all() also awaits closes already running in the background
  (dead remnants, capacity evictions); the registry now holds those
  tasks, which also keeps them from being garbage-collected.
- _discard_output sleeps on EOF instead of spinning on a readable dead
  master, and catches ValueError from select() like read() does.
This commit is contained in:
kshitijk4poor
2026-10-02 14:27:56 +05:30
committed by kshitij
parent 9236c9c63d
commit 569fd58960
4 changed files with 44 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
hermes_cli/dashboard_procs.py
View File

@@ -451,7 +451,7 @@ def _kill_pids_windows(pids: list[int], killed: list[int], failed: list[tuple[in
# SIGTERM → SIGKILL grace for the dashboard/serve backend. Must outlast the lifespan teardown in
# hermes_cli/web_server.py::_lifespan: stop_hosted_room_service(timeout=5.0) + the startup-thread
# join(1.0) + PTY_REGISTRY.close_all() (concurrent; ≤ ~4.5s with the default helper grace). A SIGKILL inside
# join(1.0) + PTY_REGISTRY.close_all() (concurrent; ≤ ~4s per PTY, see pty_bridge._MAX_HELPER_SHUTDOWN_GRACE_S). A SIGKILL inside
# that window skips close_all(), so the ui-tui / tui_gateway.entry children outlive the backend
# and keep the deleted state.db-wal inode open — the next hermes start refuses with a FATAL
# DeletedWalGenerationError (#111912). The orphan reaper's 1.5s (`_reap_orphaned_desktop_local_serves`)

24
hermes_cli/pty_bridge.py
View File

@@ -11,6 +11,7 @@ from __future__ import annotations
import asyncio
import errno
import fcntl # windows-footgun: ok — POSIX-only module by design (see docstring)
import math
import os
import select
import signal
@@ -64,6 +65,9 @@ class PtyUnavailableError(RuntimeError):
# Longer than the TUI gateway's own SIGHUP shutdown grace, so a helper that is saving state on
# SIGHUP finishes before it is SIGKILLed.
_HELPER_SHUTDOWN_GRACE_S = 1.5
# Upper bound: PTY_REGISTRY.close_all() runs inside the backend's SIGTERM -> SIGKILL budget
# (dashboard_procs._POSIX_TERM_GRACE_SECONDS), so a raised gateway grace cannot stretch it.
_MAX_HELPER_SHUTDOWN_GRACE_S = 2.0
def _helper_shutdown_grace() -> float:
@@ -72,7 +76,10 @@ def _helper_shutdown_grace() -> float:
# tui_gateway.entry, which installs signal handlers.
from utils import env_float
return max(_HELPER_SHUTDOWN_GRACE_S, env_float("HERMES_TUI_GATEWAY_SHUTDOWN_GRACE_S", 1.0) + 0.5)
gateway = env_float("HERMES_TUI_GATEWAY_SHUTDOWN_GRACE_S", 1.0)
if not math.isfinite(gateway) or gateway <= 0:
gateway = 1.0
return min(max(_HELPER_SHUTDOWN_GRACE_S, gateway + 0.5), _MAX_HELPER_SHUTDOWN_GRACE_S)
def _process_group_exists(pgid: int) -> bool:
@@ -83,8 +90,6 @@ def _process_group_exists(pgid: int) -> bool:
return True
class PtyBridge:
"""Thin wrapper around ``ptyprocess.PtyProcess`` for byte streaming. Not thread-safe: owned by
the WebSocket handler that spawned it; reads run in an executor thread, writes are awaited on
@@ -253,13 +258,14 @@ class PtyBridge:
def _discard_output(self, timeout: float) -> None:
# Exiting children flush their pending terminal output first; on macOS a process stays in
# exit (and a SIGHUP save outlasts the grace) until someone drains the master. The drain
# task has stopped reading by now, so read and drop while waiting.
# task no longer consumes the output, so read and drop while waiting.
try:
readable, _, _ = select.select([self._fd], [], [], timeout)
if readable:
os.read(self._fd, 65536)
except OSError:
time.sleep(timeout)
if readable and os.read(self._fd, 65536):
return
except (OSError, ValueError):
pass
time.sleep(timeout) # nothing read, EOF (b"") or EIO: don't spin on a readable dead fd
def _wait_for_group_exit(self, pgid: int, timeout: float) -> None:
deadline = time.monotonic() + timeout
@@ -312,7 +318,7 @@ class PtyBridge:
os.killpg(pgid, sig) # windows-footgun: ok — POSIX-only module (imports fcntl/termios/ptyprocess at top)
except OSError:
break # ESRCH: the group is empty
self._wait_for_group_exit(pgid, grace)
self._wait_for_group_exit(pgid, grace if sig == signal.SIGHUP else 0.5) # windows-footgun: ok — POSIX-only module (imports fcntl/termios/ptyprocess at top)
try:
self._proc.close(force=True)

14
hermes_cli/pty_session.py
View File

@@ -183,6 +183,9 @@ class PtySessionRegistry:
# already serialized globally for the same reason, and a spawn only
# delays NEW chats. Per-key locks if spawn throughput ever matters.
self._attach_lock = asyncio.Lock()
# Sessions popped from the registry but still closing in the background; close_all()
# awaits them too, and holding the tasks keeps them from being garbage-collected.
self._background_closes: set[asyncio.Task] = set()
async def attach_or_spawn(self, key: str, *, spawn: Callable[[], object]) -> Tuple[PtySession, bool]:
await self.reap_idle()
@@ -194,7 +197,7 @@ class PtySessionRegistry:
# Close in the background: ending a dead leader's helpers can take the helper
# grace, and this lock serializes every new chat.
self._sessions.pop(key, None)
asyncio.create_task(existing.close())
self._close_in_background(existing)
if len(self._sessions) >= self._max:
self._reap_one_idle_or_raise()
# PTY spawn does blocking fork/exec work — keep it off the event loop.
@@ -256,10 +259,15 @@ class PtySessionRegistry:
raise RegistryFull()
oldest = min(idle, key=lambda s: s.last_detached_at or 0.0)
self._sessions.pop(oldest.key, None)
asyncio.create_task(oldest.close())
self._close_in_background(oldest)
def _close_in_background(self, session: "PtySession") -> None:
task = asyncio.create_task(session.close())
self._background_closes.add(task)
task.add_done_callback(self._background_closes.discard)
async def close_all(self) -> None:
# Close concurrently: each close() may wait out its helpers' SIGHUP grace, and shutdown runs
# under the backend's SIGTERM -> SIGKILL budget (dashboard_procs._POSIX_TERM_GRACE_SECONDS).
sessions = [self._sessions.pop(key) for key in list(self._sessions)]
await asyncio.gather(*(s.close() for s in sessions), return_exceptions=True)
await asyncio.gather(*(s.close() for s in sessions), *self._background_closes, return_exceptions=True)

17
tests/hermes_cli/test_pty_session.py
View File

@@ -452,6 +452,23 @@ async def test_close_all_does_not_wait_on_one_slow_close_before_the_next():
assert all(b.closed for b in bridges)
@pytest.mark.asyncio
async def test_close_all_waits_for_closes_already_running_in_the_background():
"""A dead remnant or evicted session leaves the registry before its close() finishes; the
backend's teardown must still wait for it, or its helpers outlive the backend."""
reg = make_registry(ttl=60.0)
bridges, entered, release = await _two_idle_sessions_first_close_gated(reg)
reg._close_in_background(reg._sessions.pop("k0"))
await entered.wait() # k0 is closing outside the registry
closer = asyncio.create_task(reg.close_all())
await asyncio.sleep(0.05)
assert not closer.done() # still waiting on k0
release.set()
await closer
assert all(b.closed for b in bridges)
@pytest.mark.asyncio
async def test_close_other_sessions_removes_old_profile_session():
from hermes_cli.pty_session import WS_CLOSE_SUPERSEDED, PtySession