Serialize subscriptions/listen writes so the graceful result is the final message - #535
Open
koic wants to merge 1 commit into
Open
Conversation
… 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.
atesgoral
approved these changes
Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
On graceful teardown (
transport.close), each opensubscriptions/listenstream receives itsSubscriptionsListenResultresponse 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
closedand 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@mutexfirst, and the write-error cleanup takes@mutexonly 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
Checklist