Skip to content

Commit 4fcba8b

Browse files
Add test demonstrating issue #1027 and verifying PR #1044 fixes it
This test shows that MCP server cleanup code in lifespan doesn't run when the process is terminated, but does run when stdin is closed first (as implemented in PR #1044). The test includes: - Demonstration of current broken behavior (cleanup doesn't run) - Verification that stdin closure allows graceful shutdown - Windows-specific ResourceWarning handling - Detailed documentation of the issue and solution Github-Issue:#1027
1 parent 50559f7 commit 4fcba8b

File tree

1 file changed

+239
-0
lines changed

1 file changed

+239
-0
lines changed
Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
"""
2+
Test for issue #1027: The cleanup procedure after "yield" in lifespan is unreachable on Windows
3+
4+
This test demonstrates that on Windows, when an MCP server is terminated via process.terminate(),
5+
the cleanup code after yield in the lifespan context manager is not executed.
6+
7+
The issue occurs because Windows' TerminateProcess() forcefully kills the process without
8+
allowing cleanup handlers to run.
9+
"""
10+
11+
import asyncio
12+
import sys
13+
import tempfile
14+
import textwrap
15+
from pathlib import Path
16+
17+
import anyio
18+
import pytest
19+
20+
from mcp import ClientSession, StdioServerParameters
21+
from mcp.client.stdio import _create_platform_compatible_process, stdio_client
22+
23+
24+
@pytest.mark.anyio
25+
async def test_lifespan_cleanup_executed():
26+
"""
27+
Test that verifies cleanup code in MCP server lifespan is executed.
28+
29+
This test creates an MCP server that writes to marker files:
30+
1. When the server starts (before yield)
31+
2. When the server cleanup runs (after yield)
32+
33+
On Windows with the current implementation, the cleanup file is never created
34+
because process.terminate() kills the process immediately.
35+
"""
36+
37+
# Create marker files to track server lifecycle
38+
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as f:
39+
startup_marker = f.name
40+
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as f:
41+
cleanup_marker = f.name
42+
43+
# Remove the files so we can detect when they're created
44+
Path(startup_marker).unlink()
45+
Path(cleanup_marker).unlink()
46+
47+
# Create a minimal MCP server using FastMCP that tracks lifecycle
48+
server_code = textwrap.dedent(f"""
49+
import asyncio
50+
import sys
51+
from pathlib import Path
52+
from contextlib import asynccontextmanager
53+
from mcp.server.fastmcp import FastMCP
54+
55+
STARTUP_MARKER = {repr(startup_marker)}
56+
CLEANUP_MARKER = {repr(cleanup_marker)}
57+
58+
@asynccontextmanager
59+
async def lifespan(server):
60+
# Write startup marker
61+
Path(STARTUP_MARKER).write_text("started")
62+
try:
63+
yield {{"started": True}}
64+
finally:
65+
# This cleanup code should run when server shuts down
66+
Path(CLEANUP_MARKER).write_text("cleaned up")
67+
68+
mcp = FastMCP("test-server", lifespan=lifespan)
69+
70+
@mcp.tool()
71+
def echo(text: str) -> str:
72+
return text
73+
74+
if __name__ == "__main__":
75+
mcp.run()
76+
""")
77+
78+
# Write the server script to a temporary file
79+
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".py") as f:
80+
server_script = f.name
81+
f.write(server_code)
82+
83+
try:
84+
# Launch the MCP server
85+
params = StdioServerParameters(command=sys.executable, args=[server_script])
86+
87+
async with stdio_client(params) as (read, write):
88+
async with ClientSession(read, write) as session:
89+
# Initialize the session
90+
result = await session.initialize()
91+
assert result.protocolVersion in ["2024-11-05", "2025-06-18"]
92+
93+
# Verify startup marker was created
94+
assert Path(startup_marker).exists(), "Server startup marker not created"
95+
assert Path(startup_marker).read_text() == "started"
96+
97+
# Make a test request to ensure server is working
98+
response = await session.call_tool("echo", {"text": "hello"})
99+
assert response.content[0].type == "text"
100+
assert getattr(response.content[0], "text") == "hello"
101+
102+
# Session will be closed when exiting the context manager
103+
104+
# Give server a moment to run cleanup (if it can)
105+
await asyncio.sleep(0.5)
106+
107+
# Check if cleanup marker was created
108+
# This currently fails on all platforms because process.terminate()
109+
# doesn't allow cleanup code to run
110+
if not Path(cleanup_marker).exists():
111+
pytest.xfail("Cleanup code after yield is not executed when process is terminated (issue #1027)")
112+
else:
113+
# If cleanup succeeded, the issue may be fixed
114+
assert Path(cleanup_marker).read_text() == "cleaned up"
115+
116+
finally:
117+
# Clean up files
118+
for path in [server_script, startup_marker, cleanup_marker]:
119+
try:
120+
Path(path).unlink()
121+
except FileNotFoundError:
122+
pass
123+
124+
125+
@pytest.mark.anyio
126+
@pytest.mark.filterwarnings("ignore::ResourceWarning" if sys.platform == "win32" else "default")
127+
async def test_stdin_close_triggers_cleanup():
128+
"""
129+
Test that verifies if closing stdin allows cleanup to run.
130+
131+
This is the proposed solution from PR #1044 - close stdin first
132+
and wait for the server to exit gracefully before terminating.
133+
134+
Note on Windows ResourceWarning:
135+
On Windows, we may see ResourceWarning about unclosed file descriptors.
136+
This is expected behavior because:
137+
- We're manually managing the process lifecycle
138+
- Windows file handle cleanup works differently than Unix
139+
- The warning doesn't indicate a real issue - cleanup still works
140+
We filter this warning on Windows only to avoid test noise.
141+
"""
142+
143+
# Create marker files to track server lifecycle
144+
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as f:
145+
startup_marker = f.name
146+
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as f:
147+
cleanup_marker = f.name
148+
149+
# Remove the files so we can detect when they're created
150+
Path(startup_marker).unlink()
151+
Path(cleanup_marker).unlink()
152+
153+
# Create an MCP server that handles stdin closure gracefully
154+
server_code = textwrap.dedent(f"""
155+
import asyncio
156+
import sys
157+
from pathlib import Path
158+
from contextlib import asynccontextmanager
159+
from mcp.server.fastmcp import FastMCP
160+
161+
STARTUP_MARKER = {repr(startup_marker)}
162+
CLEANUP_MARKER = {repr(cleanup_marker)}
163+
164+
@asynccontextmanager
165+
async def lifespan(server):
166+
# Write startup marker
167+
Path(STARTUP_MARKER).write_text("started")
168+
try:
169+
yield {{"started": True}}
170+
finally:
171+
# This cleanup code should run when stdin closes
172+
Path(CLEANUP_MARKER).write_text("cleaned up")
173+
174+
mcp = FastMCP("test-server", lifespan=lifespan)
175+
176+
@mcp.tool()
177+
def echo(text: str) -> str:
178+
return text
179+
180+
if __name__ == "__main__":
181+
# The server should exit gracefully when stdin closes
182+
try:
183+
mcp.run()
184+
except Exception:
185+
# Server might get EOF or other errors when stdin closes
186+
pass
187+
""")
188+
189+
# Write the server script to a temporary file
190+
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".py") as f:
191+
server_script = f.name
192+
f.write(server_code)
193+
194+
try:
195+
# This test manually manages the process to test stdin closure
196+
# Start the server process
197+
process = await _create_platform_compatible_process(
198+
command=sys.executable, args=[server_script], env=None, errlog=sys.stderr, cwd=None
199+
)
200+
201+
# Wait for server to start
202+
await asyncio.sleep(1.0) # Give more time on Windows
203+
204+
# Check if process is still running
205+
if hasattr(process, "returncode") and process.returncode is not None:
206+
pytest.fail(f"Server process exited with code {process.returncode}")
207+
208+
assert Path(startup_marker).exists(), "Server startup marker not created"
209+
210+
# Close stdin to signal shutdown
211+
if process.stdin:
212+
await process.stdin.aclose()
213+
214+
# Wait for process to exit gracefully
215+
try:
216+
with anyio.fail_after(2.0):
217+
await process.wait()
218+
except TimeoutError:
219+
# If it doesn't exit after stdin close, terminate it
220+
process.terminate()
221+
await process.wait()
222+
223+
# Check if cleanup ran
224+
await asyncio.sleep(0.5)
225+
226+
# This should work if the server properly handles stdin closure
227+
if Path(cleanup_marker).exists():
228+
assert Path(cleanup_marker).read_text() == "cleaned up"
229+
# If this works, it shows stdin closure can trigger graceful shutdown
230+
else:
231+
pytest.xfail("Server did not run cleanup after stdin closure - " "may need additional server-side handling")
232+
233+
finally:
234+
# Clean up files
235+
for path in [server_script, startup_marker, cleanup_marker]:
236+
try:
237+
Path(path).unlink()
238+
except FileNotFoundError:
239+
pass

0 commit comments

Comments
 (0)