Skip to content

fix: handle cancellation race in RequestResponder.respond()#2417

Open
lui62233 wants to merge 1 commit intomodelcontextprotocol:mainfrom
lui62233:fix/cancellation-race-1775786164
Open

fix: handle cancellation race in RequestResponder.respond()#2417
lui62233 wants to merge 1 commit intomodelcontextprotocol:mainfrom
lui62233:fix/cancellation-race-1775786164

Conversation

@lui62233
Copy link
Copy Markdown

Fix: Cancellation Race in RequestResponder.respond()

When a CancelledNotification arrives after the handler completes but before
respond() is called, cancel() sets _completed=True and sends an error response.
The subsequent respond() call would hit AssertionError: Request already responded to.

The Fix

Replace the assert not self._completed guard with a silent early-return:

# Before:
assert not self._completed, "Request already responded to"

# After:
# Guard against race: if cancel() already set _completed, skip silently.
if self._completed:
    return

This is safe because cancel() already sent an error response ("Request cancelled"),
so skipping the duplicate response is correct behavior.

Why This Is The Right Fix

  • cancel() is called on a different task (the cancellation notification handler)
  • Between handler completion and respond(), there is no async checkpoint
  • CancelledError cannot be raised in that gap
  • BrokenResourceError/ClosedResourceError are already handled in server.py
  • This race requires handling at the source of _completed

Fixes #2416


Submitted via TTClaw bounty hunter.

When a CancelledNotification arrives after the handler completes but before
respond() is called, cancel() sets _completed=True and sends an error response.
The subsequent respond() call would hit an AssertionError.

This change replaces the assert with a guard: if _completed is already True,
respond() returns silently since the cancellation response was already sent.

Fixes: modelcontextprotocol#2416

_Submitted via TTClaw bounty hunter._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: AssertionError: Request already responded to — cancellation race in v1.27.0

1 participant