Skip to content

Commit 7c6aba7

Browse files
committed
Document and pin the accepted orphaned-modern-lock corner
A peer cancel that lands at the first success frame's own transport-write checkpoint - after the era lock committed, before the frame delivered - leaves a dual-era connection locked modern with ZERO frames on the wire. era_settles already re-checks cancel_requested at observer fire time, so this one-checkpoint window is the entire residue, and it exists only for mid-handler frames (the listen ack, progress): a plain request's response cannot be cancelled away because its in-flight entry is removed, with no checkpoint in between, before the response write starts. This is a deliberate, documented deviation from the no-frame-no-lock doctrine: closing it fully requires committing the lock only after the write is known delivered, which reopens the worse pipelined-initialize hijack the at-first-frame lock exists to prevent. The orphaned state is self-consistent and client-recoverable - only a validly-classified modern envelope reaches this path, a follow-up initialize is answered with -32022 naming the modern versions (released auto-negotiating clients treat that as modern evidence and re-probe), and the connection keeps serving modern traffic. Document the corner on serve_dual_era_loop and commit_modern, and pin it with raw-frame adjacency tests: the orphan itself (locked, silent, recoverable, slot freed), the ack beating an adjacent pipelined initialize, and the plain-request path that cannot orphan.
1 parent 8ca1fc6 commit 7c6aba7

2 files changed

Lines changed: 224 additions & 8 deletions

File tree

src/mcp/server/runner.py

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -595,8 +595,33 @@ async def serve_dual_era_loop(
595595
notifications sent in that window to be dropped. The lock settles exactly
596596
once: a request from the other era that was already in flight when the
597597
lock committed may still complete and its response stands, but the era
598-
does not move; and a success the peer cancelled away before any frame
599-
went out does not lock either.
598+
does not move; and a peer cancel observed before the first frame's write
599+
begins prevents the lock (the commit re-checks `cancel_requested` at fire
600+
time).
601+
602+
One residual corner is accepted, deliberately: the lock commits
603+
immediately before the frame's transport write, and that write's own
604+
checkpoint is the first point a pending peer cancel can land - so a
605+
cancel arriving DURING the write cancels the frame away after the lock
606+
already committed, leaving the connection locked modern with ZERO frames
607+
delivered. Only mid-handler frames (the listen ack, progress) have this
608+
window; a plain request's response cannot be cancelled away (its
609+
in-flight entry is removed before the response write starts, with no
610+
checkpoint in between, so a peer cancel no longer reaches it). The
611+
converse exists too: a handler that shields its own send past a pending
612+
cancel delivers a frame WITHOUT the lock - `era_settles` declined at fire
613+
time - but that frame is output the handler forced after the cancel said
614+
stop, not a state this loop produces. The
615+
orphaned lock is self-consistent and client-recoverable: only a
616+
validly-classified modern envelope reaches this path, so the peer already
617+
committed to modern; a follow-up `initialize` is answered with
618+
UNSUPPORTED_PROTOCOL_VERSION (-32022) naming the modern versions, which
619+
released auto-negotiating clients treat as modern evidence and re-probe;
620+
and the connection keeps serving modern traffic. Closing the corner would
621+
require committing the lock only after the write is known delivered,
622+
which reopens the worse race this design exists to prevent (an
623+
`initialize` pipelined behind an ack the client already read, flipping a
624+
live stream legacy).
600625
601626
Classified-modern requests are also governed by the 2026 cancellation
602627
rule for their whole lifetime: after an inbound `notifications/cancelled`
@@ -629,12 +654,13 @@ def era_settles(dctx: DispatchContext[TransportContext]) -> bool:
629654
return era == "unlocked" and not dctx.cancel_requested.is_set()
630655

631656
def commit_modern(dctx: DispatchContext[TransportContext], version: str) -> None:
632-
# The success-frame observer for classified-modern requests: the
633-
# dispatcher invokes it immediately before each non-error frame for
634-
# the request (ack/event notifications, progress, or the result),
635-
# with no checkpoint before the write - so by the time the client can
636-
# read any modern output, the era is already locked and a pipelined
637-
# `initialize` is rejected instead of hijacking the connection.
657+
# The success-frame observer for classified-modern requests: invoked
658+
# immediately before each non-error frame's write, with no checkpoint
659+
# in between - by the time the client can read any modern output, the
660+
# era is locked and a pipelined `initialize` is rejected instead of
661+
# hijacking the connection. `era_settles` re-checks `cancel_requested`
662+
# at fire time; the cancel-during-the-write residue is the accepted
663+
# corner documented on `serve_dual_era_loop`.
638664
nonlocal era, modern_version
639665
if era_settles(dctx):
640666
era, modern_version = "modern", version

