mcp: reject Subscribe on a closed session and roll back failed registrations - #1198
Conversation
1ae76a9 to
5ea7b3c
Compare
|
Rebased onto latest I also noticed no CI has run on this PR since it was opened. I think it needs a maintainer to approve the workflow run, since this is my first contribution to this repo — happy to be told if something else is blocking it instead. On the overlap question: #1221 also touched To confirm the change is actually load-bearing, I ran a deterministic oracle rather than relying on winning the race: Verification on the rebased head, mirroring this repo's CI: Since #1171 isn't labelled |
What
ClientSession.Subscribe(SEP-2575 path) starts asubscriptions/listenstreamwithout awaiting the send result, so on a session that is closing — or when the
listen send fails for any other reason — it can return
nileven though thestream never started. The local
resourceSubsmap also keeps an entry whosecancel func is then never called, leaking the registration and misreporting the
URI as subscribed. Fixes #1171.
How
closedflag toClientSession, set at the top ofClose.Subscribechecks the flag underresourceSubsMubefore registering, so acall after
Closehas started returnsErrConnectionClosedand registersnothing. Because the check and the registration share the mutex with the
Closecleanup sweep, a racing registration is either swept byCloseorrefused outright — no interleaving leaks.
subscriptionsListenreports an error, the registration is rolled back(entry removed and cancel func called), so a later
Subscriberetries thelisten instead of treating the URI as already subscribed.
Unsubscribeand idempotent double-Subscribebehavior are unchanged.Tests
TestResourceSubscriptions_SubscribeClosedSession: Subscribe after Closereturns
ErrConnectionClosedand leaves noresourceSubsentry.TestResourceSubscriptions_SubscribeListenFailureRollsBack: a failed listensend (injected via
AddSendingMiddleware) rolls back the registration and aretry re-attempts the listen.
TestResourceSubscriptions_SubscribeCloseRace: 50 rounds of racingCloseand
Subscribeasserting that a failedSubscribenever leaves an entry.go test -race ./mcp/andgo test ./...pass;gofmt,go vet, andstaticcheckare clean.