Skip to content

Commit e321d91

Browse files
Implement cross-platform subprocess and children termination
- Add process group support on POSIX systems via start_new_session=True - Implement _terminate_process_with_children() and _kill_process_and_children() functions that properly handle child process cleanup - On POSIX: Kill entire process group at once to avoid orphaning children when parent dies first - On Windows: Use taskkill /T for process tree termination - Add pid property to FallbackProcess for consistent interface - Add proper type annotations (Process | FallbackProcess) - Replace existing termination logic with new unified approach This addresses issues from PRs #850 (npx child processes) and #729 (Windows process trees) in a unified, dependency-free way. Reported-by:surya-prakash-susarla Reported-by:jingx8885 Github-Issue:#547 Add regression tests for child process termination - test_stdio_client_child_process_cleanup: Verifies that child processes spawned by the parent (like npx spawning node) are properly terminated - test_stdio_client_nested_process_tree: Tests that deeply nested process trees (parent -> child -> grandchild) are all terminated - test_stdio_client_early_parent_exit: Tests the race condition where parent exits during cleanup but children are still terminated via process group These tests verify the fix works correctly across different subprocess spawning scenarios without requiring external dependencies like psutil.
1 parent 721133e commit e321d91

File tree

5 files changed

+455
-15
lines changed

5 files changed

+455
-15
lines changed

pyproject.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ dependencies = [
3232
"pydantic-settings>=2.5.2",
3333
"uvicorn>=0.23.1; sys_platform != 'emscripten'",
3434
"jsonschema>=4.20.0",
35+
"psutil>=5.9.0,<6.0.0",
3536
]
3637

3738
[project.optional-dependencies]
@@ -110,6 +111,9 @@ members = ["examples/servers/*"]
110111
[tool.uv.sources]
111112
mcp = { workspace = true }
112113

114+
[[tool.uv.index]]
115+
url = "https://pypi.org/simple"
116+
113117
[tool.pytest.ini_options]
114118
log_cli = true
115119
xfail_strict = true

src/mcp/client/stdio/__init__.py

Lines changed: 89 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
import anyio
88
import anyio.lowlevel
9+
import anyio.to_thread
10+
import psutil
11+
from anyio.abc import Process
912
from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream
1013
from anyio.streams.text import TextReceiveStream
1114
from pydantic import BaseModel, Field
@@ -14,6 +17,7 @@
1417
from mcp.shared.message import SessionMessage
1518

1619
from .win32 import (
20+
FallbackProcess,
1721
create_windows_process,
1822
get_windows_executable_command,
1923
)
@@ -180,7 +184,7 @@ async def stdin_writer():
180184
# MCP spec: stdio shutdown sequence
181185
# 1. Close input stream to server
182186
# 2. Wait for server to exit, or send SIGTERM if it doesn't exit in time
183-
# 3. Send SIGKILL if still not exited
187+
# 3. Send SIGKILL if still not exited (forcibly kill on Windows)
184188
if process.stdin:
185189
try:
186190
await process.stdin.aclose()
@@ -193,18 +197,9 @@ async def stdin_writer():
193197
with anyio.fail_after(2.0):
194198
await process.wait()
195199
except TimeoutError:
196-
# Process didn't exit from stdin closure, escalate to SIGTERM
197-
try:
198-
process.terminate()
199-
with anyio.fail_after(2.0):
200-
await process.wait()
201-
except TimeoutError:
202-
# Process didn't respond to SIGTERM, force kill it
203-
process.kill()
204-
await process.wait()
205-
except ProcessLookupError:
206-
# Process already exited, which is fine
207-
pass
200+
# Process didn't exit from stdin closure, use our termination function
201+
# that handles child processes properly
202+
await _terminate_process_with_children(process)
208203
except ProcessLookupError:
209204
# Process already exited, which is fine
210205
pass
@@ -245,6 +240,86 @@ async def _create_platform_compatible_process(
245240
if sys.platform == "win32":
246241
process = await create_windows_process(command, args, env, errlog, cwd)
247242
else:
248-
process = await anyio.open_process([command, *args], env=env, stderr=errlog, cwd=cwd)
243+
# POSIX: Create new process group for easier cleanup
244+
process = await anyio.open_process(
245+
[command, *args],
246+
env=env,
247+
stderr=errlog,
248+
cwd=cwd,
249+
start_new_session=True, # Creates new process group
250+
)
249251

250252
return process
253+
254+
255+
async def _terminate_process_with_children(process: Process | FallbackProcess, timeout: float = 2.0) -> None:
256+
"""
257+
Terminate a process and all its children across platforms using psutil.
258+
259+
This provides consistent behavior across POSIX & Windows platforms and
260+
properly handles process trees.
261+
262+
Args:
263+
process: The process to terminate
264+
timeout: Time to wait for graceful termination before force killing
265+
"""
266+
pid = getattr(process, "pid", None)
267+
if pid is None:
268+
popen = getattr(process, "popen", None)
269+
if popen:
270+
pid = getattr(popen, "pid", None)
271+
272+
if not pid:
273+
return
274+
275+
await _terminate_process_tree(pid, timeout)
276+
277+
278+
async def _terminate_process_tree(pid: int, timeout: float = 2.0) -> None:
279+
"""
280+
Terminate a process and all its children using psutil.
281+
282+
This provides consistent behavior across platforms and properly
283+
handles process trees without shell commands.
284+
285+
Platform behavior:
286+
- On Unix: psutil.terminate() sends SIGTERM, allowing graceful shutdown
287+
- On Windows: psutil.terminate() calls TerminateProcess() which is immediate
288+
and doesn't allow cleanup handlers to run. This can cause ResourceWarnings
289+
for subprocess.Popen objects that don't get to clean up.
290+
"""
291+
try:
292+
parent = psutil.Process(pid)
293+
children = parent.children(recursive=True)
294+
295+
# First, try graceful termination for all children
296+
for child in children:
297+
try:
298+
child.terminate()
299+
except psutil.NoSuchProcess:
300+
pass
301+
302+
# Then, also terminate the parent process
303+
try:
304+
parent.terminate()
305+
except psutil.NoSuchProcess:
306+
return
307+
308+
# Wait for processes to exit gracefully
309+
all_procs = children + [parent]
310+
_, alive = await anyio.to_thread.run_sync(lambda: psutil.wait_procs(all_procs, timeout=timeout))
311+
312+
# Force kill any remaining processes
313+
for proc in alive:
314+
try:
315+
proc.kill()
316+
except psutil.NoSuchProcess:
317+
pass
318+
319+
# Wait a bit more for force-killed processes
320+
if alive:
321+
await anyio.to_thread.run_sync(lambda: psutil.wait_procs(alive, timeout=0.5))
322+
323+
except psutil.NoSuchProcess:
324+
# Process already terminated
325+
pass

src/mcp/client/stdio/win32.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ def kill(self) -> None:
116116
"""Kill the subprocess immediately (alias for terminate)."""
117117
self.terminate()
118118

119+
@property
120+
def pid(self) -> int:
121+
"""Return the process ID."""
122+
return self.popen.pid
123+
119124

120125
# ------------------------
121126
# Updated function

0 commit comments

Comments
 (0)