Skip to content

Serialize subscriptions/listen writes so the graceful result is the final message - #535

Open
koic wants to merge 1 commit into
modelcontextprotocol:mainfrom
koic:serialize_listen_stream_writes_on_teardown
Open

Serialize subscriptions/listen writes so the graceful result is the final message#535
koic wants to merge 1 commit into
modelcontextprotocol:mainfrom
koic:serialize_listen_stream_writes_on_teardown

Conversation

@koic

@koic koic commented Aug 25, 2026

Copy link
Copy Markdown
Member

Motivation and Context

On graceful teardown (transport.close), each open subscriptions/listen stream receives its SubscriptionsListenResult response before closing, signaling a clean end the client can distinguish from an abrupt disconnect. But a delivery that snapshotted the stream from the registry before teardown cleared it performs its write outside the lock, so a change notification could land after the final result.

Each subscription entry now carries a per-stream write mutex. Teardown marks the entry closed and writes the result under that mutex; a racing delivery either lands before the result or observes closed and skips, keeping the graceful result the stream's final message. The mutex is per stream and held only around the single write call, so deliveries to different streams stay as parallel as before, and no path nests it inside the transport mutex (delivery snapshots release @mutex first, and the write-error cleanup takes @mutex only after the write mutex is released), ruling out lock-order inversions. Keepalive pings stay outside the mutex: they are SSE comment frames, which clients ignore by specification, so one landing after the result is harmless.

How Has This Been Tested?

With a new regression test pinning the race (an entry captured before teardown and re-presented to delivery afterwards writes nothing after the result), the full suite, RuboCop, and the conformance suite, all green.

Breaking Changes

None. The write mutex is internal to the listen registry; the wire only gains the ordering guarantee.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

… final message

## Motivation and Context

On graceful teardown (`transport.close`), each open `subscriptions/listen` stream receives
its `SubscriptionsListenResult` response before closing, signaling a clean end the client can distinguish
from an abrupt disconnect. But a delivery that snapshotted the stream from the registry
before teardown cleared it performs its write outside the lock, so a change notification could land after the final result.

Each subscription entry now carries a per-stream write mutex. Teardown marks the entry closed and writes
the result under that mutex; a racing delivery either lands before the result or observes `closed` and skips,
keeping the graceful result the stream's final message. The mutex is per stream and held only around the single write call,
so deliveries to different streams stay as parallel as before, and no path nests it inside the transport mutex
(delivery snapshots release `@mutex` first, and the write-error cleanup takes `@mutex` only after the write mutex is released),
ruling out lock-order inversions. Keepalive pings stay outside the mutex: they are SSE comment frames, which clients ignore by
specification, so one landing after the result is harmless.

## How Has This Been Tested?

With a new regression test pinning the race (an entry captured before teardown and re-presented to delivery afterwards writes
nothing after the result), the full suite, RuboCop, and the conformance suite, all green.

## Breaking Changes

None. The write mutex is internal to the listen registry; the wire only gains the ordering guarantee.
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