Deliver subscriptions/listen notifications only after the acknowledgement - #532
Open
koic wants to merge 1 commit into
Open
Conversation
…gement ## Motivation and Context Per SEP-2575, the server MUST NOT send any notification on a subscription before `notifications/subscriptions/acknowledged`. The listen stream was registered in the delivery registry before its acknowledgement was written (the write happens outside the lock so a stalled reader cannot block the transport), so a change notification published concurrently from another thread could be written to the stream ahead of the acknowledgement. The entry is now inserted inactive - still reserving the id and the cap slot atomically - and becomes eligible for delivery only after the acknowledgement write has completed. A concurrent notification inside the window skips the inactive entry, which is indistinguishable from the change happening just before the subscription opened. ## How Has This Been Tested? With a new regression test pinning the window (an entry injected in the registered-but-unacknowledged state receives nothing until activation), the full suite, RuboCop, and the conformance suite, all green. ## Breaking Changes None. Activation happens synchronously inside the same response body proc, so an acknowledged stream behaves exactly as before.
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
1 participant
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
Per SEP-2575, the server MUST NOT send any notification on a subscription before
notifications/subscriptions/acknowledged. The listen stream was registered in the delivery registry before its acknowledgement was written (the write happens outside the lock so a stalled reader cannot block the transport), so a change notification published concurrently from another thread could be written to the stream ahead of the acknowledgement.The entry is now inserted inactive - still reserving the id and the cap slot atomically - and becomes eligible for delivery only after the acknowledgement write has completed. A concurrent notification inside the window skips the inactive entry, which is indistinguishable from the change happening just before the subscription opened.
How Has This Been Tested?
With a new regression test pinning the window (an entry injected in the registered-but-unacknowledged state receives nothing until activation), the full suite, RuboCop, and the conformance suite, all green.
Breaking Changes
None. Activation happens synchronously inside the same response body proc, so an acknowledged stream behaves exactly as before.
Types of changes
Checklist