Skip to content

Fix early request cancellation race - #270

Open
dariuscorvus wants to merge 1 commit into
modelcontextprotocol:mainfrom
dariuscorvus:fix/early-request-cancellation
Open

dariuscorvus wants to merge 1 commit into
modelcontextprotocol:mainfrom
dariuscorvus:fix/early-request-cancellation

Conversation

@dariuscorvus

Copy link
Copy Markdown

Summary

  • track incoming request dispatch tasks before handler registration so immediate notifications/cancelled messages cannot fall through the registration gap
  • cancel both dispatch and active handler tasks atomically, retain token-based ownership during cleanup, and suppress late responses even when handlers swallow cancellation
  • reject duplicate outstanding request IDs across standalone and batch requests
  • process the built-in cancellation handler before logging and application notification handlers can suspend
  • cancel tracked work during stop() and avoid duplicate built-in cancellation handler registration across restarts

Tests

  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer swift test --filter CancellationTests
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer swift test
  • 554 tests in 40 suites passed
  • git diff --check

Motivation

The receive loop spawns request processing asynchronously. Before this change, a cancellation notification could be consumed before the request's handler task was inserted into pendingRequestTasks, causing the cancellation to be treated as unknown. A deterministic HTTP-context barrier test now covers that exact ordering.

ianegordon pushed a commit to ianegordon/swift-sdk that referenced this pull request Oct 13, 2026
…ests without a reply (upstream modelcontextprotocol#298, EhsanAzish80)

Supersedes the contributions of modelcontextprotocol#264, modelcontextprotocol#260 and modelcontextprotocol#270 (manifest entries 1-3),
which modelcontextprotocol#298 combines. Conflicts resolved in favour of modelcontextprotocol#298 in
HTTPServerTypes.swift, StatelessHTTPServerTransport.swift, Server.swift,
CancellationTests.swift and HTTPServerTransportTests.swift.

Fork changes re-applied on top:
- modelcontextprotocol#258's Server.init description/websiteUrl/icons parameters
- modelcontextprotocol#221's MockTransport receiveCallCount / finishStream
- entry 1a's collision regression tests and their makeStatelessPOSTRequest
  authorization overload

Behavior: a cancelled original POST now ends as HTTP 200 with an empty
text/event-stream and no JSON-RPC body (previously -32002 from modelcontextprotocol#260).
Follow-up on when to use -32800: fork issue #41.

Full suite: 589 tests in 43 suites passed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@varun06

varun06 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Hi @dariuscorvus, thank you for this contribution. We're consolidating overlapping PRs.

#298 includes this fix for the early-cancellation race, rebased on current main and in the file's existing style, together with the related stateless-transport fixes. If #298's HTTP behavior ends up changing, we'll ask to carry your Server.swift part forward separately.

We plan to close this PR once #298 is merged, so the fix isn't lost in between. If you think something here isn't covered by #298, please comment there or here and we'll make sure it's carried over.

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.

2 participants