From 41d5f4da6fdd470e0ad90c8aac417c733bb1c2a4 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 2 May 2024 12:43:57 -0400 Subject: [PATCH] Make SocketsHttpHandler the default primary handler from HttpClientFactory Today the default handler is HttpClientHandler, which is just a wrapper around SocketsHttpHandler on platforms where SocketsHttpHandler is supported. This means that the configuration possible on SocketsHttpHandler isn't available as part of the default handling, which means consumers get the default PooledConnectionLifetime of infinite. The lack of that was one of the main motivations behind HttpClientFactory's handler lifetime and handler recycling. Instead of constructing an HttpClientHandler as the default handler, we can construct a SocketsHttpHandler as the default handler, and we can set its PooledConnectionLifetime to match the HttpClientFactoryOptions.HandlerLifetime, whether its default of 2 minutes or whatever a user configured it to be. This in turn means that if code gets and holds on to a handler/client from the factory for a prolonged period of time, it's still getting the connection recycling according to its configured options. --- .../src/DefaultHttpMessageHandlerBuilder.cs | 45 +++++++++++++++++-- .../DefaultHttpMessageHandlerBuilderTest.cs | 17 +++++-- .../SocketsHttpHandlerConfigurationTest.cs | 35 ++++++++++++++- 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Http/src/DefaultHttpMessageHandlerBuilder.cs b/src/libraries/Microsoft.Extensions.Http/src/DefaultHttpMessageHandlerBuilder.cs index 21b39a15b121e2..fa154227f7c679 100644 --- a/src/libraries/Microsoft.Extensions.Http/src/DefaultHttpMessageHandlerBuilder.cs +++ b/src/libraries/Microsoft.Extensions.Http/src/DefaultHttpMessageHandlerBuilder.cs @@ -5,18 +5,22 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Net.Http; +using System.Threading; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; namespace Microsoft.Extensions.Http { internal sealed class DefaultHttpMessageHandlerBuilder : HttpMessageHandlerBuilder { + private HttpMessageHandler? _primaryHandler; + private string? _name; + public DefaultHttpMessageHandlerBuilder(IServiceProvider services) { Services = services; } - private string? _name; - [DisallowNull] public override string? Name { @@ -28,7 +32,11 @@ public override string? Name } } - public override HttpMessageHandler PrimaryHandler { get; set; } = new HttpClientHandler(); + public override HttpMessageHandler PrimaryHandler + { + get => _primaryHandler ??= CreatePrimaryHandler(); + set => _primaryHandler = value; + } public override IList AdditionalHandlers { get; } = new List(); @@ -44,5 +52,36 @@ public override HttpMessageHandler Build() return CreateHandlerPipeline(PrimaryHandler, AdditionalHandlers); } + +#pragma warning disable CA1822, CA1859 // Mark members as static, Use concrete types when possible for improved performance + private HttpMessageHandler CreatePrimaryHandler() +#pragma warning restore CA1822, CA1859 + { +#if NET + // On platforms where SocketsHttpHandler is supported, HttpClientHandler is a thin wrapper + // around it. By using SocketsHttpHandler directly, we can avoid the overhead of the wrapper, + // but more importantly, we can configure it to limit the lifetime of its pooled connections + // to match the requested lifetime of the handler itself. That way, if/when someone holds on + // to a resulting HttpClient for a prolonged period of time, it'll still benefit from connection + // recycling, and without needing to tear down and reconstitute the rest of the handler pipeline. + if (SocketsHttpHandler.IsSupported) + { + SocketsHttpHandler handler = new(); + + if (Services.GetService>() is IOptionsMonitor optionsMonitor) + { + TimeSpan lifetime = optionsMonitor.Get(_name).HandlerLifetime; + if (lifetime >= TimeSpan.Zero) + { + handler.PooledConnectionLifetime = lifetime; + } + } + + return handler; + } +#endif + + return new HttpClientHandler(); + } } } diff --git a/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DefaultHttpMessageHandlerBuilderTest.cs b/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DefaultHttpMessageHandlerBuilderTest.cs index a597114d2f4e2d..acc21dc6a101fd 100644 --- a/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DefaultHttpMessageHandlerBuilderTest.cs +++ b/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/DefaultHttpMessageHandlerBuilderTest.cs @@ -28,7 +28,16 @@ public void Ctor_SetsPrimaryHandler() var builder = new DefaultHttpMessageHandlerBuilder(Services); // Act - Assert.IsType(builder.PrimaryHandler); +#if NET + if (SocketsHttpHandler.IsSupported) + { + Assert.IsType(builder.PrimaryHandler); + } + else +#endif + { + Assert.IsType(builder.PrimaryHandler); + } } // Moq heavily utilizes RefEmit, which does not work on most aot workloads @@ -78,7 +87,7 @@ public void Build_SomeAdditionalHandlers_PutsTogetherDelegatingHandlers() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/50873", TestPlatforms.Android)] - public void Build_PrimaryHandlerIsNull_ThrowsException() + public void Build_PrimaryHandlerIsNull_UsesDefault() { // Arrange var builder = new DefaultHttpMessageHandlerBuilder(Services) @@ -87,8 +96,8 @@ public void Build_PrimaryHandlerIsNull_ThrowsException() }; // Act & Assert - var exception = Assert.Throws(() => builder.Build()); - Assert.Equal("The 'PrimaryHandler' must not be null.", exception.Message); + var handler = builder.Build(); + Assert.NotNull(handler); } [Fact] diff --git a/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/SocketsHttpHandlerConfigurationTest.cs b/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/SocketsHttpHandlerConfigurationTest.cs index 059557a0784a70..017a6b44aa486d 100644 --- a/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/SocketsHttpHandlerConfigurationTest.cs +++ b/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/SocketsHttpHandlerConfigurationTest.cs @@ -42,10 +42,41 @@ public void UseSocketsHttpHandler_Parameterless_Success() var defaultPrimaryHandlerChain = messageHandlerFactory.CreateHandler("DefaultPrimaryHandler"); var socketsHttpHandlerChain = messageHandlerFactory.CreateHandler("SocketsHttpHandler"); - Assert.IsType(GetPrimaryHandler(defaultPrimaryHandlerChain)); + Assert.IsType(GetPrimaryHandler(defaultPrimaryHandlerChain)); Assert.IsType(GetPrimaryHandler(socketsHttpHandlerChain)); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] + public void DefaultPrimaryHandler_RespectsHandlerLifetime() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddHttpClient(); + serviceCollection.Configure(options => options.HandlerLifetime = TimeSpan.FromMinutes(42)); + + var services = serviceCollection.BuildServiceProvider(); + var messageHandlerFactory = services.GetRequiredService(); + var defaultPrimaryHandlerChain = messageHandlerFactory.CreateHandler(); + + SocketsHttpHandler handler = Assert.IsType(GetPrimaryHandler(defaultPrimaryHandlerChain)); + Assert.Equal(TimeSpan.FromMinutes(42), handler.PooledConnectionLifetime); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] + public void DefaultPrimaryHandler_NamedClient_RespectsHandlerLifetime() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddHttpClient("Configured"); + serviceCollection.Configure("Configured1", options => options.HandlerLifetime = TimeSpan.FromMinutes(42)); + serviceCollection.Configure("Configured2", options => options.HandlerLifetime = TimeSpan.FromMinutes(84)); + + var services = serviceCollection.BuildServiceProvider(); + var messageHandlerFactory = services.GetRequiredService(); + + Assert.Equal(TimeSpan.FromMinutes(42), Assert.IsType(GetPrimaryHandler(messageHandlerFactory.CreateHandler("Configured1"))).PooledConnectionLifetime); + Assert.Equal(TimeSpan.FromMinutes(84), Assert.IsType(GetPrimaryHandler(messageHandlerFactory.CreateHandler("Configured2"))).PooledConnectionLifetime); + Assert.Equal(TimeSpan.FromMinutes(2), Assert.IsType(GetPrimaryHandler(messageHandlerFactory.CreateHandler())).PooledConnectionLifetime); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] public void UseSocketsHttpHandler_ConfiguredByAction_Success() { @@ -66,7 +97,7 @@ public void UseSocketsHttpHandler_ConfiguredByAction_Success() var unconfiguredHandler = (SocketsHttpHandler)GetPrimaryHandler(unconfiguredHandlerChain); var configuredHandler = (SocketsHttpHandler)GetPrimaryHandler(configuredHandlerChain); - Assert.Equal(Timeout.InfiniteTimeSpan, unconfiguredHandler.PooledConnectionLifetime); + Assert.Equal(TimeSpan.FromMinutes(2), unconfiguredHandler.PooledConnectionLifetime); Assert.Equal(TimeSpan.FromMinutes(1), configuredHandler.PooledConnectionLifetime); }