Files
hermes-agent/tools/mcp_tool_lifecycle.py
Austin Pickett 0cd93f0268 fix(mcp): kill Windows stdio MCP orphan trees (#61059)
Windows has no POSIX parent-death supervisor/killpg safety net, so an
ungraceful exit of the hermes process left every stdio MCP child tree
(npx.cmd -> node.exe) running as orphans with ParentId=null, piling up
across session restarts.

- _run_stdio now attaches the process to a KILL_ON_JOB_CLOSE job object
  before spawning stdio children (self-guarded no-op off Windows), so the
  whole child tree dies with the parent at the kernel level.
- Windows reaps kill the process tree (direct child + descendants) in the
  lifecycle orphan sweep and the spawn-ledger startup sweep, where there
  is no pgid to group-kill.

Fixes #61059
2026-09-26 20:58:16 -04:00

370 lines
19 KiB
Python
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

"""MCP process lifecycle: stdio child PID tracking and orphan cleanup, graceful
server shutdown and draining of the background MCP loop."""
import logging
import asyncio
import os
import time
from typing import Dict, Optional
from tools.mcp_tool_common import _core
from tools import mcp_tool_loop as _loop
logger = logging.getLogger("tools.mcp_tool")
# Live stdio MCP children (pid -> server_name), added after connection and removed on normal
# shutdown, so they can be force-killed if SDK teardown fails.
_stdio_pids: Dict[int, str] = {}
# PIDs that survived their session context exit (detected in _run_stdio's finally, reaped by
# _kill_orphaned_mcp_children). Separate from _stdio_pids so sweeps never race active sessions.
_orphan_stdio_pids: set = set()
_orphan_stdio_pid_servers: Dict[int, str] = {}
# pid -> pgid captured at spawn. The SDK spawns with start_new_session=True (PGID == PID);
# grandchildren keep that PGID after the direct child exits, so killpg still reaches them.
# Separate from _stdio_pids so the PGID survives the child's removal. Empty on Windows.
_stdio_pgids: Dict[int, int] = {}
def _snapshot_child_pids() -> set:
"""Current direct-child PIDs: /proc on Linux, else psutil, else empty set."""
my_pid = os.getpid()
# /proc/<pid>/task/<tid>/children is per-THREAD, and stdio_client() spawns from the MCP
# loop thread, so union every task's children — reading only the main thread's file
# returns an empty set on every Linux install.
try:
# ``/proc/<pid>/task/<tid>/children`` is per-THREAD — a child forked from thread T is listed only
# under T's task dir. stdio_client() spawns from the background MCP loop thread, so reading only the
# main thread's file (``task/<pid>/children``) returned an empty set on every Linux install and left
# ``_stdio_child_pids`` / ``_stdio_pids`` empty: the #81995 dead-child fast-fail, the #96452 respawn
# signal, and the killpg shutdown sweep never saw the subprocess.
task_dir = f"/proc/{my_pid}/task"
found: set = set()
for tid in os.listdir(task_dir):
try:
with open(f"{task_dir}/{tid}/children", encoding="utf-8-sig") as f:
found.update(int(p) for p in f.read().split() if p.strip())
except (FileNotFoundError, OSError, ValueError):
continue # thread exited between listdir and open
return found
except (FileNotFoundError, OSError, ValueError):
pass
try:
import psutil
return {c.pid for c in psutil.Process(my_pid).children()}
except Exception:
return set()
# argv markers of non-MCP gateway children that can race into the snapshot delta during an
# MCP spawn (defense-in-depth; LSP/slash_worker already use start_new_session). Matched against
# argv[1:] because Python/Java children start with the interpreter path.
_NON_MCP_CHILD_CMDLINE_MARKERS: tuple[str, ...] = (
"tui_gateway.slash_worker", "tui_gateway.entry",
"-dorg.eclipse.equinox.launcher", "eclipse.jdt.ls", "org.eclipse.equinox.launcher_", # jdtls
)
def _filter_mcp_children(pids: set) -> set:
"""Drop non-MCP children from a PID snapshot delta. Tracking a stray child in _stdio_pgids
is catastrophic if it lacks start_new_session: its pgid can be the TUI parent's, so the
shutdown killpg() would kill the TUI itself."""
if not pids:
return pids
try:
import psutil
except ImportError:
return pids # keep all PIDs (prior behavior)
kept = set()
for pid in pids:
try:
argv = psutil.Process(pid).cmdline()
except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
continue # raced away or zombie — cannot be our fresh server, unsafe to track
if not any(marker in arg for arg in argv[1:] for marker in _NON_MCP_CHILD_CMDLINE_MARKERS):
kept.add(pid)
return kept
def _clear_connect_cooldowns(keys=None) -> None:
"""Drop connect-retry cooldowns: a restart must re-attempt every server immediately, not
honour a stale per-server backoff. Caller holds ``_core._lock``."""
if keys is None:
_core._server_connect_retry_after.clear()
_core._server_connect_failures.clear()
else:
for key in keys:
_core._server_connect_retry_after.pop(key, None)
_core._server_connect_failures.pop(key, None)
def _reregister_orphaned_adopters() -> None:
"""Re-run MCP registration for profiles whose ADOPTED shared connection an owner's
``/reload-mcp`` just tore down. Their tools vanished with the owner's teardown and nothing
re-runs their discovery until THEY reload, so they sat tool-less behind a healthy-looking
status (#106005). Runs after the owner's rediscovery, under each adopter's own home + secret
scope (its ``${VAR}`` refs must resolve to ITS credentials): the adopter re-adopts the owner's
new identical connection or connects its own."""
with _core._lock:
pending = dict(_core._orphaned_adopters)
_core._orphaned_adopters.clear()
if not pending:
return
from pathlib import Path
from agent.secret_scope import build_profile_secret_scope, reset_secret_scope, set_secret_scope
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
from tools import mcp_tool_discovery as _discovery
from tools.mcp_tool_config import _load_mcp_config
for adopter, names in pending.items():
home_token = secret_token = None
try:
home_token = set_hermes_home_override(adopter)
secret_token = set_secret_scope(build_profile_secret_scope(Path(adopter)), profile_home=adopter)
servers = {n: c for n, c in (_load_mcp_config() or {}).items() if n in names}
if servers:
_discovery.register_mcp_servers(servers)
except Exception:
logger.debug("MCP: re-registration for profile scope %s failed", adopter, exc_info=True)
finally:
if secret_token is not None:
reset_secret_scope(secret_token)
if home_token is not None:
reset_hermes_home_override(home_token)
def shutdown_mcp_servers(*, scope: Optional[str] = None, names: Optional[set] = None,
timeout: float = 15.0):
"""Close MCP server connections (in parallel) and stop the background loop. Each server
Task is signalled to exit its own ``async with`` so the anyio cancel-scope cleanup runs in
the Task that opened it. ``scope`` restricts teardown to one multiplexed profile's servers
(its ``/reload-mcp`` must not kill other profiles') and leaves the shared loop running if
anything else is still connected. ``names`` restricts it further to those server names
(dropped-from-config pruning); other servers' bookkeeping is untouched. Only the bare call
(no ``scope``, no ``names``) is the process-wide wildcard: the launch profile's registry
scope IS ``None``, so ``scope=None, names={...}`` prunes that unscoped owner's servers and
must leave a served profile's same-named ``(B, name)`` connection alone. ``timeout`` bounds
the wait for the close to land on the MCP loop — a caller running one pass per served
profile under a total budget divides it, or N profiles × 15s starve the wildcard pass that
actually stops the loop."""
from tools.mcp_tool_scope import _key_name
wildcard = scope is None and names is None
with _core._lock:
selected = [key for key in _core._servers if wildcard or _core._server_scope_keys.get(key) == scope]
if names is not None:
selected = [key for key in selected if _key_name(key) in names]
servers_snapshot = [_core._servers[key] for key in selected]
if names is not None:
selected_status = set(selected)
elif wildcard:
selected_status = (
set(_core._servers) | set(_core._server_scope_keys)
| set(_core._server_tool_scopes)
| set(_core._server_connecting) | set(_core._server_connect_errors))
else:
selected_status = {key for key, owner in _core._server_scope_keys.items() if owner == scope}
# Adopters of the connections being torn down lose their overlays with the tasks' own
# ``_deregister_tools``; remember them so the next discovery pass re-registers them
# (``_reregister_orphaned_adopters``).
if not wildcard:
for key in selected:
for adopter in _core._server_tool_scopes.get(key, ()):
if adopter != scope:
_core._orphaned_adopters.setdefault(adopter, set()).add(_key_name(key))
def clear_selected_status():
_core._server_connecting.difference_update(selected_status)
for key in selected_status:
_core._server_connect_errors.pop(key, None)
_core._server_scope_keys.pop(key, None)
_core._server_tool_scopes.pop(key, None)
# Fast path: nothing to shut down. The connect-cooldown maps can still be populated here — a server that
# failed to connect is never recorded in ``_servers`` (that is the very premise of the #50394 cooldown),
# so "no live servers" is the MOST likely state in which stale backoff entries exist. Clear them so a
# post-shutdown restart re-attempts every configured server immediately.
if servers_snapshot:
async def _shutdown():
results = await asyncio.gather(*(server.shutdown() for server in servers_snapshot), return_exceptions=True)
for server, result in zip(servers_snapshot, results):
if isinstance(result, Exception):
logger.debug("Error closing MCP server '%s': %s", server.name, result)
with _core._lock:
for key in selected:
_core._servers.pop(key, None)
_core._server_scope_keys.pop(key, None)
clear_selected_status()
_clear_connect_cooldowns(None if wildcard else selected_status)
with _core._lock:
loop = _core._mcp_loop
if loop is not None and loop.is_running():
from agent.async_utils import safe_schedule_threadsafe
future = safe_schedule_threadsafe(_shutdown(), loop, logger=logger, log_message="MCP shutdown: failed to schedule")
if future is not None:
try:
future.result(timeout=timeout)
except BaseException as exc:
logger.debug("Error during MCP shutdown: %s", exc)
# Unconditional final sweep: whether ``_shutdown`` ran, timed out, or was never scheduled
# (a server that failed to connect is never in ``_servers`` — the most likely state for
# stale backoff entries), no connect-cooldown state may survive shutdown.
with _core._lock:
if not servers_snapshot:
clear_selected_status()
_clear_connect_cooldowns(None if wildcard else selected_status)
_loop._stop_mcp_loop(only_if_idle=not wildcard)
# A removed subset still shares its profile's log with the remaining servers.
# Full/profile shutdown must also release handles left by completed CLI/UI probes.
if names is None:
from tools.mcp_tool_config import _close_mcp_stderr_logs
_close_mcp_stderr_logs(scope=scope)
def _take_reapable_pids(include_active: bool, server_name: Optional[str]) -> tuple[Dict[int, str], Dict[int, int]]:
"""Pop the PIDs to reap (and their spawn-time pgids) out of the ledgers under the lock, so
a future spawn can't collide with stale state. Returns ``(pid -> owner, pid -> pgid)``."""
def _owned(entries: Dict[int, str]) -> Dict[int, str]:
return {pid: owner for pid, owner in entries.items() if server_name is None or owner == server_name}
with _core._lock:
pids = _owned({opid: _orphan_stdio_pid_servers.get(opid, "orphan") for opid in _orphan_stdio_pids})
_orphan_stdio_pids.difference_update(pids)
for opid in pids:
_orphan_stdio_pid_servers.pop(opid, None)
if include_active:
active = _owned(_stdio_pids)
pids.update(active)
for pid in active:
_stdio_pids.pop(pid, None)
pgids = {pid: _stdio_pgids.pop(pid) for pid in pids if pid in _stdio_pgids}
return pids, pgids
def _signal_mcp_process(pid: int, sig: int, server_name: str, pgid: Optional[int], my_pgid: Optional[int]) -> None:
"""SIGTERM/SIGKILL via the spawn-time pgroup on POSIX (reaches reparented grandchildren),
falling back to a per-pid signal."""
killpg = getattr(os, "killpg", None)
if pgid is not None and killpg is not None:
if my_pgid is not None and pgid == my_pgid:
# Child shares the gateway's pgroup: killpg would kill the gateway too, so use
# per-pid kill. Warn because per-pid kill can't reach grandchildren in this group.
logger.warning("MCP server '%s' pgid %d matches gateway pgid; skipping "
# Fall through to the per-pid kill() path instead. Warn because per-pid kill
# cannot reach grandchildren in this shared group — if the direct child has
# already exited, they may leak (inherent: group-killing them would also kill the
# gateway). See #47134.
"killpg to avoid self-kill and using per-pid kill — any "
"grandchildren in this group may not be reaped", server_name, pgid)
else:
try:
killpg(pgid, sig)
return
except (ProcessLookupError, PermissionError, OSError) as exc:
# Pgroup gone or refused — still try the direct child.
logger.debug("killpg(%d, %d) failed for MCP server '%s': %s; falling back to kill(pid)",
pgid, sig, server_name, exc)
try:
os.kill(pid, sig)
except (ProcessLookupError, PermissionError, OSError):
pass
if os.name == "nt": # Windows has no pgid reaching reparented grandchildren — kill the tree
_kill_windows_process_tree(pid, sig)
def _kill_windows_process_tree(pid: int, sig: int) -> None:
"""Windows counterpart of the POSIX killpg path (#61059): after the direct child is signalled,
terminate every still-alive descendant (npx.cmd → node.exe) so graceful teardown cannot leave
orphans reparented with ParentId=null. Best-effort, per-descendant; never raises."""
import signal as _signal
try:
import psutil
except ImportError:
return
try:
parent = psutil.Process(pid)
descendants = parent.children(recursive=True)
except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
return
for child in descendants:
try:
child.terminate()
except Exception: # noqa: BLE001 - raced away or refused; sweep continues
pass
if sig == getattr(_signal, "SIGKILL", _signal.SIGTERM): # force pass: don't wait for graceful exit
_, alive = psutil.wait_procs(descendants, timeout=0)
for child in alive:
try:
child.kill()
except Exception: # noqa: BLE001
pass
def _kill_orphaned_mcp_children(include_active: bool = False, server_name: Optional[str] = None) -> None:
"""Best-effort reap of stdio MCP subprocesses: SIGTERM, wait 2s, SIGKILL survivors. By
default only ``_orphan_stdio_pids`` are reaped so concurrent cron jobs / live sessions are
untouched; ``include_active=True`` also kills every ``_stdio_pids`` entry and is only for
final shutdown after the MCP loop has stopped. ``server_name`` limits the sweep to one
server (stdio reconnects cleaning up their old transport)."""
import signal as _signal
pids, pgids = _take_reapable_pids(include_active, server_name)
if not pids: # skip the 2s sleep every MCP-free shutdown would otherwise pay
return
try: # our own pgid, so we never killpg() the gateway itself
my_pgid = os.getpgrp()
except (AttributeError, OSError):
my_pgid = None # Windows or restricted environment
for pid, owner in pids.items():
_signal_mcp_process(pid, _signal.SIGTERM, owner, pgids.get(pid), my_pgid)
logger.debug("Sent SIGTERM to orphaned MCP process %d (%s)", pid, owner)
time.sleep(2)
sigkill = getattr(_signal, "SIGKILL", _signal.SIGTERM)
from gateway.status import _pid_exists # ``os.kill(pid, 0)`` is NOT a no-op on Windows
for pid, owner in pids.items():
if _pid_exists(pid): # survived SIGTERM
_signal_mcp_process(pid, sigkill, owner, pgids.get(pid), my_pgid)
logger.warning("Force-killed MCP process %d (%s) after SIGTERM timeout", pid, owner)
# These groups are reaped. Release them last, so a crash partway through the SIGTERM/SIGKILL
# dance still leaves the supervisor holding them.
_core._update_death_supervisor("unregister", pgids.values())
def _stop_mcp_loop_if_idle() -> bool:
"""Stop the MCP loop only when no registered server still owns it. Probe paths create
temporary MCPServerTasks not placed in ``_servers``; they may clean up an idle loop but
must not tear down the process-global loop under live agent tools."""
return _loop._stop_mcp_loop(only_if_idle=True)
async def _drain_mcp_loop_tasks(*, timeout: Optional[float] = None) -> None:
"""Cancel every task still pending on the MCP loop and reap it. ``Task.cancel()`` only
schedules the throw, so tasks need a cancellation cycle before the loop goes away; wait
for them here, on their owning loop, bounded so a task that suppresses cancellation
cannot hang process exit."""
if timeout is None:
timeout = _core._MCP_LOOP_DRAIN_TIMEOUT
current = asyncio.current_task()
pending = [t for t in asyncio.all_tasks() if t is not current and not t.done()]
if not pending:
return
logger.debug("Draining %d pending task(s) from the MCP loop", len(pending))
for task in pending:
task.cancel()
done, still_pending = await asyncio.wait(pending, timeout=timeout)
for task in done:
if not task.cancelled():
task.exception() # mark retrieved so asyncio doesn't warn "exception was never retrieved"
if still_pending:
logger.warning("%d MCP loop task(s) still pending after %.1fs drain", len(still_pending), timeout)
async def _drain_and_stop_mcp_loop() -> None:
"""Drain pending tasks, then stop the loop from its owning thread. Both must run as one
loop-owned sequence: a ``loop.stop`` queued separately by a timed-out caller can overtake
the scheduled drain, leaving the drain coroutine itself pending when the loop is closed."""
loop = asyncio.get_running_loop()
try:
await _drain_mcp_loop_tasks(timeout=_core._MCP_LOOP_DRAIN_TIMEOUT)
finally:
loop.call_soon(loop.stop)