fix(redis): consistent grain directory disposal + multiplexer ownership tests#10182
Merged
ReubenBond merged 2 commits intoJun 8, 2026
Merged
Conversation
Mark RedisGrainDirectory as disposed before the early return in Dispose/DisposeAsync so that disposing before (or without) Initialize leaves the instance in a disposed state instead of throwing NullReferenceException from later calls such as Lookup. Add tests covering shared (not disposed) and exclusive (disposed) multiplexers for the grain storage, membership, reminder, and grain directory providers, exercising both synchronous and asynchronous disposal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t tests The dispose-before-initialize directory tests are not gated by CheckForRedis and never connect, but GetConfigurationOptions parsed the Redis connection string unconditionally, throwing ArgumentNullException in CI environments where no Redis is configured (for example, the Functional test job). Fall back to an empty ConfigurationOptions when no connection string is set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Jun 14, 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
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.
Follow-up to #10146 (already merged), addressing the remaining Copilot review feedback on that PR.
Problem
Two issues were raised on the merged Redis multiplexer-ownership change:
RedisGrainDirectory.Dispose()/DisposeAsync()returned early when_rediswas null (for example, if the directory was disposed beforeInitializeran, or after a failed initialization). In that case_disposedwas never set and_databaseremainednull!, so a subsequentLookupwould throw aNullReferenceExceptioninstead of behaving as disposed.Solution
_disposed = trueat the very start of bothDispose()andDisposeAsync(), before the early return, so the directory consistently behaves as disposed even when initialization never completed.RedisMultiplexerOwnershipTestscovering the grain storage, membership, reminder, and grain directory providers. Each verifies that a shared (IsShared: true)IConnectionMultiplexeris left alone while an exclusively-created one is disposed, exercising both synchronousDispose()and asynchronousDisposeAsync(). Two additional Redis-free tests cover the dispose-before-initialize regression directly.Microsoft Reviewers: Open in CodeFlow