Skip to content

Commit 0ac8b69

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
1 parent 3f6c472 commit 0ac8b69

File tree

2 files changed

+86
-13
lines changed

2 files changed

+86
-13
lines changed

src/mcp/client/stdio/__init__.py

Lines changed: 81 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import os
2+
import signal
3+
import subprocess
24
import sys
35
from contextlib import asynccontextmanager
46
from pathlib import Path
57
from typing import Literal, TextIO
68

79
import anyio
810
import anyio.lowlevel
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
)
@@ -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,79 @@ 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.
258+
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
263+
264+
Args:
265+
process: The process to terminate
266+
timeout: Time to wait for graceful termination before force killing
267+
"""
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
273+
274+
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
284+
try:
285+
pgid = os.getpgid(pid)
286+
os.killpg(pgid, signal.SIGKILL)
287+
except (OSError, ProcessLookupError):
288+
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)
299+
300+
if not pid:
301+
return
302+
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

src/mcp/client/stdio/win32.py

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

104+
@property
105+
def pid(self) -> int:
106+
"""Return the process ID."""
107+
return self.popen.pid
108+
104109

105110
# ------------------------
106111
# Updated function

0 commit comments

Comments
 (0)