fix: tolerate missing/invalidated stream subscription handles in grain activation#169
Open
d-barker wants to merge 1 commit into
Open
Conversation
…n activation SignalR.Orleans grains stored stream subscription handles and, on reactivation, indexed the result of GetAllSubscriptionHandles() with [0]. When a silo restarts (or a subscription was never persisted), GetAllSubscriptionHandles() returns an empty collection, so [0] threw IndexOutOfRangeException and the grain failed to activate. ClientGrain additionally mis-handled the resume path: it assigned the resumed handle to a *local* variable that shadowed the field, so the live handle was never stored. Because ResumeAsync() invalidates the handle it is called on and returns a new one, the next OnDisconnect() called UnsubscribeAsync() on a stale handle and threw "Handle is no longer valid. It has been used to unsubscribe or resume." Changes: - Replace `(await ...GetAllSubscriptionHandles())[0]` with a private GetSubscriptionHandleAsync(...) helper that returns handles.FirstOrDefault(). - On activation, if no handle is found, log a warning and re-subscribe to the client/server disconnect stream instead of throwing. - ClientGrain: capture and store the new handle returned by ResumeAsync() in the _serverDisconnectedSubscription field (fixes the shadowed-local bug and the "Handle is no longer valid" exception on disconnect). Files: - src/SignalR.Orleans/Clients/ClientGrain.cs - src/SignalR.Orleans/ConnectionGroups/ConnectionGroupGrain.cs
d-barker
requested review from
andrund,
claylaut,
galvesribeiro and
zeus82
as code owners
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.
SignalR.Orleans grains stored stream subscription handles and, on reactivation, indexed the result of GetAllSubscriptionHandles() with [0]. When a silo restarts (or a subscription was never persisted), GetAllSubscriptionHandles() returns an empty collection, so [0] threw IndexOutOfRangeException and the grain failed to activate.
ClientGrain had the same issue as the connection groups grain; where reactivation would fail with an empty collection.
Changes:
(await ...GetAllSubscriptionHandles())[0]with a private GetSubscriptionHandleAsync(...) helper that returns handles.FirstOrDefault().(await ...GetAllSubscriptionHandles())[0]with a private GetSubscriptionHandleAsync(...) helper that returns handles.FirstOrDefault().Files:
refs: #168