Improve child process termination on POSIX & Windows by felixweinberger · Pull Request #1078 · modelcontextprotocol/python-sdk · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pyproject.toml
45 changes: 39 additions & 6 deletions src/mcp/client/stdio/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
import os
import sys
from contextlib import asynccontextmanager
Expand All @@ -6,17 +7,22 @@

import anyio
import anyio.lowlevel
from anyio.abc import Process
from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream
from anyio.streams.text import TextReceiveStream
from pydantic import BaseModel, Field

import mcp.types as types
from mcp.shared.message import SessionMessage

from .win32 import (
from mcp.os.posix.utilities import terminate_posix_process_tree
from mcp.os.win32.utilities import (
FallbackProcess,
create_windows_process,
get_windows_executable_command,
terminate_windows_process_tree,
)
from mcp.shared.message import SessionMessage

logger = logging.getLogger(__name__)

# Environment variables to inherit by default
DEFAULT_INHERITED_ENV_VARS = (
Expand Down Expand Up @@ -187,7 +193,7 @@ async def stdin_writer():
await process.wait()
except TimeoutError:
# If process doesn't terminate in time, force kill it
process.kill()
await _terminate_process_tree(process)
Comment thread
felixweinberger marked this conversation as resolved.
Outdated
except ProcessLookupError:
# Process already exited, which is fine
pass
Expand Down Expand Up @@ -222,11 +228,38 @@ async def _create_platform_compatible_process(
):
"""
Creates a subprocess in a platform-compatible way.
Returns a process handle.

Unix: Creates process in a new session/process group for killpg support
Windows: Creates process in a Job Object for reliable child termination
"""
if sys.platform == "win32":
process = await create_windows_process(command, args, env, errlog, cwd)
else:
process = await anyio.open_process([command, *args], env=env, stderr=errlog, cwd=cwd)
process = await anyio.open_process(
[command, *args],
env=env,
stderr=errlog,
cwd=cwd,
start_new_session=True,
)

return process


async def _terminate_process_tree(process: Process | FallbackProcess, timeout_seconds: float = 2.0) -> None:
"""
Terminate a process and all its children using platform-specific methods.

Unix: Uses os.killpg() for atomic process group termination
Windows: Uses Job Objects via pywin32 for reliable child process cleanup

Args:
process: The process to terminate
timeout_seconds: Timeout in seconds before force killing (default: 2.0)
"""
if sys.platform == "win32":
await terminate_windows_process_tree(process, timeout_seconds)
else:
# FallbackProcess should only be used for Windows compatibility
assert isinstance(process, Process)
await terminate_posix_process_tree(process, timeout_seconds)
1 change: 1 addition & 0 deletions src/mcp/os/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Platform-specific utilities for MCP."""
1 change: 1 addition & 0 deletions src/mcp/os/posix/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""POSIX-specific utilities for MCP."""
60 changes: 60 additions & 0 deletions src/mcp/os/posix/utilities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
"""
POSIX-specific functionality for stdio client operations.
"""

import logging
import os
import signal

import anyio
from anyio.abc import Process

logger = logging.getLogger(__name__)


async def terminate_posix_process_tree(process: Process, timeout_seconds: float = 2.0) -> None:
"""
Terminate a process and all its children on POSIX systems.

Uses os.killpg() for atomic process group termination.

Args:
process: The process to terminate
timeout_seconds: Timeout in seconds before force killing (default: 2.0)
"""
pid = getattr(process, "pid", None) or getattr(getattr(process, "popen", None), "pid", None)
if not pid:
# No PID means there's no process to terminate - it either never started,
# already exited, or we have an invalid process object
return

try:
pgid = os.getpgid(pid)
os.killpg(pgid, signal.SIGTERM)

with anyio.move_on_after(timeout_seconds):
while True:
try:
# Check if process group still exists (signal 0 = check only)
os.killpg(pgid, 0)
await anyio.sleep(0.1)
except ProcessLookupError:
return

try:
os.killpg(pgid, signal.SIGKILL)
except ProcessLookupError:
pass

except (ProcessLookupError, PermissionError, OSError) as e:
logger.warning(f"Process group termination failed for PID {pid}: {e}, falling back to simple terminate")
try:
process.terminate()
with anyio.fail_after(timeout_seconds):
await process.wait()
except Exception as term_error:
logger.warning(f"Process termination failed for PID {pid}: {term_error}, attempting force kill")
try:
process.kill()
except Exception as kill_error:
logger.error(f"Failed to kill process {pid}: {kill_error}")
1 change: 1 addition & 0 deletions src/mcp/os/win32/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Windows-specific utilities for MCP."""
129 changes: 120 additions & 9 deletions src/mcp/client/stdio/win32.py → src/mcp/os/win32/utilities.py
Loading