Skip to content

Commit 791b6dd

Browse files
Fix Windows process tree termination by using shell=True for taskkill
The issue was that taskkill /T requires shell=True to work properly on Windows. This was shown by the debug_process_flags.py output where taskkill successfully terminated process trees when called directly. Simplified the implementation: - Use shell=True for taskkill commands on Windows - Remove complex WMI fallback code that wasn't working - Keep simple fallback for single process termination This should fix the three failing Windows tests: - test_stdio_client_child_process_cleanup - test_stdio_client_early_parent_exit - test_stdio_client_nested_process_tree
1 parent b4a2bf9 commit 791b6dd

File tree

4 files changed

+254
-46
lines changed

4 files changed

+254
-46
lines changed

debug_process_tree.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def test_process_tree():
5353
# Child script that writes continuously
5454
child_script = """
5555
import time
56-
with open({repr(marker_file)}, 'a') as f:
56+
with open(r'{marker_file}', 'a') as f:
5757
while True:
5858
f.write(f"{{time.time()}}")
5959
f.flush()

debug_windows_termination.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
"""Debug Windows process termination - uses PowerShell instead of wmic"""
2+
import subprocess
3+
import sys
4+
import time
5+
import tempfile
6+
import os
7+
import json
8+
9+
def get_process_tree_powershell(pid):
10+
"""Get process tree using PowerShell"""
11+
ps_script = f"""
12+
Get-WmiObject Win32_Process | Where-Object {{ $_.ProcessId -eq {pid} -or $_.ParentProcessId -eq {pid} }} |
13+
Select-Object ProcessId, ParentProcessId, Name | ConvertTo-Json
14+
"""
15+
16+
try:
17+
result = subprocess.run(
18+
["powershell", "-Command", ps_script],
19+
capture_output=True,
20+
text=True,
21+
shell=True
22+
)
23+
if result.returncode == 0 and result.stdout:
24+
return json.loads(result.stdout)
25+
return []
26+
except:
27+
return []
28+
29+
def test_termination():
30+
# Create a marker file
31+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
32+
marker_file = f.name
33+
34+
print(f"Marker file: {marker_file}")
35+
36+
# Simple child script
37+
child_script = f"""
38+
import time
39+
print(f"Child PID: {{os.getpid()}}")
40+
with open(r'{marker_file}', 'a') as f:
41+
while True:
42+
f.write('x')
43+
f.flush()
44+
time.sleep(0.1)
45+
"""
46+
47+
# Parent script that spawns child
48+
parent_script = f"""
49+
import subprocess
50+
import sys
51+
import os
52+
print(f"Parent PID: {{os.getpid()}}")
53+
child = subprocess.Popen([sys.executable, '-c', r'''{child_script}'''])
54+
print(f"Started child with PID: {{child.pid}}")
55+
while True:
56+
time.sleep(0.1)
57+
"""
58+
59+
print("\n=== Starting test processes ===")
60+
parent = subprocess.Popen([sys.executable, "-c", parent_script])
61+
print(f"Parent PID: {parent.pid}")
62+
63+
# Wait for processes to start
64+
time.sleep(1.0)
65+
66+
# Check file is being written
67+
if os.path.exists(marker_file):
68+
size1 = os.path.getsize(marker_file)
69+
time.sleep(0.5)
70+
size2 = os.path.getsize(marker_file)
71+
print(f"File growing: {size1} -> {size2} bytes")
72+
73+
# Show process tree
74+
print("\n=== Process tree before termination ===")
75+
processes = get_process_tree_powershell(parent.pid)
76+
for proc in processes if isinstance(processes, list) else [processes]:
77+
print(f"PID: {proc.get('ProcessId')} Parent: {proc.get('ParentProcessId')} Name: {proc.get('Name')}")
78+
79+
# Test different termination methods
80+
print("\n=== Test 1: parent.terminate() ===")
81+
parent.terminate()
82+
time.sleep(0.5)
83+
84+
# Check if child stopped
85+
if os.path.exists(marker_file):
86+
size3 = os.path.getsize(marker_file)
87+
time.sleep(0.5)
88+
size4 = os.path.getsize(marker_file)
89+
if size4 > size3:
90+
print(f"FAILED: Child still writing ({size3} -> {size4})")
91+
92+
# Try taskkill /T
93+
print("\n=== Test 2: taskkill /T ===")
94+
result = subprocess.run(
95+
["taskkill", "/F", "/T", "/PID", str(parent.pid)],
96+
capture_output=True,
97+
text=True,
98+
shell=True
99+
)
100+
print(f"Result: {result.returncode}")
101+
print(f"Output: {result.stdout}")
102+
103+
# Final check
104+
time.sleep(0.5)
105+
size5 = os.path.getsize(marker_file)
106+
time.sleep(0.5)
107+
size6 = os.path.getsize(marker_file)
108+
if size6 > size5:
109+
print(f"STILL FAILED: Child still writing ({size5} -> {size6})")
110+
111+
# Show remaining processes
112+
print("\n=== Remaining processes ===")
113+
remaining = get_process_tree_powershell(parent.pid)
114+
for proc in remaining if isinstance(remaining, list) else [remaining]:
115+
print(f"PID: {proc.get('ProcessId')} Parent: {proc.get('ParentProcessId')}")
116+
else:
117+
print("SUCCESS: Child stopped")
118+
else:
119+
print("SUCCESS: Child stopped with parent")
120+
121+
# Cleanup
122+
try:
123+
os.unlink(marker_file)
124+
except:
125+
pass
126+
127+
if __name__ == "__main__":
128+
test_termination()

src/mcp/client/stdio/__init__.py

Lines changed: 14 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -254,59 +254,28 @@ async def _create_platform_compatible_process(
254254

255255
async def _kill_process_tree_windows(pid: int) -> None:
256256
"""
257-
Recursively kill a process and all its children on Windows.
258-
Uses WMI to find child processes since taskkill /T doesn't always work.
257+
Kill a process and all its children on Windows using taskkill /T.
259258
"""
260-
# First try taskkill with /T flag
259+
# Use taskkill with /T flag to kill the process tree
260+
# Note: shell=True is needed on Windows for taskkill to work properly
261261
result = await anyio.to_thread.run_sync(
262262
subprocess.run,
263263
["taskkill", "/F", "/T", "/PID", str(pid)],
264264
capture_output=True,
265-
shell=False,
265+
shell=True,
266266
check=False,
267267
)
268268

269-
# If that worked, we're done
270-
if result.returncode == 0:
271-
return
272-
273-
# Otherwise, try to find and kill children manually
274-
# First, get all child processes
275-
children_pids: set[int] = set()
276-
277-
# Use wmic to get child processes
278-
wmic_result = await anyio.to_thread.run_sync(
279-
subprocess.run,
280-
["wmic", "process", "where", f"ParentProcessId={pid}", "get", "ProcessId", "/FORMAT:VALUE"],
281-
capture_output=True,
282-
shell=False,
283-
check=False,
284-
text=True,
285-
)
286-
287-
if wmic_result.returncode == 0 and wmic_result.stdout:
288-
# Parse child PIDs
289-
for line in wmic_result.stdout.strip().split('\n'):
290-
if line.startswith('ProcessId='):
291-
try:
292-
child_pid = int(line.split('=')[1].strip())
293-
if child_pid > 0:
294-
children_pids.add(child_pid)
295-
except (ValueError, IndexError):
296-
pass
297-
298-
# Recursively kill all children first
299-
for child_pid in children_pids:
300-
await _kill_process_tree_windows(child_pid)
301-
302-
# Finally kill the parent process
303-
await anyio.to_thread.run_sync(
304-
subprocess.run,
305-
["taskkill", "/F", "/PID", str(pid)],
306-
capture_output=True,
307-
shell=False,
308-
check=False,
309-
)
269+
# If taskkill failed, try without /T flag as a fallback
270+
# This handles the case where the process might have already exited
271+
if result.returncode != 0:
272+
await anyio.to_thread.run_sync(
273+
subprocess.run,
274+
["taskkill", "/F", "/PID", str(pid)],
275+
capture_output=True,
276+
shell=True,
277+
check=False,
278+
)
310279

311280

312281
async def _terminate_process_with_children(process: Process | FallbackProcess, timeout: float = 2.0) -> None:

test_simple_termination.py

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
"""Simple test to understand Windows process termination"""
2+
import subprocess
3+
import sys
4+
import time
5+
import os
6+
import tempfile
7+
8+
# Test with direct subprocess
9+
print("=== Direct subprocess test ===")
10+
11+
# Create marker file
12+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
13+
marker_file = f.name
14+
15+
# Start a simple child process
16+
child_code = f"""
17+
import time
18+
with open(r'{marker_file}', 'w') as f:
19+
for i in range(100):
20+
f.write(str(i) + ' ')
21+
f.flush()
22+
time.sleep(0.1)
23+
"""
24+
25+
print("Starting child process...")
26+
child = subprocess.Popen([sys.executable, "-c", child_code])
27+
print(f"Child PID: {child.pid}")
28+
29+
# Let it write for a bit
30+
time.sleep(0.5)
31+
size1 = os.path.getsize(marker_file)
32+
print(f"File size after 0.5s: {size1}")
33+
34+
# Terminate child
35+
print("Terminating child...")
36+
child.terminate()
37+
child.wait()
38+
39+
# Check if it stopped
40+
time.sleep(0.5)
41+
size2 = os.path.getsize(marker_file)
42+
print(f"File size after termination: {size2}")
43+
44+
if size2 > size1:
45+
print("SUCCESS: Direct child termination works")
46+
47+
# Cleanup
48+
os.unlink(marker_file)
49+
50+
print("\n=== Nested subprocess test ===")
51+
52+
# Now test with nested processes (parent spawns child)
53+
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
54+
marker_file = f.name
55+
56+
parent_code = f"""
57+
import subprocess
58+
import sys
59+
import time
60+
61+
child_code = '''
62+
import time
63+
with open(r'{marker_file}', 'w') as f:
64+
for i in range(100):
65+
f.write(str(i) + ' ')
66+
f.flush()
67+
time.sleep(0.1)
68+
'''
69+
70+
# Start child
71+
child = subprocess.Popen([sys.executable, '-c', child_code])
72+
print(f'Parent started child with PID: {{child.pid}}')
73+
74+
# Parent just waits
75+
while True:
76+
time.sleep(0.1)
77+
"""
78+
79+
print("Starting parent process...")
80+
parent = subprocess.Popen([sys.executable, "-c", parent_code])
81+
print(f"Parent PID: {parent.pid}")
82+
83+
# Let child write
84+
time.sleep(1.0)
85+
size1 = os.path.getsize(marker_file)
86+
print(f"File size after 1s: {size1}")
87+
88+
# Terminate parent only
89+
print("Terminating parent...")
90+
parent.terminate()
91+
92+
# Check if child stopped
93+
time.sleep(0.5)
94+
size2 = os.path.getsize(marker_file)
95+
time.sleep(0.5)
96+
size3 = os.path.getsize(marker_file)
97+
98+
if size3 > size2:
99+
print(f"FAILED: Child still writing ({size2} -> {size3})")
100+
print("This confirms that terminating parent doesn't kill children on Windows")
101+
else:
102+
print("SUCCESS: Child stopped with parent")
103+
104+
# Cleanup
105+
try:
106+
# Kill any remaining processes
107+
subprocess.run(["taskkill", "/F", "/IM", "python.exe", "/FI", f"PID ne {os.getpid()}"],
108+
capture_output=True, shell=True)
109+
os.unlink(marker_file)
110+
except:
111+
pass

0 commit comments

Comments
 (0)