mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(mcp): bound stdio initialize handshake to stop subprocess/FD leak
A stdio MCP server that never completes `initialize` (e.g. emits a non-JSON-RPC frame and then blocks on stdin) leaks a child process plus its stdio pipes/pidfd on every discovery-retry cycle — unbounded, until the gateway hits EMFILE and every new open()/spawn fails (#59349). Root cause (confirmed by instrumenting the live repro, and different from the issue's own hypothesis): the spawned child IS captured in `new_pids`, so the report's "new_pids empty at finally" guess is not it. The real cause is that `session.initialize()` hangs forever on the garbage stream. `connect_timeout` only bounds the caller's `.result()` wait on the foreground thread — it does NOT cancel the `_run_stdio` coroutine on the background MCP loop. So the coroutine is stuck at `await session.initialize()` permanently, its cleanup `finally` never runs, the child is never reaped, and it stays invisible to the orphan-reaper (whose `_orphan_stdio_pids` set never gets populated). Fix: wrap `session.initialize()` in `asyncio.wait_for(..., connect_timeout)` so a stalled handshake fails instead of hanging. The TimeoutError unwinds through the SDK context managers (closing the child's stdin -> EOF -> exit) and lets the existing `finally` reap any straggler. Cross-platform — no signals/pgid/proc. Scope: stdio only. The HTTP path has the same `await session.initialize()` shape but spawns no subprocess (so it can't cause this leak) and already has httpx transport timeouts. Verified: the reporter's repro goes from unbounded growth to draining to zero; added a hermetic regression test (fake transport whose `initialize()` hangs, asserts the connect is bounded by connect_timeout) that fails on the pre-fix code and passes on the fix; 566 existing MCP tests pass; ruff clean. Repro confirmed on macOS (pipe FDs); the Linux-specific pidfd growth in the report should be equivalent — the reporter offered to validate on Linux. Closes #59349
This commit is contained in:
parent
743c116fb2
commit
1f6836cd81
2 changed files with 111 additions and 1 deletions
|
|
@ -2007,7 +2007,23 @@ class MCPServerTask:
|
|||
async with ClientSession(
|
||||
read_stream, write_stream, **sampling_kwargs
|
||||
) as session:
|
||||
self.initialize_result = await session.initialize()
|
||||
# Bound the MCP handshake. A stdio server that never
|
||||
# completes ``initialize`` (e.g. emits a non-JSON-RPC frame
|
||||
# and then blocks on stdin) otherwise hangs this coroutine
|
||||
# forever on the background loop: ``connect_timeout`` only
|
||||
# bounds the caller's ``.result()`` wait, not the coroutine
|
||||
# itself. Because the connect never unwinds, the cleanup
|
||||
# ``finally`` below never runs, so the spawned child and its
|
||||
# stdio pipes/pidfd leak on every discovery retry — unbounded
|
||||
# until the gateway hits EMFILE. Timing out here converts the
|
||||
# hang into a normal failure, letting the ``finally`` reap the
|
||||
# child. See #59349.
|
||||
connect_timeout = float(
|
||||
config.get("connect_timeout", _DEFAULT_CONNECT_TIMEOUT)
|
||||
)
|
||||
self.initialize_result = await asyncio.wait_for(
|
||||
session.initialize(), timeout=connect_timeout
|
||||
)
|
||||
self.session = session
|
||||
await self._discover_tools()
|
||||
self._ready.set()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue