fix(redis)!: fix multiplexer ownership#10146
Merged
ReubenBond merged 4 commits intoJun 4, 2026
Merged
Conversation
ReubenBond
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Redis provider connection ownership handling so DI-provided IConnectionMultiplexer instances are treated as shared and not disposed by Orleans providers.
Changes:
- Changes Redis provider
CreateMultiplexerdelegates to return both the multiplexer and an ownership/shared flag. - Marks
ServiceKey-resolved multiplexers as shared across clustering, reminders, persistence, and grain directory providers. - Adds disposal paths for owned multiplexers and updates Redis public API baselines.
Show a summary per file
| File | Description |
|---|---|
src/Redis/Orleans.Reminders.Redis/Storage/RedisReminderTable.cs |
Adds tracked shared/owned multiplexer disposal. |
src/Redis/Orleans.Reminders.Redis/Providers/RedisReminderTableOptions.cs |
Updates multiplexer factory contract with ownership metadata. |
src/Redis/Orleans.Reminders.Redis/Hosting/RedisRemindersProviderBuilder.cs |
Marks keyed DI multiplexers as shared. |
src/Redis/Orleans.Persistence.Redis/Storage/RedisGrainStorage.cs |
Adds disposal and removes lifecycle close callback. |
src/Redis/Orleans.Persistence.Redis/Providers/RedisStorageOptions.cs |
Updates storage multiplexer factory contract. |
src/Redis/Orleans.Persistence.Redis/Hosting/RedisGrainStorageProviderBuilder.cs |
Marks keyed DI multiplexers as shared. |
src/Redis/Orleans.GrainDirectory.Redis/RedisGrainDirectory.cs |
Adds shared/owned multiplexer disposal for grain directory. |
src/Redis/Orleans.GrainDirectory.Redis/Options/RedisGrainDirectoryOptions.cs |
Updates grain directory multiplexer factory contract. |
src/Redis/Orleans.GrainDirectory.Redis/Hosting/RedisGrainDirectoryProviderBuilder.cs |
Marks keyed DI multiplexers as shared. |
src/Redis/Orleans.Clustering.Redis/Storage/RedisMembershipTable.cs |
Adds async disposal and shared multiplexer tracking. |
src/Redis/Orleans.Clustering.Redis/Providers/RedisClusteringOptions.cs |
Updates clustering multiplexer factory contract. |
src/Redis/Orleans.Clustering.Redis/Hosting/RedisClusteringProviderBuilder.cs |
Marks keyed DI multiplexers as shared for silo/client clustering. |
src/api/Redis/Orleans.Reminders.Redis/Orleans.Reminders.Redis.cs |
Updates generated API baseline for reminder options. |
src/api/Redis/Orleans.Persistence.Redis/Orleans.Persistence.Redis.cs |
Updates generated API baseline for storage disposal/options. |
src/api/Redis/Orleans.GrainDirectory.Redis/Orleans.GrainDirectory.Redis.cs |
Updates generated API baseline for directory disposal/options. |
src/api/Redis/Orleans.Clustering.Redis/Orleans.Clustering.Redis.cs |
Updates generated API baseline for clustering options. |
Copilot's findings
- Files reviewed: 16/16 changed files
- Comments generated: 2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ReubenBond
force-pushed
the
reubenbond/fix-redis-multiplexer-ownership
branch
from
394768a to
326095a
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+192
to
196
| public void Dispose() | ||
| { | ||
| if (_redis != null && _redis.IsConnected) | ||
| var redis = _redis; | ||
| if (redis is null) | ||
| { |
Member
Author
There was a problem hiding this comment.
Fixed in 0d58300: _disposed is now set at the very start of Dispose(), before the early
eturn when _redis is null. So disposing before (or without) Initialize leaves the instance in a disposed state and later Lookup calls return null instead of throwing a NullReferenceException.
Comment on lines
+216
to
238
| public async ValueTask DisposeAsync() | ||
| { | ||
| var redis = _redis; | ||
| if (redis is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var redisIsShared = _redisIsShared; | ||
| redis.ConnectionRestored -= LogConnectionRestored; | ||
| redis.ConnectionFailed -= LogConnectionFailed; | ||
| redis.ErrorMessage -= LogErrorMessage; | ||
| redis.InternalError -= LogInternalError; | ||
| _disposed = true; | ||
| _redis = null!; | ||
| _database = null!; | ||
| _redisIsShared = false; | ||
|
|
||
| if (!redisIsShared) | ||
| { | ||
| await redis.DisposeAsync().ConfigureAwait(false); | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
Fixed in 0d58300: same change applied to DisposeAsync() — _disposed is set before the early return so the object behaves consistently as disposed even when initialization never completed.
Comment on lines
72
to
+76
| try | ||
| { | ||
| LogDebugInitializing(_name, _serviceId, _options.DeleteStateOnClear); | ||
|
|
||
| _connection = await _options.CreateMultiplexer(_options).ConfigureAwait(false); | ||
| (_connection, _connectionIsShared) = await _options.CreateMultiplexer(_options).ConfigureAwait(false); |
Member
Author
There was a problem hiding this comment.
Added targeted tests in RedisMultiplexerOwnershipTests covering both a shared (IsShared: true, must NOT be disposed) and an exclusively-created (IsShared: false, must be disposed) IConnectionMultiplexer for all four non-streaming providers (grain storage, membership, reminders, grain directory), exercising both synchronous Dispose() and asynchronous DisposeAsync().
This was referenced Jun 4, 2026
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
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.
Fixes Redis provider ownership handling for shared
IConnectionMultiplexerinstances.Redis providers can be configured with a
ServiceKeyto use a DI-provided multiplexer. Those instances are shared and owned by DI, so providers should not close or dispose them.This updates the non-streaming Redis providers to report whether created multiplexers are shared, marks
ServiceKey-resolved multiplexers as shared, and implements standardDispose/DisposeAsynccleanup so owned multiplexers are disposed naturally when the container is disposed while shared multiplexers are left alone.