Skip to content

Commit 1a6c19c

Browse files
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 <shivam.singh.koyad2006@gmail.com> Github-Issue: #2416
1 parent ba33472 commit 1a6c19c

2 files changed

Lines changed: 32 additions & 2 deletions

File tree

src/mcp/shared/session.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ async def respond(self, response: SendResultT | ErrorData) -> None:
122122
Must be called within a context manager block.
123123
Raises:
124124
RuntimeError: If not used within a context manager
125-
AssertionError: If request was already responded to
126125
"""
127126
if not self._entered: # pragma: no cover
128127
raise RuntimeError("RequestResponder must be used as a context manager")
129-
assert not self._completed, "Request already responded to"
128+
if self._completed:
129+
return
130130

131131
if not self.cancelled: # pragma: no branch
132132
self._completed = True
@@ -141,6 +141,8 @@ async def cancel(self) -> None:
141141
raise RuntimeError("RequestResponder must be used as a context manager")
142142
if not self._cancel_scope: # pragma: no cover
143143
raise RuntimeError("No active cancel scope")
144+
if self._completed: # pragma: no cover
145+
return
144146

145147
self._cancel_scope.cancel()
146148
self._completed = True # Mark as completed so it's removed from in_flight

tests/shared/test_session.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from collections.abc import AsyncGenerator
22
from typing import Any
3+
from unittest import mock
34

45
import anyio
56
import pytest
@@ -10,6 +11,7 @@
1011
from mcp.shared.exceptions import McpError
1112
from mcp.shared.memory import create_client_server_memory_streams, create_connected_server_and_client_session
1213
from mcp.shared.message import SessionMessage
14+
from mcp.shared.session import RequestResponder
1315
from mcp.types import (
1416
CancelledNotification,
1517
CancelledNotificationParams,
@@ -339,3 +341,29 @@ async def mock_server():
339341
await ev_closed.wait()
340342
with anyio.fail_after(1): # pragma: no cover
341343
await ev_response.wait()
344+
345+
346+
@pytest.mark.anyio
347+
async def test_respond_after_cancel_does_not_raise():
348+
"""
349+
Test that calling respond() after cancel() does not raise AssertionError.
350+
351+
Regression test for https://github.com/modelcontextprotocol/python-sdk/issues/2416.
352+
When a CancelledNotification arrives after the handler returns but before
353+
respond() is called, both cancel() and respond() see _completed=False.
354+
cancel() sets it to True and sends the cancellation error; respond() should
355+
silently return instead of asserting.
356+
"""
357+
mock_session: Any = mock.AsyncMock()
358+
mock_session._send_response = mock.AsyncMock()
359+
request = JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/call")
360+
responder: RequestResponder[Any, Any] = RequestResponder(
361+
request_id=1,
362+
request_meta=None,
363+
request=request, # type: ignore[reportArgumentType]
364+
session=mock_session,
365+
on_complete=lambda r: None,
366+
)
367+
with responder:
368+
await responder.cancel()
369+
await responder.respond(ErrorData(code=0, message="ok"))

0 commit comments

Comments
 (0)