tests/server/test_runner.py

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,19 @@
3131
ErrorData,
3232
Implementation,
3333
InitializeRequestParams,
34+
JSONRPCError,
35+
JSONRPCNotification,
3436
JSONRPCRequest,
37+
JSONRPCResponse,
3538
ListToolsResult,
3639
NotificationParams,
3740
PaginatedRequestParams,
3841
ProgressNotificationParams,
42+
RequestId,
3943
RequestParams,
4044
SetLevelRequestParams,
4145
Tool,
46+
UnsupportedProtocolVersionErrorData,
4247
)
4348
from mcp_types.version import (
4449
LATEST_HANDSHAKE_VERSION,
@@ -1466,6 +1471,57 @@ async def on_notify(dctx: DispatchContext[TransportContext], method: str, params
14661471
return on_notify, send, recv
14671472

14681473

1474+
def _raw_req(rid: RequestId, method: str, params: dict[str, Any] | None = None) -> SessionMessage:
1475+
return SessionMessage(message=JSONRPCRequest(jsonrpc="2.0", id=rid, method=method, params=params))
1476+
1477+
1478+
def _raw_note(method: str, params: dict[str, Any] | None = None) -> SessionMessage:
1479+
return SessionMessage(message=JSONRPCNotification(jsonrpc="2.0", method=method, params=params))
1480+
1481+
1482+
async def _expect_ack(s2c: MemoryObjectReceiveStream[SessionMessage], subscription_id: RequestId) -> None:
1483+
"""Receive the next frame and assert it is the ack stamped with `subscription_id`."""
1484+
ack = (await s2c.receive()).message
1485+
assert isinstance(ack, JSONRPCNotification)
1486+
assert ack.method == ACK_METHOD
1487+
assert ack.params is not None
1488+
assert ack.params["_meta"][SUBSCRIPTION_ID_META_KEY] == subscription_id
1489+
1490+
1491+
@asynccontextmanager
1492+
async def raw_dual_era_loop(
1493+
server: SrvT,
1494+
) -> AsyncIterator[
1495+
tuple[MemoryObjectSendStream[SessionMessage | Exception], MemoryObjectReceiveStream[SessionMessage]]
1496+
]:
1497+
"""Yield raw `(c2s_send, s2c_recv)` streams around a live `serve_dual_era_loop`.
1498+
1499+
For frame-adjacency tests: `send_nowait` places frames in the loop's
1500+
buffer back-to-back, so races are driven at the wire with no client
1501+
dispatcher reordering between them. Assert absence with
1502+
`anyio.wait_all_tasks_blocked()` + `receive_nowait` (every frame the
1503+
server will ever write for the consumed input is buffered once all tasks
1504+
are parked), never with timed drains.
1505+
"""
1506+
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](32)
1507+
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage](32)
1508+
body_exc: BaseException | None = None
1509+
try:
1510+
async with anyio.create_task_group() as tg:
1511+
tg.start_soon(partial(serve_dual_era_loop, server, c2s_recv, s2c_send, lifespan_state=_LIFESPAN))
1512+
try:
1513+
with anyio.fail_after(5):
1514+
yield c2s_send, s2c_recv
1515+
except BaseException as e:
1516+
body_exc = e
1517+
tg.cancel_scope.cancel()
1518+
if body_exc is not None:
1519+
raise body_exc
1520+
finally:
1521+
for stream in (c2s_send, c2s_recv, s2c_send, s2c_recv):
1522+
stream.close()
1523+
1524+
14691525
@pytest.mark.anyio
14701526
async def test_dual_era_loop_serves_listen_and_locks_the_era_at_the_ack():
14711527
"""The full listen lifecycle over the stream loop: the request stays
@@ -1919,6 +1975,132 @@ async def modern_call() -> None:
19191975
assert outcome == []
19201976

19211977

1978+
@pytest.mark.anyio
1979+
async def test_dual_era_loop_cancel_racing_the_listen_ack_write_pins_the_accepted_orphaned_modern_lock():
1980+
"""ACCEPTED DOCTRINE DEVIATION, pinned deliberately: a peer cancel landing
1981+
at the ack write's own checkpoint - after `commit_modern` ran, before the
1982+
frame delivered - leaves the era locked modern with ZERO frames on the
1983+
wire. `era_settles` re-checks `cancel_requested` at fire time, so this is
1984+
the only remaining window; closing it entirely needs a post-write commit,
1985+
which reopens the pipelined-initialize hijack the at-first-frame lock
1986+
exists to prevent (see the residual-corner paragraph on
1987+
`serve_dual_era_loop`). What this test pins is that the orphaned state is
1988+
self-consistent and client-recoverable: total silence for the cancelled
1989+
listen, its subscription slot freed, a follow-up initialize answered with
1990+
-32022 naming the modern versions (released auto-negotiating clients
1991+
treat that as modern evidence and re-probe), and the connection still
1992+
serving modern traffic. Only a validly-classified modern envelope reaches
1993+
this path, so the peer already committed to modern and cannot be
1994+
bricked.
1995+
1996+
Steps:
1997+
1. listen + cancel adjacent in the buffer -> zero frames delivered.
1998+
2. initialize -> -32022 with the typed payload (era IS locked).
1999+
3. fresh listen acks (slot freed) and resolves on graceful close.
2000+
"""
2001+
bus = InMemorySubscriptionBus()
2002+
handler = ListenHandler(bus, max_subscriptions=1)
2003+
srv: SrvT = Server(name="listen-server", version="0.0.1", on_subscriptions_listen=handler)
2004+
async with raw_dual_era_loop(srv) as (c2s, s2c):
2005+
# Adjacent in the buffer before the server reads either: the loop
2006+
# spawns the listen handler, the handler commits the modern lock
2007+
# immediately before the ack write, the write's checkpoint yields to
2008+
# the read loop, and the read loop's cancel then cancels the ack away.
2009+
c2s.send_nowait(_raw_req("L", "subscriptions/listen", _modern_listen_params()))
2010+
c2s.send_nowait(_raw_note("notifications/cancelled", {"requestId": "L"}))
2011+
await anyio.wait_all_tasks_blocked()
2012+
# Zero frames delivered: no ack, and no response/error for the
2013+
# cancelled listen (the 2026 silence rule).
2014+
with pytest.raises(anyio.WouldBlock):
2015+
s2c.receive_nowait()
2016+
# The era is locked modern despite the empty wire: initialize gets the
2017+
# typed -32022 rejection naming what the connection serves.
2018+
c2s.send_nowait(_raw_req(2, "initialize", _initialize_params()))
2019+
init_answer = (await s2c.receive()).message
2020+
assert isinstance(init_answer, JSONRPCError)
2021+
assert init_answer.id == 2
2022+
assert init_answer.error.code == UNSUPPORTED_PROTOCOL_VERSION
2023+
assert init_answer.error.data == UnsupportedProtocolVersionErrorData(
2024+
supported=list(MODERN_PROTOCOL_VERSIONS), requested=LATEST_HANDSHAKE_VERSION
2025+
).model_dump(mode="json")
2026+
# Recoverable: the cancelled listen's max_subscriptions=1 slot was
2027+
# freed, and the connection serves modern traffic - a new listen acks
2028+
# and resolves gracefully.
2029+
c2s.send_nowait(_raw_req("L2", "subscriptions/listen", _modern_listen_params()))
2030+
await _expect_ack(s2c, "L2")
2031+
handler.close()
2032+
result = (await s2c.receive()).message
2033+
assert isinstance(result, JSONRPCResponse)
2034+
assert result.id == "L2"
2035+
2036+
2037+
@pytest.mark.anyio
2038+
async def test_dual_era_loop_initialize_pipelined_adjacent_behind_a_listen_cannot_hijack_the_stream():
2039+
"""listen + initialize adjacent in the loop's buffer, before the ack write
2040+
could complete: the modern lock commits immediately before the ack write
2041+
starts, and initialize runs inline only after the read loop resumes - so
2042+
modern wins, the initialize is rejected with -32022, the rejection is
2043+
sticky, and the listen stream stays live underneath it."""
2044+
bus = InMemorySubscriptionBus()
2045+
handler = ListenHandler(bus)
2046+
srv: SrvT = Server(name="listen-server", version="0.0.1", on_subscriptions_listen=handler)
2047+
async with raw_dual_era_loop(srv) as (c2s, s2c):
2048+
c2s.send_nowait(_raw_req("L", "subscriptions/listen", _modern_listen_params()))
2049+
c2s.send_nowait(_raw_req(2, "initialize", _initialize_params()))
2050+
# Exactly two frames, in either write order: the ack for the live
2051+
# listen and the -32022 for the hijack attempt. Nothing for id "L".
2052+
frames = [(await s2c.receive()).message, (await s2c.receive()).message]
2053+
(ack,) = [f for f in frames if isinstance(f, JSONRPCNotification)]
2054+
(init_answer,) = [f for f in frames if isinstance(f, JSONRPCError)]
2055+
assert ack.method == ACK_METHOD
2056+
assert ack.params is not None
2057+
assert ack.params["_meta"][SUBSCRIPTION_ID_META_KEY] == "L"
2058+
assert init_answer.id == 2
2059+
assert init_answer.error.code == UNSUPPORTED_PROTOCOL_VERSION
2060+
# Sticky: a retried initialize is rejected identically.
2061+
c2s.send_nowait(_raw_req(3, "initialize", _initialize_params()))
2062+
follow = (await s2c.receive()).message
2063+
assert isinstance(follow, JSONRPCError)
2064+
assert follow.id == 3
2065+
assert follow.error.code == UNSUPPORTED_PROTOCOL_VERSION
2066+
# The stream the initialize tried to hijack is genuinely live.
2067+
await bus.publish(ToolsListChanged())
2068+
event = (await s2c.receive()).message
2069+
assert isinstance(event, JSONRPCNotification)
2070+
assert event.method == "notifications/tools/list_changed"
2071+
assert event.params is not None
2072+
assert event.params["_meta"][SUBSCRIPTION_ID_META_KEY] == "L"
2073+
handler.close()
2074+
result = (await s2c.receive()).message
2075+
assert isinstance(result, JSONRPCResponse)
2076+
assert result.id == "L"
2077+
2078+
2079+
@pytest.mark.anyio
2080+
async def test_dual_era_loop_adjacent_cancel_behind_a_fast_modern_request_cannot_orphan_the_lock():
2081+
"""The orphaned-lock corner is confined to mid-handler frames (ack,
2082+
progress): a plain request's in-flight entry is removed before its
2083+
response write starts, with no checkpoint in between, so an adjacent
2084+
cancel consumed during the write no longer reaches the request - the
2085+
response is delivered and the era locks WITH a frame on the wire."""
2086+
2087+
async def list_tools(ctx: Ctx, params: PaginatedRequestParams | None) -> ListToolsResult:
2088+
return ListToolsResult(tools=[Tool(name="t", input_schema={"type": "object"})])
2089+
2090+
srv: SrvT = Server(name="fast-server", version="0.0.1", on_list_tools=list_tools)
2091+
async with raw_dual_era_loop(srv) as (c2s, s2c):
2092+
c2s.send_nowait(_raw_req(5, "tools/list", _modern_params()))
2093+
c2s.send_nowait(_raw_note("notifications/cancelled", {"requestId": 5}))
2094+
result = (await s2c.receive()).message
2095+
assert isinstance(result, JSONRPCResponse)
2096+
assert result.id == 5
2097+
c2s.send_nowait(_raw_req(6, "initialize", _initialize_params()))
2098+
answer = (await s2c.receive()).message
2099+
assert isinstance(answer, JSONRPCError)
2100+
assert answer.id == 6
2101+
assert answer.error.code == UNSUPPORTED_PROTOCOL_VERSION
2102+
2103+
19222104
@pytest.mark.anyio
19232105
async def test_dual_era_loop_maps_unmapped_handler_exceptions_like_the_modern_http_entry():
19242106
"""An unmapped handler exception on a modern request surfaces as the
@@ -2071,3 +2253,11 @@ async def test_dual_era_client_propagates_body_exception_unwrapped(server: SrvT)
20712253
with pytest.raises(RuntimeError, match="boom"):
20722254
async with dual_era_client(server):
20732255
raise RuntimeError("boom")
2256+
2257+
2258+
@pytest.mark.anyio
2259+
async def test_raw_dual_era_loop_propagates_body_exception_unwrapped(server: SrvT):
2260+
"""The raw-frame harness re-raises body exceptions as-is, not as `ExceptionGroup`."""
2261+
with pytest.raises(RuntimeError, match="boom"):
2262+
async with raw_dual_era_loop(server):
2263+
raise RuntimeError("boom")

0 commit comments

Comments
 (0)