From 1a6c19cd17cc73e5bddfd66409c8a8d35ac60b12 Mon Sep 17 00:00:00 2001 From: Shivam Singh Date: Sat, 11 Jul 2026 18:37:34 +0530 Subject: [PATCH] fix: guard respond() and cancel() against concurrent cancellation race When a CancelledNotification arrives after the handler has returned but before respond() is called, cancel() sets _completed=True and sends an error response. respond() then hits assert not self._completed and crashes the server. Replace the assert with a silent early return in respond(), and add the same guard to cancel() so duplicate completion is harmless either way. Either path wins exactly once under cooperative multitasking. Fixes #2416 Signed-off-by: Shivam Singh Github-Issue: #2416 --- src/mcp/shared/session.py | 6 ++++-- tests/shared/test_session.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 35a83fcf1b..0cda019d6b 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -122,11 +122,11 @@ async def respond(self, response: SendResultT | ErrorData) -> None: Must be called within a context manager block. Raises: RuntimeError: If not used within a context manager - AssertionError: If request was already responded to """ if not self._entered: # pragma: no cover raise RuntimeError("RequestResponder must be used as a context manager") - assert not self._completed, "Request already responded to" + if self._completed: + return if not self.cancelled: # pragma: no branch self._completed = True @@ -141,6 +141,8 @@ async def cancel(self) -> None: raise RuntimeError("RequestResponder must be used as a context manager") if not self._cancel_scope: # pragma: no cover raise RuntimeError("No active cancel scope") + if self._completed: # pragma: no cover + return self._cancel_scope.cancel() self._completed = True # Mark as completed so it's removed from in_flight diff --git a/tests/shared/test_session.py b/tests/shared/test_session.py index f4010141d8..6f666b67a8 100644 --- a/tests/shared/test_session.py +++ b/tests/shared/test_session.py @@ -1,5 +1,6 @@ from collections.abc import AsyncGenerator from typing import Any +from unittest import mock import anyio import pytest @@ -10,6 +11,7 @@ from mcp.shared.exceptions import McpError from mcp.shared.memory import create_client_server_memory_streams, create_connected_server_and_client_session from mcp.shared.message import SessionMessage +from mcp.shared.session import RequestResponder from mcp.types import ( CancelledNotification, CancelledNotificationParams, @@ -339,3 +341,29 @@ async def mock_server(): await ev_closed.wait() with anyio.fail_after(1): # pragma: no cover await ev_response.wait() + + +@pytest.mark.anyio +async def test_respond_after_cancel_does_not_raise(): + """ + Test that calling respond() after cancel() does not raise AssertionError. + + Regression test for https://github.com/modelcontextprotocol/python-sdk/issues/2416. + When a CancelledNotification arrives after the handler returns but before + respond() is called, both cancel() and respond() see _completed=False. + cancel() sets it to True and sends the cancellation error; respond() should + silently return instead of asserting. + """ + mock_session: Any = mock.AsyncMock() + mock_session._send_response = mock.AsyncMock() + request = JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/call") + responder: RequestResponder[Any, Any] = RequestResponder( + request_id=1, + request_meta=None, + request=request, # type: ignore[reportArgumentType] + session=mock_session, + on_complete=lambda r: None, + ) + with responder: + await responder.cancel() + await responder.respond(ErrorData(code=0, message="ok"))