From fafbf668b774ae967ee88d7a2c806cb2f973f36d Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sat, 3 May 2025 11:10:43 -0400 Subject: [PATCH 1/2] Connections: Move all AuthenticateAsClient to async Previously when supporting earlier versions of .NET Framework, we did not have an async version of `AuthenticateAsClient` due to socket itself not having async handlers in earlier versions of `netstandard`. These days though, this can be fully async to prevent thread starvation specifically in the case where we're able to connect a socket but the server is overwhelmed and unable to finish TLS negotiations. Note that none of these take a cancellation token, so while we can avoid thread growth on the client side as a primary symptom of the case here, the server-side impact of backlogged but unconnected things may continue to grow. It's net better overall, though, and should help a bit on mass connection cases for some applications as well, in the cases TLS negotiation isn't instant. --- .../Configuration/LoggingTunnel.cs | 4 ++-- src/StackExchange.Redis/ExtensionMethods.cs | 13 ++++--------- src/StackExchange.Redis/PhysicalConnection.cs | 4 ++-- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs index 987d2075c..d61442071 100644 --- a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs +++ b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs @@ -367,10 +367,10 @@ private async Task TlsHandshakeAsync(Stream stream, EndPoint endpoint) } else { - ssl.AuthenticateAsClient(host, _options.SslProtocols, _options.CheckCertificateRevocation); + await ssl.AuthenticateAsClientAsync(host, _options.SslProtocols, _options.CheckCertificateRevocation).ForAwait(); } #else - ssl.AuthenticateAsClient(host, _options.SslProtocols, _options.CheckCertificateRevocation); + await ssl.AuthenticateAsClientAsync(host, _options.SslProtocols, _options.CheckCertificateRevocation).ForAwait(); #endif return ssl; } diff --git a/src/StackExchange.Redis/ExtensionMethods.cs b/src/StackExchange.Redis/ExtensionMethods.cs index 87904aa9c..e5a5c4d4d 100644 --- a/src/StackExchange.Redis/ExtensionMethods.cs +++ b/src/StackExchange.Redis/ExtensionMethods.cs @@ -7,6 +7,7 @@ using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using System.Text; +using System.Threading.Tasks; using Pipelines.Sockets.Unofficial.Arenas; namespace StackExchange.Redis @@ -188,22 +189,16 @@ public static class ExtensionMethods return Array.ConvertAll(values, x => (string?)x); } - internal static void AuthenticateAsClient(this SslStream ssl, string host, SslProtocols? allowedProtocols, bool checkCertificateRevocation) + internal static Task AuthenticateAsClientAsync(this SslStream ssl, string host, SslProtocols? allowedProtocols, bool checkCertificateRevocation) { if (!allowedProtocols.HasValue) { // Default to the sslProtocols defined by the .NET Framework - AuthenticateAsClientUsingDefaultProtocols(ssl, host); - return; + return ssl.AuthenticateAsClientAsync(host); } var certificateCollection = new X509CertificateCollection(); - ssl.AuthenticateAsClient(host, certificateCollection, allowedProtocols.Value, checkCertificateRevocation); - } - - private static void AuthenticateAsClientUsingDefaultProtocols(SslStream ssl, string host) - { - ssl.AuthenticateAsClient(host); + return ssl.AuthenticateAsClientAsync(host, certificateCollection, allowedProtocols.Value, checkCertificateRevocation); } /// diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index df30ab207..51fac7c3d 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -1585,10 +1585,10 @@ internal async ValueTask ConnectedAsync(Socket? socket, ILogger? log, Sock } else { - ssl.AuthenticateAsClient(host, config.SslProtocols, config.CheckCertificateRevocation); + await ssl.AuthenticateAsClientAsync(host, config.SslProtocols, config.CheckCertificateRevocation).ForAwait(); } #else - ssl.AuthenticateAsClient(host, config.SslProtocols, config.CheckCertificateRevocation); + await ssl.AuthenticateAsClientAsync(host, config.SslProtocols, config.CheckCertificateRevocation).ForAwait(); #endif } catch (Exception ex) From 7bf8797a7be8f7ed1a937737fab735f48828d2d0 Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sat, 3 May 2025 11:23:35 -0400 Subject: [PATCH 2/2] Add release notes --- docs/ReleaseNotes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 4b26d9348..c1e77aa17 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -10,6 +10,7 @@ Current package versions: No pending unreleased changes - Add `ConfigurationOptions.SetUserPemCertificate(...)` and `ConfigurationOptions.SetUserPfxCertificate(...)` methods to simplify using client certificates ([#2873 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2873)) +- Fix: Move `AuthenticateAsClient` to fully async after dropping older framework support, to help client thread starvation in cases TLS negotiation stalls server-side ([#2878 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2878)) ## 2.8.31