fix: reap terminal shells on shutdown

This commit is contained in:
Michael Lam
2026-05-19 04:57:07 -07:00
parent 71c70352c1
commit 71d8a8fb1b
3 changed files with 138 additions and 0 deletions
+4
View File
@@ -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)
+31
View File
@@ -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)
+103
View File
@@ -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