From a25c4c7bf57b60047e72b6f17224dd181219cc46 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 13 Jul 2017 17:04:29 -0400 Subject: [PATCH] Use a shared MultiAgent in CurlHandler when using LibreSSL backend on macOS LibreSSL needs locks initialized in order to be used safely from multiple threads, which happens when multiple HttpClients are used. But we don't currently have a good way to perform that initialization as we do for OpenSSL. As a workaround, add a shared MultiAgent that's initialized when LibreSSL is used on macOS, and use that same MultiAgent from all CurlHandler instances rather than giving each their own. Since MultiAgent is 1:1 with a thread, all HttpClient's will then end up sharing the same libcurl event loop thread, and we'll serialize all of our interactions with libcurl and thus hopefully with LibreSSL. --- .../Interop.VersionInfo.cs | 1 + .../Net/Http/OSX/CurlHandler.SslProvider.cs | 13 ++++++ .../Net/Http/Unix/CurlHandler.MultiAgent.cs | 44 +++++++++++-------- .../src/System/Net/Http/Unix/CurlHandler.cs | 20 ++++++++- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs b/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs index 09a60043557b..1899fd0af3ca 100644 --- a/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs +++ b/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs @@ -49,5 +49,6 @@ internal enum CurlFeatures : int internal const string OpenSsl10Description = "openssl/1.0"; internal const string SecureTransportDescription = "SecureTransport"; + internal const string LibreSslDescription = "LibreSSL"; } } diff --git a/src/System.Net.Http/src/System/Net/Http/OSX/CurlHandler.SslProvider.cs b/src/System.Net.Http/src/System/Net/Http/OSX/CurlHandler.SslProvider.cs index 4c84760379d0..ba11f8433da3 100644 --- a/src/System.Net.Http/src/System/Net/Http/OSX/CurlHandler.SslProvider.cs +++ b/src/System.Net.Http/src/System/Net/Http/OSX/CurlHandler.SslProvider.cs @@ -12,6 +12,19 @@ namespace System.Net.Http { internal partial class CurlHandler : HttpMessageHandler { + static partial void UseSingletonMultiAgent(ref bool result) + { + // Some backends other than OpenSSL need locks initialized in order to use them in a + // multithreaded context, which would happen with multiple HttpClients and thus multiple + // MultiAgents. Since we don't currently have the ability to do so initialization, instead we + // restrict all HttpClients to use the same MultiAgent instance in this case. We know LibreSSL + // is in this camp, so we currently special-case it. + string curlSslVersion = Interop.Http.GetSslVersionDescription(); + result = + !string.IsNullOrEmpty(curlSslVersion) && + curlSslVersion.StartsWith(Interop.Http.LibreSslDescription, StringComparison.OrdinalIgnoreCase); + } + private static class SslProvider { internal static void SetSslOptions(EasyRequest easy, ClientCertificateOption clientCertOption) diff --git a/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.MultiAgent.cs b/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.MultiAgent.cs index 67473615bcb9..209ccc5ad03d 100644 --- a/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.MultiAgent.cs +++ b/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.MultiAgent.cs @@ -37,8 +37,8 @@ private sealed class MultiAgent : IDisposable private static readonly Interop.Http.ReadWriteCallback s_receiveBodyCallback = CurlReceiveBodyCallback; private static readonly Interop.Http.DebugCallback s_debugCallback = CurlDebugFunction; - /// CurlHandler that owns this MultiAgent. - private readonly CurlHandler _associatedHandler; + /// CurlHandler that created this MultiAgent. If null, this is a shared handler. + private readonly CurlHandler _creatingHandler; /// /// A collection of not-yet-processed incoming requests for work to be done @@ -79,18 +79,21 @@ private sealed class MultiAgent : IDisposable /// private Interop.Http.SafeCurlMultiHandle _multiHandle; + /// Set when Dispose has been called. + private bool _disposed; + /// Initializes the MultiAgent. - /// The handler that owns this agent. + /// The handler that created this agent, or null if it's shared. public MultiAgent(CurlHandler handler) { - Debug.Assert(handler != null, "Expected non-null handler"); - _associatedHandler = handler; + _creatingHandler = handler; } /// Disposes of the agent. public void Dispose() { EventSourceTrace(null); + _disposed = true; QueueIfRunning(new IncomingRequest { Type = IncomingRequestType.Shutdown }); _multiHandle?.Dispose(); } @@ -247,20 +250,25 @@ private Interop.Http.SafeCurlMultiHandle CreateAndConfigureMultiHandle() EventSourceTrace("Set multiplexing on multi handle"); } - // Configure max connections per host if it was changed from the default - int maxConnections = _associatedHandler.MaxConnectionsPerServer; - if (maxConnections < int.MaxValue) // int.MaxValue considered infinite, mapping to libcurl default of 0 + // Configure max connections per host if it was changed from the default. In shared mode, + // this will be pulled from the handler that first created the agent; the setting from subsequent + // handlers that use this will be ignored. + if (_creatingHandler != null) { - CURLMcode code = Interop.Http.MultiSetOptionLong(multiHandle, Interop.Http.CURLMoption.CURLMOPT_MAX_HOST_CONNECTIONS, maxConnections); - switch (code) + int maxConnections = _creatingHandler.MaxConnectionsPerServer; + if (maxConnections < int.MaxValue) // int.MaxValue considered infinite, mapping to libcurl default of 0 { - case CURLMcode.CURLM_OK: - EventSourceTrace("Set max host connections to {0}", maxConnections); - break; - default: - // Treat failures as non-fatal in release; worst case is we employ more connections than desired. - EventSourceTrace("Setting CURLMOPT_MAX_HOST_CONNECTIONS failed: {0}. Ignoring option.", code); - break; + CURLMcode code = Interop.Http.MultiSetOptionLong(multiHandle, Interop.Http.CURLMoption.CURLMOPT_MAX_HOST_CONNECTIONS, maxConnections); + switch (code) + { + case CURLMcode.CURLM_OK: + EventSourceTrace("Set max host connections to {0}", maxConnections); + break; + default: + // Treat failures as non-fatal in release; worst case is we employ more connections than desired. + EventSourceTrace("Setting CURLMOPT_MAX_HOST_CONNECTIONS failed: {0}. Ignoring option.", code); + break; + } } } @@ -306,7 +314,7 @@ private void WorkerBody() // more requests could have been added. If they were, // kick off another processing loop. _runningWorker = null; - if (_incomingRequests.Count > 0 && !_associatedHandler._disposed) + if (_incomingRequests.Count > 0 && !_disposed) { EnsureWorkerIsRunning(); } diff --git a/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.cs b/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.cs index 486231efeb48..6861c9df443e 100644 --- a/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.cs +++ b/src/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.cs @@ -126,6 +126,7 @@ internal partial class CurlHandler : HttpMessageHandler private static string s_curlVersionDescription; private static string s_curlSslVersionDescription; + private static readonly MultiAgent s_singletonSharedAgent; private readonly MultiAgent _agent; private volatile bool _anyOperationStarted; private volatile bool _disposed; @@ -169,13 +170,28 @@ static CurlHandler() { EventSourceTrace($"libcurl: {CurlVersionDescription} {CurlSslVersionDescription} {features}"); } + + // By default every CurlHandler gets its own MultiAgent. But for some backends, + // we need to restrict the number of threads involved in processing libcurl work, + // so we create a single MultiAgent that's used by all handlers. + bool useSingleton = false; + UseSingletonMultiAgent(ref useSingleton); + if (useSingleton) + { + s_singletonSharedAgent = new MultiAgent(null); + } } public CurlHandler() { - _agent = new MultiAgent(this); + // If the shared MultiAgent was initialized, use it. + // Otherwise, create a new MultiAgent for this handler. + _agent = s_singletonSharedAgent ?? new MultiAgent(this); } + /// Overridden by another partial implementation to set to true if a single MultiAgent should be used. + static partial void UseSingletonMultiAgent(ref bool result); + #region Properties private static string CurlVersionDescription => s_curlVersionDescription ?? (s_curlVersionDescription = Interop.Http.GetVersionDescription() ?? string.Empty); @@ -414,7 +430,7 @@ internal bool UseDefaultCredentials protected override void Dispose(bool disposing) { _disposed = true; - if (disposing) + if (disposing && _agent != s_singletonSharedAgent) { _agent.Dispose(); }