mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 03:00:23 +00:00
fix: reap terminal shells on shutdown
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- **PR #2582** by @Michaelyklam (refs #2577) — Harden embedded workspace-terminal shell cleanup so graceful WebUI shutdowns close/reap every active PTY shell and the spawned shell receives a Linux parent-death signal if the WebUI process dies. The terminal close path now waits again after SIGKILL so timed-out shells do not remain unreaped.
|
||||
|
||||
|
||||
## [v0.51.92] — 2026-05-19 — Release BP (stage-385 — 7-PR full sweep batch — RFC Slice 3c clarification + workspace tree icon alignment + project move cache refresh + auto-compression handoff metadata + Grok OAuth provider catalog + anonymous custom endpoint picker fallback + PWA standalone reload + pull-to-refresh)
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ in the agent execution layer.
|
||||
from __future__ import annotations
|
||||
|
||||
import errno
|
||||
import atexit
|
||||
import codecs
|
||||
import fcntl
|
||||
import os
|
||||
@@ -69,6 +70,20 @@ _TERMINALS: dict[str, TerminalSession] = {}
|
||||
_LOCK = threading.RLock()
|
||||
|
||||
|
||||
def _terminal_shell_preexec_fn() -> None:
|
||||
"""Ask Linux to terminate the PTY shell when the WebUI parent dies."""
|
||||
try:
|
||||
import ctypes
|
||||
|
||||
libc = ctypes.CDLL(None)
|
||||
libc.prctl(1, signal.SIGTERM) # PR_SET_PDEATHSIG=1, SIGTERM=15
|
||||
except Exception:
|
||||
# Non-Linux platforms or restricted runtimes should still be able to
|
||||
# open an embedded terminal; they just do not get the Linux pdeathsig
|
||||
# hardening.
|
||||
pass
|
||||
|
||||
|
||||
def _decode_terminal_output(decoder, data: bytes) -> str:
|
||||
"""Decode PTY bytes without stripping terminal control sequences."""
|
||||
return decoder.decode(data)
|
||||
@@ -178,6 +193,7 @@ def start_terminal(session_id: str, workspace: Path, rows: int = 24, cols: int =
|
||||
stdout=slave_fd,
|
||||
stderr=slave_fd,
|
||||
close_fds=True,
|
||||
preexec_fn=_terminal_shell_preexec_fn,
|
||||
start_new_session=True,
|
||||
)
|
||||
os.close(slave_fd)
|
||||
@@ -240,9 +256,24 @@ def close_terminal(session_id: str) -> bool:
|
||||
os.killpg(term.proc.pid, signal.SIGKILL)
|
||||
except ProcessLookupError:
|
||||
pass
|
||||
try:
|
||||
term.proc.wait(timeout=1.0)
|
||||
except (subprocess.TimeoutExpired, ProcessLookupError):
|
||||
pass
|
||||
finally:
|
||||
try:
|
||||
os.close(term.master_fd)
|
||||
except OSError:
|
||||
pass
|
||||
return True
|
||||
|
||||
|
||||
def close_all_terminals() -> None:
|
||||
"""Best-effort reap of embedded shells during graceful WebUI shutdown."""
|
||||
with _LOCK:
|
||||
session_ids = list(_TERMINALS)
|
||||
for session_id in session_ids:
|
||||
close_terminal(session_id)
|
||||
|
||||
|
||||
atexit.register(close_all_terminals)
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
import subprocess
|
||||
|
||||
import api.terminal as terminal
|
||||
|
||||
|
||||
class _DummyThread:
|
||||
def __init__(self, *args, **kwargs):
|
||||
self.args = args
|
||||
self.kwargs = kwargs
|
||||
self.started = False
|
||||
|
||||
def start(self):
|
||||
self.started = True
|
||||
|
||||
|
||||
class _FakeProc:
|
||||
pid = 999_999_999
|
||||
|
||||
def __init__(self):
|
||||
self.wait_calls = []
|
||||
|
||||
def poll(self):
|
||||
return None
|
||||
|
||||
def wait(self, timeout=None):
|
||||
self.wait_calls.append(timeout)
|
||||
return 0
|
||||
|
||||
|
||||
def test_terminal_shell_uses_parent_death_signal_preexec(monkeypatch, tmp_path):
|
||||
captured = {}
|
||||
proc = _FakeProc()
|
||||
|
||||
def fake_popen(*args, **kwargs):
|
||||
captured["args"] = args
|
||||
captured["kwargs"] = kwargs
|
||||
return proc
|
||||
|
||||
monkeypatch.setattr(terminal.subprocess, "Popen", fake_popen)
|
||||
monkeypatch.setattr(terminal.threading, "Thread", _DummyThread)
|
||||
monkeypatch.setattr(terminal, "_set_size", lambda *args, **kwargs: None)
|
||||
|
||||
term = terminal.start_terminal("term-preexec", tmp_path)
|
||||
|
||||
try:
|
||||
assert term.proc is proc
|
||||
assert captured["kwargs"]["preexec_fn"] is terminal._terminal_shell_preexec_fn
|
||||
assert captured["kwargs"]["start_new_session"] is True
|
||||
assert captured["kwargs"]["stdin"] == captured["kwargs"]["stdout"] == captured["kwargs"]["stderr"]
|
||||
finally:
|
||||
terminal.close_terminal("term-preexec")
|
||||
|
||||
|
||||
def test_close_terminal_waits_again_after_sigkill(monkeypatch):
|
||||
class TimeoutThenReapedProc(_FakeProc):
|
||||
def wait(self, timeout=None):
|
||||
self.wait_calls.append(timeout)
|
||||
if len(self.wait_calls) == 1:
|
||||
raise subprocess.TimeoutExpired(cmd="shell", timeout=timeout)
|
||||
return -9
|
||||
|
||||
proc = TimeoutThenReapedProc()
|
||||
term = terminal.TerminalSession(
|
||||
session_id="term-timeout",
|
||||
workspace="/tmp",
|
||||
proc=proc,
|
||||
master_fd=12345,
|
||||
)
|
||||
terminal._TERMINALS["term-timeout"] = term
|
||||
kills = []
|
||||
monkeypatch.setattr(terminal.os, "killpg", lambda pid, sig: kills.append((pid, sig)))
|
||||
monkeypatch.setattr(terminal.os, "close", lambda fd: None)
|
||||
|
||||
assert terminal.close_terminal("term-timeout") is True
|
||||
|
||||
assert proc.wait_calls == [1.5, 1.0]
|
||||
assert kills == [(proc.pid, terminal.signal.SIGHUP), (proc.pid, terminal.signal.SIGKILL)]
|
||||
|
||||
|
||||
def test_close_all_terminals_closes_snapshot(monkeypatch):
|
||||
terminal._TERMINALS.clear()
|
||||
terminal._TERMINALS.update({"a": object(), "b": object()})
|
||||
closed = []
|
||||
|
||||
def fake_close(session_id):
|
||||
closed.append(session_id)
|
||||
terminal._TERMINALS.pop(session_id, None)
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(terminal, "close_terminal", fake_close)
|
||||
|
||||
terminal.close_all_terminals()
|
||||
|
||||
assert closed == ["a", "b"]
|
||||
assert terminal._TERMINALS == {}
|
||||
|
||||
|
||||
def test_terminal_module_registers_graceful_shutdown_reaper():
|
||||
src = terminal.Path(terminal.__file__).read_text()
|
||||
|
||||
assert "atexit.register(close_all_terminals)" in src
|
||||
assert "preexec_fn=_terminal_shell_preexec_fn" in src
|
||||
assert "libc.prctl(1, signal.SIGTERM)" in src
|
||||
Reference in New Issue
Block a user