LightClient: Unsubscribe from subscriptions #1408
Merged
Conversation
This file contains 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
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
tadeohepperle
approved these changes
Jan 29, 2024
niklasad1
reviewed
Jan 29, 2024
niklasad1
approved these changes
Jan 29, 2024
jsdw
reviewed
Jan 30, 2024
jsdw
reviewed
Jan 30, 2024
| } | ||
| } else { | ||
| // User dropped the receiver, unsubscribe from the method and remove internal tracking. | ||
| let Some(subscription_state) = self.subscriptions.remove(&(id, chain_id)) else { |
There was a problem hiding this comment.
I'd have to check; is the only kind of error that can be returned from the channel above one that indicates it was closed by the receiver?
There was a problem hiding this comment.
Yep, taken from https://docs.rs/tokio/latest/tokio/sync/mpsc/struct.UnboundedSender.html#method.send:
If the receive half of the channel is closed, either due to close being called or the UnboundedReceiver having been dropped, this function returns an error. The error includes the value passed to send.
jsdw
approved these changes
Jan 30, 2024
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
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.
Previous to the PR, subscribing to a method would never call unsubscribe from that method, even when the user dropped the subscription.
This PR ensures that the appropriate
unsubscribemethod is being called to avoid consuming unnecessary memory, overloading the smoldot instance by never-ending subscriptions, and unwanted side-effects as imposing a maximum limit of subscriptions after which they are rejected.To achieve this:
Running the subxt test suite for the light-client produced multiple warnings;
With this PR, the smoldot acknowledges the unsubscribe method and stops the subscription:
Closes: #1402