From 086596ca2b988ca42fd57a5a674b7a00d37998be Mon Sep 17 00:00:00 2001
From: liuhao1024 <sunsky.lau@gmail.com>
Date: Fri, 3 Jul 2026 05:27:31 +0800
Subject: [PATCH] fix(mcp): reap orphaned subprocesses before spawning new ones
 on retry
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When an MCP stdio subprocess fails to connect (token expiry, port
contention, timeout), the run() reconnect loop retries with backoff.
Each retry calls _run_stdio() which spawns a new process pair, but the
previous failed pair was only detected as orphaned (added to
_orphan_stdio_pids) — never actually killed.  This caused rapid zombie
accumulation: 5 failed attempts × 2 procs each = 10 orphans competing
for the same port.

Add a _kill_orphaned_mcp_children() call at the top of _run_stdio(),
before the _snapshot_child_pids() baseline, so any orphans from prior
failed attempts are reaped before a new subprocess is spawned.

Fixes #57355
---
 tests/tools/test_mcp_stability.py | 62 +++++++++++++++++++++++++++++++
 tools/mcp_tool.py                 |  7 ++++
 2 files changed, 69 insertions(+)

diff --git a/tests/tools/test_mcp_stability.py b/tests/tools/test_mcp_stability.py
index 796dbed913d..c266dc98bc2 100644
--- a/tests/tools/test_mcp_stability.py
+++ b/tests/tools/test_mcp_stability.py
@@ -199,6 +199,68 @@ class TestStdioPidTracking:
         with _lock:
             assert fake_pid not in _orphan_stdio_pids
 
+    def test_run_stdio_reaps_orphans_before_spawn(self):
+        """_run_stdio kills orphaned PIDs from prior failed attempts (#57355)."""
+        from tools.mcp_tool import (
+            _kill_orphaned_mcp_children,
+            _orphan_stdio_pids,
+            _stdio_pids,
+            _stdio_pgids,
+            _lock,
+            MCPServerTask,
+        )
+        from unittest.mock import patch, MagicMock, AsyncMock
+
+        # Seed an orphan PID that belongs to a prior failed connection.
+        fake_pid = 999999997
+        with _lock:
+            _orphan_stdio_pids.add(fake_pid)
+
+        server = MCPServerTask.__new__(MCPServerTask)
+        server.name = "test-zombie-reap"
+        server._ready = MagicMock()
+        server._shutdown_event = MagicMock()
+        server._shutdown_event.is_set.return_value = True
+        server._reconnect_event = MagicMock()
+        server._sampling = None
+        server._elicitation = None
+        server._registered_tool_names = []
+
+        config = {"command": "echo", "args": ["hello"]}
+
+        import asyncio
+
+        async def _run():
+            # _run_stdio should reap orphans before it gets to the
+            # stdio_client spawn.  Patch the OSV check (local import)
+            # and stdio_client so no real subprocess is spawned.
+            with patch("tools.mcp_tool._MCP_AVAILABLE", True), \
+                 patch("tools.mcp_tool._build_safe_env", return_value={}), \
+                 patch("tools.mcp_tool._resolve_stdio_command",
+                       return_value=("echo", {})), \
+                 patch("tools.mcp_tool._write_stderr_log_header"), \
+                 patch("tools.mcp_tool._get_mcp_stderr_log",
+                       return_value=None), \
+                 patch("tools.mcp_tool.check_package_for_malware",
+                       return_value=None, create=True), \
+                 patch("tools.osv_check.check_package_for_malware",
+                       return_value=None):
+                # Patch stdio_client to raise so the test exits quickly
+                cm = MagicMock()
+                cm.__aenter__ = AsyncMock(side_effect=RuntimeError("test"))
+                cm.__aexit__ = AsyncMock(return_value=False)
+                with patch("tools.mcp_tool.stdio_client", return_value=cm):
+                    try:
+                        await server._run_stdio(config)
+                    except Exception:
+                        pass
+
+        asyncio.run(_run())
+
+        # The orphan must have been reaped before the spawn attempt.
+        with _lock:
+            assert fake_pid not in _orphan_stdio_pids
+
 
 # ---------------------------------------------------------------------------
 # Fix 2b: stdio descendant reaping via process group (issue #23799)
diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py
index 329ddebdd73..a57bdf98aff 100644
--- a/tools/mcp_tool.py
+++ b/tools/mcp_tool.py
@@ -1951,6 +1951,13 @@ class MCPServerTask:
         if _MCP_LOGGING_CALLBACK_SUPPORTED:
             sampling_kwargs["logging_callback"] = self._make_logging_callback()
 
+        # Reap any orphaned subprocesses from a prior failed connection
+        # attempt before spawning a new one.  Without this, each retry in
+        # the run() reconnect loop spawns a fresh process pair while the
+        # previous failed pair lingers — leading to rapid zombie
+        # accumulation (see #57355).
+        _kill_orphaned_mcp_children()
+
         # Snapshot child PIDs before spawning so we can track the new one.
         pids_before = _snapshot_child_pids()
         new_pids: set = set()