Skip to content

Commit 7c3e67c

Browse files
Replace platform-specific process termination with psutil
This commit replaces the complex platform-specific process termination logic with a unified psutil-based implementation. This provides: - Cross-platform consistency (same code for Windows, macOS, Linux) - Better child process detection and termination - No shell commands (no subprocess spawning for process management) - Proper cleanup of subprocess objects to avoid ResourceWarnings - Graceful termination with timeout before force kill The new implementation uses psutil.Process to: 1. Find all child processes recursively 2. Terminate all processes gracefully 3. Wait for them to exit with timeout 4. Force kill any remaining processes This eliminates the need for: - Windows: taskkill /T shell commands - POSIX: os.killpg() and signal handling - Platform-specific code branches Reported-by: fweinberger
1 parent 2d1ec8b commit 7c3e67c

File tree

4 files changed

+78
-84
lines changed

4 files changed

+78
-84
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: 59 additions & 69 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,82 +252,72 @@ async def _create_platform_compatible_process(
252252
return process
253253

254254

255-
async def _kill_process_tree_windows(pid: int) -> None:
255+
async def _terminate_process_tree(pid: int, timeout: float = 2.0) -> None:
256256
"""
257-
Kill a process and all its children on Windows using taskkill /T.
257+
Terminate a process and all its children using psutil.
258+
259+
This provides consistent behavior across platforms and properly
260+
handles process trees without shell commands.
258261
"""
259-
# Use taskkill with /T flag to kill the process tree
260-
# Note: shell=True is needed on Windows for taskkill to work properly
261-
result = await anyio.to_thread.run_sync(
262-
lambda: subprocess.run(
263-
["taskkill", "/F", "/T", "/PID", str(pid)],
264-
capture_output=True,
265-
shell=True,
266-
check=False,
262+
try:
263+
parent = psutil.Process(pid)
264+
children = parent.children(recursive=True)
265+
266+
# First, try graceful termination for all processes
267+
for child in children:
268+
try:
269+
child.terminate()
270+
except psutil.NoSuchProcess:
271+
pass
272+
273+
try:
274+
parent.terminate()
275+
except psutil.NoSuchProcess:
276+
return # Parent already dead
277+
278+
# Wait for processes to exit gracefully
279+
all_procs = children + [parent]
280+
_, alive = await anyio.to_thread.run_sync(
281+
lambda: psutil.wait_procs(all_procs, timeout=timeout)
267282
)
268-
)
269-
270-
# If taskkill failed, try without /T flag as a fallback
271-
# This handles the case where the process might have already exited
272-
if result.returncode != 0:
273-
await anyio.to_thread.run_sync(
274-
lambda: subprocess.run(
275-
["taskkill", "/F", "/PID", str(pid)],
276-
capture_output=True,
277-
shell=True,
278-
check=False,
283+
284+
# Force kill any remaining processes
285+
for proc in alive:
286+
try:
287+
proc.kill()
288+
except psutil.NoSuchProcess:
289+
pass
290+
291+
# Wait a bit more for force-killed processes
292+
if alive:
293+
await anyio.to_thread.run_sync(
294+
lambda: psutil.wait_procs(alive, timeout=0.5)
279295
)
280-
)
296+
297+
except psutil.NoSuchProcess:
298+
# Process already terminated
299+
pass
281300

282301

283302
async def _terminate_process_with_children(process: Process | FallbackProcess, timeout: float = 2.0) -> None:
284303
"""
285-
Terminate a process and all its children across platforms.
286-
287-
There's no cross-platform way in the stdlib to kill a process AND its children,
288-
so we need platform-specific handling:
289-
- POSIX: Use process groups and os.killpg()
290-
- Windows: Use taskkill with /T flag for tree termination
291-
304+
Terminate a process and all its children across platforms using psutil.
305+
306+
This provides consistent behavior across all platforms and properly
307+
handles process trees.
308+
292309
Args:
293310
process: The process to terminate
294311
timeout: Time to wait for graceful termination before force killing
295312
"""
296-
if sys.platform != "win32":
297-
# POSIX: Kill entire process group to avoid orphaning children
298-
pid = getattr(process, "pid", None)
299-
if not pid:
300-
return
301-
302-
try:
303-
# Try graceful termination of the group first
304-
pgid = os.getpgid(pid)
305-
os.killpg(pgid, signal.SIGTERM)
306-
307-
# Wait for termination
308-
with anyio.fail_after(timeout):
309-
await process.wait()
310-
except TimeoutError:
311-
# Force kill the process group
312-
try:
313-
pgid = os.getpgid(pid)
314-
os.killpg(pgid, signal.SIGKILL)
315-
except (OSError, ProcessLookupError):
316-
pass
317-
except (OSError, ProcessLookupError):
318-
# Process or group already dead
319-
pass
320-
else:
321-
# Windows: Extract PID from FallbackProcess if needed
322-
pid = getattr(process, "pid", None)
323-
if pid is None:
324-
popen = getattr(process, "popen", None)
325-
if popen:
326-
pid = getattr(popen, "pid", None)
327-
328-
if not pid:
329-
return
330-
331-
# On Windows, directly use taskkill /T to kill the entire process tree
332-
# Don't try graceful termination first as it only kills the parent
333-
await _kill_process_tree_windows(pid)
313+
# Extract PID from any process type
314+
pid = getattr(process, "pid", None)
315+
if pid is None:
316+
popen = getattr(process, "popen", None)
317+
if popen:
318+
pid = getattr(popen, "pid", None)
319+
320+
if not pid:
321+
return
322+
323+
await _terminate_process_tree(pid, timeout)

tests/client/test_stdio.py

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -443,11 +443,6 @@ async def test_stdio_client_child_process_cleanup():
443443
from mcp.client.stdio import _terminate_process_with_children
444444

445445
await _terminate_process_with_children(proc)
446-
447-
# Ensure the process object is properly cleaned up
448-
if sys.platform == "win32":
449-
# Wait for the process to be reaped to avoid ResourceWarning
450-
proc._popen.wait()
451446

452447
# Verify processes stopped
453448
await anyio.sleep(0.5)
@@ -557,11 +552,6 @@ async def test_stdio_client_nested_process_tree():
557552
from mcp.client.stdio import _terminate_process_with_children
558553

559554
await _terminate_process_with_children(proc)
560-
561-
# Ensure the process object is properly cleaned up
562-
if sys.platform == "win32":
563-
# Wait for the process to be reaped to avoid ResourceWarning
564-
proc._popen.wait()
565555

566556
# Verify all stopped
567557
await anyio.sleep(0.5)
@@ -651,11 +641,6 @@ def handle_term(sig, frame):
651641
from mcp.client.stdio import _terminate_process_with_children
652642

653643
await _terminate_process_with_children(proc)
654-
655-
# Ensure the process object is properly cleaned up
656-
if sys.platform == "win32":
657-
# Wait for the process to be reaped to avoid ResourceWarning
658-
proc._popen.wait()
659644

660645
# Verify child stopped
661646
await anyio.sleep(0.5)

uv.lock

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)