fix(tui-gateway): guard entry signal installs to main thread
Importing tui_gateway.entry from a worker thread raised 'ValueError: signal only works in main thread of the main interpreter' because signal.signal() was called unconditionally at import time. On the Desktop/WebSocket path, server._build() runs in a daemon thread and does 'from tui_gateway.entry import ensure_mcp_discovery_started' as the first import of entry (entry.main() is never run there), which crashed and aborted MCP discovery startup — every session then lost its MCP servers (e.g. Dart-mcp) with ClosedResourceError. signal handlers are process-global, so installing them only when the module is first imported in the main thread is sufficient; importing from a worker thread becomes a safe no-op. Fixes #72667
This commit is contained in:
parent
4b4d2ae4cd
commit
8b11249755
|
|
@ -0,0 +1,85 @@
|
|||
"""Regression test: importing tui_gateway.entry off the main thread must not crash.
|
||||
|
||||
Background
|
||||
----------
|
||||
``entry.py`` installs signal handlers (SIGPIPE, SIGTERM, …) at *import time*.
|
||||
``signal.signal`` is only legal in the main thread, so a first import of
|
||||
``entry`` from a worker thread raises
|
||||
``ValueError: signal only works in main thread of the main interpreter``.
|
||||
|
||||
The Desktop/WebSocket agent-build path (``server._build``) runs in a daemon
|
||||
thread and does ``from tui_gateway.entry import ensure_mcp_discovery_started``
|
||||
(server.py:1866). On that path ``entry.main()`` is never run, so the worker
|
||||
thread performs the *first* import of ``entry`` — which crashed and aborted
|
||||
MCP discovery startup (#72667, regression from the 2026-07-26 websocket-MCP
|
||||
discovery fix).
|
||||
|
||||
Fix: guard the import-time signal installs with a main-thread check. Handlers
|
||||
are process-global, so installing them only when reached in the main thread is
|
||||
sufficient; importing from a worker thread becomes a safe no-op.
|
||||
|
||||
This test runs the import in a *fresh* subprocess worker thread to guarantee a
|
||||
clean first-import (the test process already imported entry in its main
|
||||
thread, so an in-process import would not reproduce the bug).
|
||||
"""
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
REPO_ROOT = "."
|
||||
|
||||
|
||||
def _spawn_worker_import_entry():
|
||||
"""Run `import tui_gateway.entry` for the first time inside a worker thread.
|
||||
|
||||
Returns (returncode, stdout, stderr).
|
||||
"""
|
||||
code = (
|
||||
"import threading, sys, signal, os\n"
|
||||
"sys.stdout.reconfigure(line_buffering=True)\n"
|
||||
"sys.stderr.reconfigure(line_buffering=True)\n"
|
||||
"errs = []\n"
|
||||
"def _worker():\n"
|
||||
" try:\n"
|
||||
" import tui_gateway.entry\n"
|
||||
" except Exception as e:\n"
|
||||
" errs.append(repr(e))\n"
|
||||
"t = threading.Thread(target=_worker, daemon=True)\n"
|
||||
"t.start(); t.join(timeout=15)\n"
|
||||
"if errs:\n"
|
||||
" sys.stdout.write('IMPORT_FAILED: ' + errs[0] + '\\n')\n"
|
||||
" sys.exit(2)\n"
|
||||
"# main thread of this process still installs SIGPIPE handler\n"
|
||||
"h = signal.getsignal(signal.SIGPIPE)\n"
|
||||
"sys.stdout.write('OK handler_installed=' + str(h is signal.SIG_IGN or callable(h)) + '\\n')\n"
|
||||
"sys.exit(0)\n"
|
||||
)
|
||||
proc = subprocess.run(
|
||||
[sys.executable, "-c", code],
|
||||
cwd=REPO_ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=60,
|
||||
env={**__import__("os").environ},
|
||||
)
|
||||
return proc.returncode, proc.stdout, proc.stderr
|
||||
|
||||
|
||||
def test_entry_imports_cleanly_from_worker_thread():
|
||||
"""First import of tui_gateway.entry from a worker thread must succeed."""
|
||||
rc, out, err = _spawn_worker_import_entry()
|
||||
# entry import may emit to stdout or stderr depending on the runner; check both.
|
||||
combined = out + err
|
||||
assert rc == 0, f"entry import off main thread failed (rc={rc}): {err!r}"
|
||||
assert "OK" in combined, f"unexpected output: {out!r} / {err!r}"
|
||||
|
||||
|
||||
def test_entry_installs_sigpipe_handler_in_main_thread():
|
||||
"""Even though it can be imported off-thread, the main-thread path still
|
||||
installs the SIGPIPE handler (process-global, so it applies everywhere)."""
|
||||
rc, out, err = _spawn_worker_import_entry()
|
||||
combined = out + err
|
||||
assert rc == 0, f"entry import off main thread failed (rc={rc}): {err!r}"
|
||||
assert "handler_installed=True" in combined, (
|
||||
f"SIGPIPE handler not installed: {out!r} / {err!r}"
|
||||
)
|
||||
|
|
@ -13,6 +13,7 @@ hermes_bootstrap.harden_import_path()
|
|||
import json
|
||||
import logging
|
||||
import signal
|
||||
import threading
|
||||
import time
|
||||
import traceback
|
||||
|
||||
|
|
@ -184,18 +185,49 @@ def _log_signal(signum: int, frame) -> None:
|
|||
# with hasattr so ``python -m tui_gateway.entry`` (spawned by
|
||||
# ``hermes --tui``) imports cleanly there. SIGBREAK (Windows' Ctrl+Break)
|
||||
# is installed when available as a weaker equivalent of SIGHUP.
|
||||
if hasattr(signal, "SIGPIPE"):
|
||||
signal.signal(signal.SIGPIPE, signal.SIG_IGN)
|
||||
if hasattr(signal, "SIGTERM"):
|
||||
signal.signal(signal.SIGTERM, _log_signal)
|
||||
#
|
||||
# signal.signal() is only legal in the MAIN thread. On the Desktop/WebSocket
|
||||
# agent-build path, server._build() runs in a daemon thread and does
|
||||
# ``from tui_gateway.entry import ensure_mcp_discovery_started`` as the first
|
||||
# import of entry (entry.main() is never run there), which used to raise
|
||||
# "ValueError: signal only works in main thread of the main interpreter" and
|
||||
# abort MCP discovery startup. Install each handler only when we're in the
|
||||
# main thread: handlers are process-global, so a main-thread import anywhere
|
||||
# in the process still installs them for everyone, and an off-thread import
|
||||
# (Desktop build path) simply no-ops instead of crashing the import. This
|
||||
# preserves the original SIG_IGN/SIG_DFL behavior on the classic TUI/serve
|
||||
# path while fixing the off-thread import crash.
|
||||
|
||||
|
||||
def _install_signal(signame, handler):
|
||||
"""Install a signal handler if legal in this thread.
|
||||
|
||||
signal.signal() raises ValueError outside the main thread; skip silently
|
||||
there so a worker-thread import of this module (Desktop build path) does
|
||||
not abort. On any main-thread import the handler is installed as before.
|
||||
"""
|
||||
if threading.current_thread() is not threading.main_thread():
|
||||
return
|
||||
sig = getattr(signal, signame, None)
|
||||
if sig is None:
|
||||
return # Windows: SIGPIPE/SIGHUP absent
|
||||
try:
|
||||
signal.signal(sig, handler)
|
||||
except (ValueError, OSError, RuntimeError):
|
||||
# Not in the main thread despite the check, or handler rejected.
|
||||
# Skip rather than crash the import (see above).
|
||||
pass
|
||||
|
||||
|
||||
_install_signal("SIGPIPE", signal.SIG_IGN)
|
||||
_install_signal("SIGTERM", _log_signal)
|
||||
if hasattr(signal, "SIGHUP"):
|
||||
signal.signal(signal.SIGHUP, _log_signal)
|
||||
_install_signal("SIGHUP", _log_signal)
|
||||
elif hasattr(signal, "SIGBREAK"):
|
||||
# Windows-only: Ctrl+Break in a console window delivers SIGBREAK.
|
||||
# Route it through the same handler so kills are diagnosable.
|
||||
signal.signal(signal.SIGBREAK, _log_signal)
|
||||
if hasattr(signal, "SIGINT"):
|
||||
signal.signal(signal.SIGINT, signal.SIG_IGN)
|
||||
_install_signal("SIGBREAK", _log_signal)
|
||||
_install_signal("SIGINT", signal.SIG_IGN)
|
||||
|
||||
|
||||
def _log_exit(reason: str) -> None:
|
||||
|
|
|
|||
Loading…
Reference in New Issue