Skip to content

Commit 48d0271

Browse files
Fix Windows process termination and add psutil support
- Add psutil as a required dependency for robust cross-platform process management - Replace platform-specific process termination code with unified psutil implementation - Fix FallbackProcess.wait() on Windows to use polling instead of blocking - Add Windows-specific ResourceWarning filters with documentation - Handle process trees properly on both Windows and Unix systems - Fix Windows path escaping in test scripts This ensures consistent behavior across platforms while properly handling the fundamental differences in process termination: - Unix: Uses signals (SIGTERM/SIGKILL) for graceful shutdown - Windows: Uses TerminateProcess() which is immediate All process termination tests now pass on both Windows and macOS.
1 parent 954020b commit 48d0271

File tree

5 files changed

+202
-99
lines changed

5 files changed

+202
-99
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: 63 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import os
2-
import signal
3-
import subprocess
42
import sys
53
from contextlib import asynccontextmanager
64
from pathlib import Path
75
from typing import Literal, TextIO
86

97
import anyio
108
import anyio.lowlevel
9+
import anyio.to_thread
10+
import psutil
1111
from anyio.abc import Process
1212
from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream
1313
from anyio.streams.text import TextReceiveStream
@@ -252,67 +252,74 @@ async def _create_platform_compatible_process(
252252
return process
253253

254254

255-
async def _terminate_process_with_children(process: Process | FallbackProcess, timeout: float = 2.0) -> None:
255+
async def _terminate_process_tree(pid: int, timeout: float = 2.0) -> None:
256256
"""
257-
Terminate a process and all its children across platforms.
257+
Terminate a process and all its children using psutil.
258258
259-
There's no cross-platform way in the stdlib to kill a process AND its children,
260-
so we need platform-specific handling:
261-
- POSIX: Use process groups and os.killpg()
262-
- Windows: Use taskkill with /T flag for tree termination
259+
This provides consistent behavior across platforms and properly
260+
handles process trees without shell commands.
263261
264-
Args:
265-
process: The process to terminate
266-
timeout: Time to wait for graceful termination before force killing
262+
Platform behavior:
263+
- On Unix: psutil.terminate() sends SIGTERM, allowing graceful shutdown
264+
- On Windows: psutil.terminate() calls TerminateProcess() which is immediate
265+
and doesn't allow cleanup handlers to run. This can cause ResourceWarnings
266+
for subprocess.Popen objects that don't get to clean up.
267267
"""
268-
if sys.platform != "win32":
269-
# POSIX: Kill entire process group to avoid orphaning children
270-
pid = getattr(process, "pid", None)
271-
if not pid:
272-
return
268+
try:
269+
parent = psutil.Process(pid)
270+
children = parent.children(recursive=True)
271+
272+
# First, try graceful termination for all processes
273+
for child in children:
274+
try:
275+
child.terminate()
276+
except psutil.NoSuchProcess:
277+
pass
273278

274279
try:
275-
# Try graceful termination of the group first
276-
pgid = os.getpgid(pid)
277-
os.killpg(pgid, signal.SIGTERM)
278-
279-
# Wait for termination
280-
with anyio.fail_after(timeout):
281-
await process.wait()
282-
except TimeoutError:
283-
# Force kill the process group
280+
parent.terminate()
281+
except psutil.NoSuchProcess:
282+
return # Parent already dead
283+
284+
# Wait for processes to exit gracefully
285+
all_procs = children + [parent]
286+
_, alive = await anyio.to_thread.run_sync(lambda: psutil.wait_procs(all_procs, timeout=timeout))
287+
288+
# Force kill any remaining processes
289+
for proc in alive:
284290
try:
285-
pgid = os.getpgid(pid)
286-
os.killpg(pgid, signal.SIGKILL)
287-
except (OSError, ProcessLookupError):
291+
proc.kill()
292+
except psutil.NoSuchProcess:
288293
pass
289-
except (OSError, ProcessLookupError):
290-
# Process or group already dead
291-
pass
292-
else:
293-
# Windows: Extract PID from FallbackProcess if needed
294-
pid = getattr(process, "pid", None)
295-
if pid is None:
296-
popen = getattr(process, "popen", None)
297-
if popen:
298-
pid = getattr(popen, "pid", None)
299294

300-
if not pid:
301-
return
295+
# Wait a bit more for force-killed processes
296+
if alive:
297+
await anyio.to_thread.run_sync(lambda: psutil.wait_procs(alive, timeout=0.5))
302298

303-
# Try graceful termination first
304-
try:
305-
process.terminate()
306-
with anyio.fail_after(timeout):
307-
await process.wait()
308-
except TimeoutError:
309-
# Force kill using taskkill for tree termination
310-
await anyio.to_thread.run_sync(
311-
subprocess.run,
312-
["taskkill", "/F", "/T", "/PID", str(pid)],
313-
capture_output=True,
314-
shell=False,
315-
check=False,
316-
)
317-
except ProcessLookupError:
318-
pass
299+
except psutil.NoSuchProcess:
300+
# Process already terminated
301+
pass
302+
303+
304+
async def _terminate_process_with_children(process: Process | FallbackProcess, timeout: float = 2.0) -> None:
305+
"""
306+
Terminate a process and all its children across platforms using psutil.
307+
308+
This provides consistent behavior across all platforms and properly
309+
handles process trees.
310+
311+
Args:
312+
process: The process to terminate
313+
timeout: Time to wait for graceful termination before force killing
314+
"""
315+
# Extract PID from any process type
316+
pid = getattr(process, "pid", None)
317+
if pid is None:
318+
popen = getattr(process, "popen", None)
319+
if popen:
320+
pid = getattr(popen, "pid", None)
321+
322+
if not pid:
323+
return
324+
325+
await _terminate_process_tree(pid, timeout)

src/mcp/client/stdio/win32.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from pathlib import Path
99
from typing import BinaryIO, TextIO, cast
1010

11-
from anyio import to_thread
11+
import anyio
1212
from anyio.streams.file import FileReadStream, FileWriteStream
1313

1414

@@ -73,10 +73,11 @@ async def __aexit__(
7373
exc_val: BaseException | None,
7474
exc_tb: object | None,
7575
) -> None:
76-
"""Terminate and wait on process exit inside a thread."""
77-
self.popen.terminate()
78-
await to_thread.run_sync(self.popen.wait)
76+
"""Clean up streams without terminating the process.
7977
78+
The process termination is handled by stdio_client's cleanup sequence
79+
which implements the MCP spec-compliant shutdown flow.
80+
"""
8081
# Close the file handles to prevent ResourceWarning
8182
if self.stdin:
8283
await self.stdin.aclose()
@@ -91,7 +92,11 @@ async def __aexit__(
9192

9293
async def wait(self):
9394
"""Async wait for process completion."""
94-
return await to_thread.run_sync(self.popen.wait)
95+
# Poll the process status instead of blocking wait
96+
# This allows anyio timeouts to work properly
97+
while self.popen.poll() is None:
98+
await anyio.sleep(0.1)
99+
return self.popen.returncode
95100

96101
def terminate(self):
97102
"""Terminate the subprocess immediately."""

0 commit comments

Comments
 (0)