Skip to content

Commit a328c9b

Browse files
Refactor platform-specific code into src/mcp/os directory structure
Per @dsp-ant's review feedback, reorganize platform-specific utilities: - Create src/mcp/os/ with win32/ and posix/ subdirectories - Move all Windows-specific code to mcp.os.win32.utilities - Extract POSIX process termination to mcp.os.posix.utilities - Update logger to use __name__ instead of hardcoded string - Rename timeout parameter to timeout_seconds for clarity - Document that missing PID means no process to terminate This structure allows platform utilities to be shared across client, server, and other components as needed.
1 parent 046bed4 commit a328c9b

File tree

6 files changed

+85
-42
lines changed

6 files changed

+85
-42
lines changed

src/mcp/client/stdio/__init__.py

Lines changed: 11 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import logging
22
import os
3-
import signal
43
import sys
54
from contextlib import asynccontextmanager
65
from pathlib import Path
@@ -14,16 +13,16 @@
1413
from pydantic import BaseModel, Field
1514

1615
import mcp.types as types
17-
from mcp.shared.message import SessionMessage
18-
19-
from .win32 import (
16+
from mcp.os.posix.utilities import terminate_posix_process_tree
17+
from mcp.os.win32.utilities import (
2018
FallbackProcess,
2119
create_windows_process,
2220
get_windows_executable_command,
2321
terminate_windows_process_tree,
2422
)
23+
from mcp.shared.message import SessionMessage
2524

26-
logger = logging.getLogger("client.stdio")
25+
logger = logging.getLogger(__name__)
2726

2827
# Environment variables to inherit by default
2928
DEFAULT_INHERITED_ENV_VARS = (
@@ -247,47 +246,18 @@ async def _create_platform_compatible_process(
247246
return process
248247

249248

250-
async def _terminate_process_tree(process: Process | FallbackProcess, timeout: float = 2.0) -> None:
249+
async def _terminate_process_tree(process: Process | FallbackProcess, timeout_seconds: float = 2.0) -> None:
251250
"""
252251
Terminate a process and all its children using platform-specific methods.
253252
254253
Unix: Uses os.killpg() for atomic process group termination
255254
Windows: Uses Job Objects via pywin32 for reliable child process cleanup
255+
256+
Args:
257+
process: The process to terminate
258+
timeout_seconds: Timeout in seconds before force killing (default: 2.0)
256259
"""
257260
if sys.platform == "win32":
258-
await terminate_windows_process_tree(process, timeout)
261+
await terminate_windows_process_tree(process, timeout_seconds)
259262
else:
260-
pid = getattr(process, "pid", None) or getattr(getattr(process, "popen", None), "pid", None)
261-
if not pid:
262-
return
263-
264-
try:
265-
pgid = os.getpgid(pid)
266-
os.killpg(pgid, signal.SIGTERM)
267-
268-
with anyio.move_on_after(timeout):
269-
while True:
270-
try:
271-
# Check if process group still exists (signal 0 = check only)
272-
os.killpg(pgid, 0)
273-
await anyio.sleep(0.1)
274-
except ProcessLookupError:
275-
return
276-
277-
try:
278-
os.killpg(pgid, signal.SIGKILL)
279-
except ProcessLookupError:
280-
pass
281-
282-
except (ProcessLookupError, PermissionError, OSError) as e:
283-
logger.warning(f"Process group termination failed for PID {pid}: {e}, falling back to simple terminate")
284-
try:
285-
process.terminate()
286-
with anyio.fail_after(timeout):
287-
await process.wait()
288-
except Exception as term_error:
289-
logger.warning(f"Process termination failed for PID {pid}: {term_error}, attempting force kill")
290-
try:
291-
process.kill()
292-
except Exception as kill_error:
293-
logger.error(f"Failed to kill process {pid}: {kill_error}")
263+
await terminate_posix_process_tree(process, timeout_seconds)

src/mcp/os/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""Platform-specific utilities for MCP."""

src/mcp/os/posix/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""POSIX-specific utilities for MCP."""

src/mcp/os/posix/utilities.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
"""
2+
POSIX-specific functionality for stdio client operations.
3+
"""
4+
5+
import logging
6+
import os
7+
import signal
8+
from typing import TYPE_CHECKING, Union
9+
10+
import anyio
11+
from anyio.abc import Process
12+
13+
if TYPE_CHECKING:
14+
from mcp.os.win32.utilities import FallbackProcess
15+
16+
logger = logging.getLogger(__name__)
17+
18+
19+
async def terminate_posix_process_tree(
20+
process: Union[Process, "FallbackProcess"], timeout_seconds: float = 2.0
21+
) -> None:
22+
"""
23+
Terminate a process and all its children on POSIX systems.
24+
25+
Uses os.killpg() for atomic process group termination.
26+
27+
Args:
28+
process: The process to terminate
29+
timeout_seconds: Timeout in seconds before force killing (default: 2.0)
30+
"""
31+
pid = getattr(process, "pid", None) or getattr(getattr(process, "popen", None), "pid", None)
32+
if not pid:
33+
# No PID means there's no process to terminate - it either never started,
34+
# already exited, or we have an invalid process object
35+
return
36+
37+
try:
38+
pgid = os.getpgid(pid)
39+
os.killpg(pgid, signal.SIGTERM)
40+
41+
with anyio.move_on_after(timeout_seconds):
42+
while True:
43+
try:
44+
# Check if process group still exists (signal 0 = check only)
45+
os.killpg(pgid, 0)
46+
await anyio.sleep(0.1)
47+
except ProcessLookupError:
48+
return
49+
50+
try:
51+
os.killpg(pgid, signal.SIGKILL)
52+
except ProcessLookupError:
53+
pass
54+
55+
except (ProcessLookupError, PermissionError, OSError) as e:
56+
logger.warning(f"Process group termination failed for PID {pid}: {e}, falling back to simple terminate")
57+
try:
58+
process.terminate()
59+
with anyio.fail_after(timeout_seconds):
60+
await process.wait()
61+
except Exception as term_error:
62+
logger.warning(f"Process termination failed for PID {pid}: {term_error}, attempting force kill")
63+
try:
64+
process.kill()
65+
except Exception as kill_error:
66+
logger.error(f"Failed to kill process {pid}: {kill_error}")

src/mcp/os/win32/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""Windows-specific utilities for MCP."""

src/mcp/client/stdio/win32.py renamed to src/mcp/os/win32/utilities.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,16 @@ def _maybe_assign_process_to_job(process: Process | FallbackProcess, job: JobHan
278278
win32api.CloseHandle(job)
279279

280280

281-
async def terminate_windows_process_tree(process: Process | FallbackProcess, timeout: float = 2.0) -> None:
281+
async def terminate_windows_process_tree(process: Process | FallbackProcess, timeout_seconds: float = 2.0) -> None:
282282
"""
283283
Terminate a process and all its children on Windows.
284284
285285
If the process has an associated job object, it will be terminated.
286286
Otherwise, falls back to basic process termination.
287+
288+
Args:
289+
process: The process to terminate
290+
timeout_seconds: Timeout in seconds before force killing (default: 2.0)
287291
"""
288292
if sys.platform != "win32":
289293
return

0 commit comments

Comments
 (0)