Skip to content

Commit 34d4f74

Browse files
committed
Honor the cancel-silence commitment in the shutdown drain
The dispatcher's shutdown arm wrote the CONNECTION_CLOSED drain answer for every in-flight request it caught unwinding, including one whose peer had already cancelled it and whose loop layer had committed it to 2026 cancel semantics - where no frame of any kind may follow notifications/cancelled. A silence-committed peer-cancelled request caught at loop shutdown now stays silent. The rule lives in one predicate on the dispatch context, must_stay_silent, consulted by both frame-suppressing sites (_answer_error and the shutdown drain) so it cannot drift between them; the interrupt arm remains its complementary write side. Requests the peer did NOT cancel keep the drain answer regardless of the commitment - the EOF-drain doctrine is unchanged.
1 parent 7c6aba7 commit 34d4f74

2 files changed

Lines changed: 80 additions & 4 deletions

File tree

src/mcp/shared/jsonrpc_dispatcher.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,20 @@ def request_id(self) -> RequestId | None:
179179
def can_send_request(self) -> bool:
180180
return self.transport.can_send_request and not self._closed
181181

182+
@property
183+
def must_stay_silent(self) -> bool:
184+
"""The peer cancelled this silence-committed request: nothing may be written for it.
185+
186+
Consulted by both frame-suppressing sites - `_answer_error` and the
187+
shutdown drain. The interrupt arm expresses the same fact from the
188+
write side: within `scope.cancelled_caught`, `cancel_requested` is
189+
definitionally set, so its `cancel_answer is not None` check is
190+
exactly `not must_stay_silent`. Once true, result, error, and drain
191+
frames for this id are all forbidden - the transport spec's MUST NOT
192+
after `notifications/cancelled`.
193+
"""
194+
return self.cancel_answer is None and self.cancel_requested.is_set()
195+
182196
async def notify(self, method: str, params: Mapping[str, Any] | None, opts: CallOptions | None = None) -> None:
183197
if self._closed:
184198
logger.debug("dropped %s: dispatch context closed", method)
@@ -796,9 +810,13 @@ async def _handle_request(
796810
except anyio.get_cancelled_exc_class():
797811
# Shutdown: answer the request so the peer isn't left waiting - unless
798812
# an answer write already started (it may have reached the transport;
799-
# prefer possibly-zero answers over possibly-two). The shielded helper
800-
# is needed because bare awaits re-raise here.
801-
if not answer_write_started:
813+
# prefer possibly-zero answers over possibly-two), or the peer already
814+
# cancelled a silence-committed request (the 2026 rule forbids ANY
815+
# frame after the cancel, the shutdown drain included; requests the
816+
# peer did NOT cancel keep their drain answer regardless of the
817+
# commitment). The shielded helper is needed because bare awaits
818+
# re-raise here.
819+
if not answer_write_started and not dctx.must_stay_silent:
802820
await self._final_write(
803821
partial(self._write_error, req.id, ErrorData(code=CONNECTION_CLOSED, message="Connection closed")),
804822
shield=True,
@@ -848,7 +866,7 @@ async def _answer_error(
848866
failure was computed before the cancellation interrupt could land (a
849867
synchronous raise after the cancel arrived, or a shielded handler).
850868
"""
851-
if dctx.cancel_answer is None and dctx.cancel_requested.is_set():
869+
if dctx.must_stay_silent:
852870
logger.debug("dropped error for %r: peer cancelled a silence-committed request", request_id)
853871
return
854872
await self._write_error(request_id, error)

tests/shared/test_jsonrpc_dispatcher.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,64 @@ async def on_request(ctx: DCtx, method: str, params: Mapping[str, Any] | None) -
308308
assert await client.send_raw_request("m", None) == {"ok": True}
309309

310310

311+
@pytest.mark.anyio
312+
async def test_shutdown_drain_stays_silent_for_a_peer_cancelled_silence_committed_request():
313+
"""Loop shutdown catching a silence-committed request whose peer already
314+
cancelled writes NOTHING for it: the 2026 rule forbids any frame after
315+
the cancel, the CONNECTION_CLOSED drain answer included. Requests the
316+
peer did NOT cancel keep the drain answer regardless of the commitment
317+
(see test_shutdown_answers_in_flight_request_with_connection_closed)."""
318+
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](32)
319+
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](32)
320+
server: JSONRPCDispatcher[TransportContext] = JSONRPCDispatcher(c2s_recv, s2c_send)
321+
handler_started = anyio.Event()
322+
release_shield = anyio.Event()
323+
324+
async def on_request(ctx: DCtx, method: str, params: Mapping[str, Any] | None) -> dict[str, Any]:
325+
suppress_cancel_answer(ctx)
326+
handler_started.set()
327+
# Survive the peer-cancel interrupt inside the shield, then hold the
328+
# request in flight until the loop shutdown catches it unwinding.
329+
with anyio.CancelScope(shield=True):
330+
await ctx.cancel_requested.wait()
331+
await release_shield.wait()
332+
await anyio.sleep_forever()
333+
raise NotImplementedError
334+
335+
async def on_notify(ctx: DCtx, method: str, params: Mapping[str, Any] | None) -> None:
336+
pass
337+
338+
late: list[SessionMessage | Exception] = []
339+
try:
340+
async with anyio.create_task_group() as tg:
341+
await tg.start(server.run, on_request, on_notify)
342+
with anyio.fail_after(5):
343+
await c2s_send.send(SessionMessage(message=JSONRPCRequest(jsonrpc="2.0", id=7, method="slow")))
344+
await handler_started.wait()
345+
await c2s_send.send(
346+
SessionMessage(
347+
message=JSONRPCNotification(
348+
jsonrpc="2.0", method="notifications/cancelled", params={"requestId": 7}
349+
)
350+
)
351+
)
352+
await anyio.wait_all_tasks_blocked()
353+
# The handler observed the cancel inside its shield; shut the
354+
# loop down while the request is still in flight.
355+
tg.cancel_scope.cancel()
356+
release_shield.set()
357+
while True:
358+
try:
359+
late.append(s2c_recv.receive_nowait())
360+
except (anyio.WouldBlock, anyio.EndOfStream, anyio.ClosedResourceError):
361+
break
362+
finally:
363+
for s in (c2s_send, c2s_recv, s2c_send, s2c_recv):
364+
s.close()
365+
# Total silence for the cancelled id: no drain frame at shutdown.
366+
assert late == []
367+
368+
311369
@pytest.mark.anyio
312370
async def test_peer_cancel_landing_after_handlers_last_checkpoint_writes_only_the_result():
313371
"""A peer cancel that fails to interrupt the handler writes only the result: one answer per

0 commit comments

Comments
 (0)