diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 758e0a30f..9ee5e42fd 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -8,7 +8,7 @@ Current package versions: ## Unreleased -- nothing yet +- Make ConfigurationOptions IDisposable ([#2922 by philon-msft](https://github.com/StackExchange/StackExchange.Redis/pull/2922)) ## 2.8.58 diff --git a/src/StackExchange.Redis/ConfigurationOptions.cs b/src/StackExchange.Redis/ConfigurationOptions.cs index dfdab5f4e..e917cefa3 100644 --- a/src/StackExchange.Redis/ConfigurationOptions.cs +++ b/src/StackExchange.Redis/ConfigurationOptions.cs @@ -28,7 +28,7 @@ namespace StackExchange.Redis /// /// /// - public sealed class ConfigurationOptions : ICloneable + public sealed class ConfigurationOptions : ICloneable, IDisposable { private static class OptionKeys { @@ -178,6 +178,8 @@ public static string TryNormalize(string value) private ILoggerFactory? loggerFactory; + private bool _isDisposed = false; + /// /// A LocalCertificateSelectionCallback delegate responsible for selecting the certificate used for authentication; note /// that this cannot be specified in the configuration-string. @@ -197,8 +199,16 @@ public static string TryNormalize(string value) /// public DefaultOptionsProvider Defaults { - get => defaultOptions ??= DefaultOptionsProvider.GetProvider(EndPoints); - set => defaultOptions = value; + get + { + ThrowIfDisposed(); + return defaultOptions ??= DefaultOptionsProvider.GetProvider(EndPoints); + } + set + { + ThrowIfDisposed(); + defaultOptions = value; + } } /// @@ -305,8 +315,8 @@ public bool HighIntegrity /// /// Supply a user certificate from a PEM file pair and enable TLS. /// - /// The path for the the user certificate (commonly a .crt file). - /// The path for the the user key (commonly a .key file). + /// The path for the user certificate (commonly a .crt file). + /// The path for the user key (commonly a .key file). public void SetUserPemCertificate(string userCertificatePath, string? userKeyPath = null) { CertificateSelectionCallback = CreatePemUserCertificateCallback(userCertificatePath, userKeyPath); @@ -317,7 +327,7 @@ public void SetUserPemCertificate(string userCertificatePath, string? userKeyPat /// /// Supply a user certificate from a PFX file and optional password and enable TLS. /// - /// The path for the the user certificate (commonly a .pfx file). + /// The path for the user certificate (commonly a .pfx file). /// The password for the certificate file. public void SetUserPfxCertificate(string userCertificatePath, string? password = null) { @@ -383,14 +393,14 @@ private static bool CheckTrustedIssuer(X509Certificate2 certificateToValidate, X chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; chain.ChainPolicy.VerificationTime = chainToValidate?.ChainPolicy?.VerificationTime ?? DateTime.Now; chain.ChainPolicy.UrlRetrievalTimeout = new TimeSpan(0, 0, 0); - // Ensure entended key usage checks are run and that we're observing a server TLS certificate + // Ensure intended key usage checks are run and that we're observing a server TLS certificate chain.ChainPolicy.ApplicationPolicy.Add(_serverAuthOid); chain.ChainPolicy.ExtraStore.Add(authority); try { // This only verifies that the chain is valid, but with AllowUnknownCertificateAuthority could trust - // self-signed or partial chained vertificates + // self-signed or partial chained certificates var chainIsVerified = chain.Build(certificateToValidate); if (chainIsVerified) { @@ -787,53 +797,58 @@ public static ConfigurationOptions Parse(string configuration, bool ignoreUnknow /// /// Create a copy of the configuration. /// - public ConfigurationOptions Clone() => new ConfigurationOptions + public ConfigurationOptions Clone() { - defaultOptions = defaultOptions, - ClientName = ClientName, - ServiceName = ServiceName, - keepAlive = keepAlive, - syncTimeout = syncTimeout, - asyncTimeout = asyncTimeout, - allowAdmin = allowAdmin, - defaultVersion = defaultVersion, - connectTimeout = connectTimeout, - user = user, - password = password, - tieBreaker = tieBreaker, - ssl = ssl, - sslHost = sslHost, - configChannel = configChannel, - abortOnConnectFail = abortOnConnectFail, - resolveDns = resolveDns, - proxy = proxy, - commandMap = commandMap, - CertificateValidationCallback = CertificateValidationCallback, - CertificateSelectionCallback = CertificateSelectionCallback, - ChannelPrefix = ChannelPrefix.Clone(), - SocketManager = SocketManager, - connectRetry = connectRetry, - configCheckSeconds = configCheckSeconds, - responseTimeout = responseTimeout, - DefaultDatabase = DefaultDatabase, - reconnectRetryPolicy = reconnectRetryPolicy, - backlogPolicy = backlogPolicy, - SslProtocols = SslProtocols, - checkCertificateRevocation = checkCertificateRevocation, - BeforeSocketConnect = BeforeSocketConnect, - EndPoints = EndPoints.Clone(), - LoggerFactory = LoggerFactory, + ThrowIfDisposed(); + + return new ConfigurationOptions + { + defaultOptions = defaultOptions, + ClientName = ClientName, + ServiceName = ServiceName, + keepAlive = keepAlive, + syncTimeout = syncTimeout, + asyncTimeout = asyncTimeout, + allowAdmin = allowAdmin, + defaultVersion = defaultVersion, + connectTimeout = connectTimeout, + user = user, + password = password, + tieBreaker = tieBreaker, + ssl = ssl, + sslHost = sslHost, + configChannel = configChannel, + abortOnConnectFail = abortOnConnectFail, + resolveDns = resolveDns, + proxy = proxy, + commandMap = commandMap, + CertificateValidationCallback = CertificateValidationCallback, + CertificateSelectionCallback = CertificateSelectionCallback, + ChannelPrefix = ChannelPrefix.Clone(), + SocketManager = SocketManager, + connectRetry = connectRetry, + configCheckSeconds = configCheckSeconds, + responseTimeout = responseTimeout, + DefaultDatabase = DefaultDatabase, + reconnectRetryPolicy = reconnectRetryPolicy, + backlogPolicy = backlogPolicy, + SslProtocols = SslProtocols, + checkCertificateRevocation = checkCertificateRevocation, + BeforeSocketConnect = BeforeSocketConnect, + EndPoints = EndPoints.Clone(), + LoggerFactory = LoggerFactory, #if NETCOREAPP3_1_OR_GREATER - SslClientAuthenticationOptions = SslClientAuthenticationOptions, + SslClientAuthenticationOptions = SslClientAuthenticationOptions, #endif - Tunnel = Tunnel, - setClientLibrary = setClientLibrary, - LibraryName = LibraryName, - Protocol = Protocol, - heartbeatInterval = heartbeatInterval, - heartbeatConsistencyChecks = heartbeatConsistencyChecks, - highIntegrity = highIntegrity, - }; + Tunnel = Tunnel, + setClientLibrary = setClientLibrary, + LibraryName = LibraryName, + Protocol = Protocol, + heartbeatInterval = heartbeatInterval, + heartbeatConsistencyChecks = heartbeatConsistencyChecks, + highIntegrity = highIntegrity, + }; + } /// /// Apply settings to configure this instance of , e.g. for a specific scenario. @@ -922,7 +937,8 @@ public string ToString(bool includePassword) commandMap?.AppendDeltas(sb); return sb.ToString(); - static string? FormatProtocol(RedisProtocol? protocol) => protocol switch { + static string? FormatProtocol(RedisProtocol? protocol) => protocol switch + { null => null, RedisProtocol.Resp2 => "resp2", RedisProtocol.Resp3 => "resp3", @@ -1204,5 +1220,41 @@ internal static bool TryParseRedisProtocol(string? value, out RedisProtocol prot protocol = default; return false; } + + /// + /// Release all resources associated with this configuration. + /// + public void Dispose() + { + if (_isDisposed) + { + return; + } + + _isDisposed = true; + GC.SuppressFinalize(this); + + BeforeSocketConnect = null; + CertificateSelection = null; + CertificateValidation = null; + + backlogPolicy = null; + commandMap = null; + loggerFactory = null; + reconnectRetryPolicy = null; +#if NETCOREAPP3_1_OR_GREATER + SslClientAuthenticationOptions = null; +#endif + + try { (defaultOptions as IDisposable)?.Dispose(); } catch { } + } + + private void ThrowIfDisposed() + { + if (_isDisposed) + { + throw new ObjectDisposedException(nameof(ConfigurationOptions)); + } + } } } diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt index 43b35ba58..cc5facbb4 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt @@ -246,6 +246,7 @@ StackExchange.Redis.ConfigurationOptions.Defaults.get -> StackExchange.Redis.Con StackExchange.Redis.ConfigurationOptions.Defaults.set -> void StackExchange.Redis.ConfigurationOptions.DefaultVersion.get -> System.Version! StackExchange.Redis.ConfigurationOptions.DefaultVersion.set -> void +StackExchange.Redis.ConfigurationOptions.Dispose() -> void StackExchange.Redis.ConfigurationOptions.EndPoints.get -> StackExchange.Redis.EndPointCollection! StackExchange.Redis.ConfigurationOptions.EndPoints.init -> void StackExchange.Redis.ConfigurationOptions.HeartbeatConsistencyChecks.get -> bool diff --git a/tests/StackExchange.Redis.Tests/ConfigTests.cs b/tests/StackExchange.Redis.Tests/ConfigTests.cs index 995b66a5a..5d8d326eb 100644 --- a/tests/StackExchange.Redis.Tests/ConfigTests.cs +++ b/tests/StackExchange.Redis.Tests/ConfigTests.cs @@ -34,6 +34,7 @@ public void ExpectedFields() Assert.Equal( new[] { + "_isDisposed", "abortOnConnectFail", "allowAdmin", "asyncTimeout", @@ -761,4 +762,20 @@ public void CheckHighIntegrity(bool? assigned, bool expected, string cs) var parsed = ConfigurationOptions.Parse(cs); Assert.Equal(expected, parsed.HighIntegrity); } + + [Fact] + public void DisposedThrows() + { + var options = ConfigurationOptions.Parse("myhost"); + Assert.IsType(options.Defaults); + + options.Dispose(); + + Assert.Throws(() => options.Defaults); + Assert.Throws(() => options.BacklogPolicy); + Assert.Throws(() => options.CommandMap); + Assert.Throws(() => options.LoggerFactory); + Assert.Throws(() => options.ReconnectRetryPolicy); + Assert.Throws(() => options.Clone()); + } }