Skip to content

fix: cancel pending request tokens after transport EOF - #1333

Merged
alexhancock merged 1 commit into
modelcontextprotocol:mainfrom
alishobeiri-oai:dev/alishobeiri/cancel-pending-requests-on-close
Oct 9, 2026
Merged

alexhancock merged 1 commit into
modelcontextprotocol:mainfrom
alishobeiri-oai:dev/alishobeiri/cancel-pending-requests-on-close

Conversation

@alishobeiri-oai

Copy link
Copy Markdown
Contributor

Pending request handlers can outlive transport EOF while a caller retains the RunningService or a Peer. Their RequestContext cancellation tokens are children of the service loop token, but the EOF path did not cancel that token.

Cancel the service loop token after the existing response-drain period and before closing the transport. This preserves graceful response draining and wakes remaining handlers without cancelling an externally supplied parent token or sibling services.

The regression sends two requests over a real in-memory transport, leaves both handlers pending, and closes the peer input. It retains the service and peer while asserting that both request tokens are cancelled and both handlers finish. Tokio time is paused so the drain deadline adds no wall-clock delay.

Validation

  • The new regression fails without the service-token cancellation.
  • cargo +1.97.1 test -p rmcp --test test_inflight_response_drain --test test_close_connection --test test_cancelled_response --features "client server transport-io": all 16 tests passed.
  • Formatted the changed files with rustfmt.

@alishobeiri-oai
alishobeiri-oai requested a review from a team as a code owner October 9, 2026 10:47
@github-actions github-actions Bot added T-test Testing related changes T-core Core library changes labels Oct 9, 2026

@alexhancock alexhancock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@alexhancock
alexhancock merged commit 4048ac4 into modelcontextprotocol:main Oct 9, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants