Skip to content

Commit 37c28c6

Browse files
committed
fix: bound the shielded exit-stack teardown in ServerRunner.run()
The exit_stack unwind after dispatcher.run() exits must stay shielded: without the shield, outer cancellation (server shutdown, SHTTP idle timeout) would skip per-connection cleanup registered by handlers and middleware. But an unbounded shield means one misbehaving cleanup callback that never returns wedges shutdown forever. Replace the bare CancelScope(shield=True) with move_on_after(_EXIT_STACK_CLOSE_TIMEOUT, shield=True) (5 seconds) so well-behaved cleanup still runs to completion while a hung callback is abandoned at the deadline. Log a warning when the grace period expires so abandoned callbacks are visible rather than silent.
1 parent 0d36610 commit 37c28c6

2 files changed

Lines changed: 70 additions & 3 deletions

File tree

‎src/mcp/server/runner.py‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@
6363

6464
_INIT_EXEMPT: frozenset[str] = frozenset({"ping"})
6565

66+
_EXIT_STACK_CLOSE_TIMEOUT: float = 5
67+
"""Grace period in seconds for `connection.exit_stack` teardown in `run()`.
68+
69+
The unwind is shielded from outer cancellation so per-connection cleanup runs
70+
even when the server is being torn down; the shield must be bounded or a
71+
misbehaving cleanup callback would hang shutdown indefinitely."""
72+
6673

6774
def _extract_meta(params: Mapping[str, Any] | None) -> RequestParamsMeta | None:
6875
"""Lift `_meta` from raw params with the same key-aliasing pydantic applies.
@@ -194,13 +201,15 @@ async def run(self, *, task_status: anyio.abc.TaskStatus[None] = anyio.TASK_STAT
194201
to `dispatcher.run()`. `task_status.started()` is forwarded so callers
195202
can `await tg.start(runner.run)` and resume once the dispatcher is
196203
ready to accept requests. Once the dispatcher exits,
197-
`connection.exit_stack` is unwound (shielded) so any per-connection
198-
cleanup registered by handlers or middleware runs to completion.
204+
`connection.exit_stack` is unwound (shielded from outer cancellation,
205+
bounded by `_EXIT_STACK_CLOSE_TIMEOUT`) so any per-connection cleanup
206+
registered by handlers or middleware gets a chance to run without a
207+
misbehaving callback hanging shutdown indefinitely.
199208
"""
200209
try:
201210
await self.dispatcher.run(self._compose_on_request(), self._on_notify, task_status=task_status)
202211
finally:
203-
with anyio.CancelScope(shield=True):
212+
with anyio.move_on_after(_EXIT_STACK_CLOSE_TIMEOUT, shield=True) as scope:
204213
try:
205214
await self.connection.exit_stack.aclose()
206215
except Exception:
@@ -210,6 +219,11 @@ async def run(self, *, task_status: anyio.abc.TaskStatus[None] = anyio.TASK_STAT
210219
# exception from `dispatcher.run()` (including the
211220
# CancelledError that SHTTP idle-timeout teardown checks).
212221
logger.exception("connection exit_stack cleanup raised")
222+
if scope.cancelled_caught:
223+
logger.warning(
224+
"connection exit_stack cleanup exceeded %s seconds; abandoning remaining callbacks",
225+
_EXIT_STACK_CLOSE_TIMEOUT,
226+
)
213227

214228
def _compose_on_request(self) -> OnRequest:
215229
"""Wrap `_on_request` in `dispatch_middleware`, outermost-first.

‎tests/server/test_runner.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import pytest
1515
from opentelemetry.trace import SpanKind, StatusCode
1616

17+
import mcp.server.runner
1718
from mcp.server.context import ServerRequestContext
1819
from mcp.server.lowlevel.server import NotificationOptions, Server
1920
from mcp.server.models import InitializationOptions
@@ -775,3 +776,55 @@ async def _boom() -> None:
775776
await client.send_raw_request("tools/list", None)
776777
assert cleaned == ["ok"]
777778
assert "connection exit_stack cleanup raised" in caplog.text
779+
780+
781+
@pytest.mark.anyio
782+
async def test_runner_exit_stack_blocking_cleanup_abandoned_after_grace(
783+
server: SrvT, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
784+
) -> None:
785+
"""A cleanup callback that never returns is abandoned once the grace period
786+
elapses: `run()` exits, later callbacks in the unwind are cancelled at
787+
their first checkpoint, and a warning is logged. Grace 0 means the deadline
788+
is already expired on entry, so the abandonment is immediate."""
789+
monkeypatch.setattr(mcp.server.runner, "_EXIT_STACK_CLOSE_TIMEOUT", 0)
790+
ran: list[str] = []
791+
release = anyio.Event()
792+
793+
async def _abandoned() -> None:
794+
# LIFO unwind: pushed first, so it runs after the blocker. By then the
795+
# deadline has fired, so this checkpoint raises and the append never runs.
796+
await anyio.sleep(0)
797+
ran.append("abandoned")
798+
799+
async def _blocker() -> None:
800+
ran.append("blocker started")
801+
await release.wait()
802+
ran.append("blocker finished")
803+
804+
async with connected_runner(server) as (client, runner):
805+
runner.connection.exit_stack.push_async_callback(_abandoned)
806+
runner.connection.exit_stack.push_async_callback(_blocker)
807+
await client.send_raw_request("tools/list", None)
808+
assert ran == ["blocker started"]
809+
assert "abandoning remaining callbacks" in caplog.text
810+
811+
812+
@pytest.mark.anyio
813+
async def test_runner_exit_stack_fast_cleanup_completes_within_grace(
814+
server: SrvT, caplog: pytest.LogCaptureFixture
815+
) -> None:
816+
"""Well-behaved cleanup callbacks run to completion under the bounded
817+
unwind and no abandonment warning is logged. Uses the production grace;
818+
the deadline never delays a fast unwind, it only bounds a hung one."""
819+
cleaned: list[int] = []
820+
821+
async def _append(i: int) -> None:
822+
await anyio.sleep(0)
823+
cleaned.append(i)
824+
825+
async with connected_runner(server) as (client, runner):
826+
for i in (1, 2):
827+
runner.connection.exit_stack.push_async_callback(_append, i)
828+
await client.send_raw_request("tools/list", None)
829+
assert cleaned == [2, 1]
830+
assert "abandoning remaining callbacks" not in caplog.text

0 commit comments

Comments
 (0)