Skip to content

mcp: reject resource subscriptions during close - #1172

Open
jstar0 wants to merge 4 commits into
modelcontextprotocol:mainfrom
jstar0:fix/resource-subscribe-close
Open

jstar0 wants to merge 4 commits into
modelcontextprotocol:mainfrom
jstar0:fix/resource-subscribe-close

Conversation

@jstar0

@jstar0 jstar0 commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1171.

This change prevents new SEP-2575 resource subscriptions from racing ClientSession.Close:

  • Close marks the session as closing before cancelling active listen streams.
  • Subscribe checks the close state before and after registering the listen, and clears its local entry if startup fails or close wins the race.
  • Connect treats server rejection of the optional list-changed subscriptions/listen as non-fatal; an explicit resource Subscribe still returns the rejection.

Tests:

  • go test ./... — passed.
  • go test -race ./mcp -run 'TestResourceSubscriptions_SubscribeConcurrentCloseFails$' — passed.
  • go test ./mcp -run 'TestStreamableClient_(StatelessSubscriptionsListen404|ResourceSubscribeListen404ReturnsError)|TestResourceSubscriptions' -count=1 — passed.

@jstar0

jstar0 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased the PR onto current main and updated head 10ab202 after preserving the close-race fix against the latest client lifecycle changes. One compatibility boundary is explicit now: rejection of Connect's optional list-changed listen leaves the session usable, while an explicit resource Subscribe still returns the rejection and clears its registration.

Verification on the current head: go test ./... passed; the focused listen/resource-subscription tests and go test -race ./mcp -run TestResourceSubscriptions_SubscribeConcurrentCloseFails passed; all 9 hosted checks, including both conformance jobs and the race job, passed.

Could you review this current head when convenient?

This branch has not been deployed

No deployments
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.

ClientSession.Subscribe can silently succeed while the session is closing

2 participants