Skip to content

Commit 439f5a4

Browse files
fix: bound stdin closure during subprocess shutdown (#152)
Co-authored-by: Oscar <319719771+Oscar-Williams@users.noreply.github.com>
1 parent 835cbed commit 439f5a4

2 files changed

Lines changed: 71 additions & 8 deletions

File tree

‎src/acp/transports.py‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ async def spawn_stdio_transport(
5858
5959
This mirrors the defensive shutdown behaviour used by the MCP Python SDK:
6060
close stdin first, wait for graceful exit, then escalate to terminate/kill.
61+
Stdin closure uses the same timeout as the waits before terminate/kill.
6162
"""
6263
merged_env = dict(default_environment())
6364
if env:
@@ -96,16 +97,9 @@ async def spawn_stdio_transport(
9697
finally:
9798
# Attempt graceful stdin shutdown first
9899
if process.stdin is not None:
99-
try:
100-
process.stdin.write_eof()
101-
except (AttributeError, OSError, RuntimeError):
102-
process.stdin.close()
103-
with contextlib.suppress(Exception):
104-
await process.stdin.drain()
105100
with contextlib.suppress(Exception):
106101
process.stdin.close()
107-
with contextlib.suppress(Exception):
108-
await process.stdin.wait_closed()
102+
await asyncio.wait_for(process.stdin.wait_closed(), timeout=shutdown_timeout)
109103

110104
try:
111105
await asyncio.wait_for(process.wait(), timeout=shutdown_timeout)
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import asyncio
2+
import contextlib
3+
import signal
4+
import sys
5+
6+
import pytest
7+
8+
from acp.transports import spawn_stdio_transport
9+
10+
11+
@pytest.mark.asyncio
12+
@pytest.mark.parametrize("ignore_terminate", [False, True])
13+
async def test_shutdown_with_unread_stdin(ignore_terminate: bool) -> None:
14+
if ignore_terminate and sys.platform == "win32":
15+
pytest.skip("Windows terminate() cannot be ignored")
16+
script = "import signal, time; "
17+
if ignore_terminate:
18+
script += "signal.signal(signal.SIGTERM, signal.SIG_IGN); "
19+
script += "print('ready', flush=True); time.sleep(60)"
20+
transport = spawn_stdio_transport(sys.executable, "-c", script, shutdown_timeout=0.1)
21+
reader, writer, process = await transport.__aenter__()
22+
closing = None
23+
try:
24+
assert (await asyncio.wait_for(reader.readline(), timeout=5)).strip() == b"ready"
25+
writer.write(b"x" * (1024 * 1024))
26+
assert writer.transport.get_write_buffer_size() > 0
27+
closing = asyncio.create_task(transport.__aexit__(None, None, None))
28+
await asyncio.wait_for(asyncio.shield(closing), timeout=5)
29+
assert process.returncode is not None
30+
if sys.platform != "win32":
31+
assert process.returncode == -(signal.SIGKILL if ignore_terminate else signal.SIGTERM)
32+
finally:
33+
if process.returncode is None:
34+
process.kill()
35+
await process.wait()
36+
if closing is not None:
37+
await closing
38+
else:
39+
await transport.__aexit__(None, None, None)
40+
41+
42+
@pytest.mark.asyncio
43+
async def test_shutdown_flushes_stdin_before_eof() -> None:
44+
script = "import sys; data = sys.stdin.buffer.read(); print(len(data), flush=True)"
45+
async with spawn_stdio_transport(sys.executable, "-c", script) as (reader, writer, process):
46+
writer.write(b"x" * (1024 * 1024))
47+
assert process.returncode == 0
48+
assert (await reader.readline()).strip() == b"1048576"
49+
50+
51+
@pytest.mark.asyncio
52+
async def test_shutdown_preserves_body_exception() -> None:
53+
with pytest.raises(ValueError, match="body failure"):
54+
async with spawn_stdio_transport(sys.executable, "-c", "import sys; sys.stdin.buffer.read()") as (
55+
_reader,
56+
_writer,
57+
process,
58+
):
59+
raise ValueError("body failure")
60+
assert process.returncode == 0
61+
62+
63+
@pytest.mark.asyncio
64+
async def test_shutdown_after_child_exit() -> None:
65+
async with spawn_stdio_transport(sys.executable, "-c", "pass") as (_reader, writer, process):
66+
await asyncio.wait_for(process.wait(), timeout=5)
67+
with contextlib.suppress(ConnectionError):
68+
await writer.drain()
69+
assert process.returncode == 0

0 commit comments

Comments
 (0)