diff --git a/src/Libraries/Microsoft.Extensions.ServiceDiscovery/Http/ServiceDiscoveryHttpMessageHandlerFactory.cs b/src/Libraries/Microsoft.Extensions.ServiceDiscovery/Http/ServiceDiscoveryHttpMessageHandlerFactory.cs index e5e7f7587bb..874506550ab 100644 --- a/src/Libraries/Microsoft.Extensions.ServiceDiscovery/Http/ServiceDiscoveryHttpMessageHandlerFactory.cs +++ b/src/Libraries/Microsoft.Extensions.ServiceDiscovery/Http/ServiceDiscoveryHttpMessageHandlerFactory.cs @@ -6,14 +6,9 @@ namespace Microsoft.Extensions.ServiceDiscovery.Http; internal sealed class ServiceDiscoveryHttpMessageHandlerFactory( - TimeProvider timeProvider, - IServiceProvider serviceProvider, - ServiceEndpointWatcherFactory factory, + HttpServiceEndpointResolver resolver, IOptions options) : IServiceDiscoveryHttpMessageHandlerFactory { public HttpMessageHandler CreateHandler(HttpMessageHandler handler) - { - var registry = new HttpServiceEndpointResolver(factory, serviceProvider, timeProvider); - return new ResolvingHttpDelegatingHandler(registry, options, handler); - } + => new ResolvingHttpDelegatingHandler(resolver, options, handler); } diff --git a/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryHttpClientBuilderExtensions.cs b/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryHttpClientBuilderExtensions.cs index d2890ae8c8d..0f31c490c13 100644 --- a/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryHttpClientBuilderExtensions.cs +++ b/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryHttpClientBuilderExtensions.cs @@ -30,11 +30,9 @@ public static IHttpClientBuilder AddServiceDiscovery(this IHttpClientBuilder htt services.AddServiceDiscoveryCore(); httpClientBuilder.AddHttpMessageHandler(services => { - var timeProvider = services.GetService() ?? TimeProvider.System; - var watcherFactory = services.GetRequiredService(); - var registry = new HttpServiceEndpointResolver(watcherFactory, services, timeProvider); + var resolver = services.GetRequiredService(); var options = services.GetRequiredService>(); - return new ResolvingHttpDelegatingHandler(registry, options); + return new ResolvingHttpDelegatingHandler(resolver, options); }); #if NET diff --git a/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryServiceCollectionExtensions.cs b/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryServiceCollectionExtensions.cs index 8de759af1f6..0e707789216 100644 --- a/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryServiceCollectionExtensions.cs +++ b/src/Libraries/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryServiceCollectionExtensions.cs @@ -65,6 +65,13 @@ public static IServiceCollection AddServiceDiscoveryCore(this IServiceCollection services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(sp => new ServiceEndpointResolver(sp.GetRequiredService(), sp.GetRequiredService())); + + // Registered as a singleton (rather than created per HTTP message handler) so its refresh + // timers and configuration change-token subscriptions are created once and disposed with the + // container, instead of leaking on every handler rotation. It has no per-client state and + // caches watchers per service name internally, so a single shared instance is correct. + services.TryAddSingleton(sp => new HttpServiceEndpointResolver(sp.GetRequiredService(), sp, sp.GetRequiredService())); + if (configureOptions is not null) { services.Configure(configureOptions); diff --git a/test/Libraries/Microsoft.Extensions.ServiceDiscovery.Tests/HttpServiceEndpointResolverSharingTests.cs b/test/Libraries/Microsoft.Extensions.ServiceDiscovery.Tests/HttpServiceEndpointResolverSharingTests.cs new file mode 100644 index 00000000000..fb7e41ff2d2 --- /dev/null +++ b/test/Libraries/Microsoft.Extensions.ServiceDiscovery.Tests/HttpServiceEndpointResolverSharingTests.cs @@ -0,0 +1,63 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Reflection; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.ServiceDiscovery.Http; +using Xunit; + +namespace Microsoft.Extensions.ServiceDiscovery.Tests; + +/// +/// Tests that a single is shared across HTTP message handlers +/// rather than created per handler build. Creating one per build leaks the resolver (an +/// that roots refresh timers and configuration change-token subscriptions), +/// because never disposes it. +/// +public class HttpServiceEndpointResolverSharingTests +{ + [Fact] + public async Task AddServiceDiscoveryCore_RegistersResolverAsSingleton() + { + await using var services = new ServiceCollection() + .AddServiceDiscoveryCore() + .BuildServiceProvider(); + + var first = services.GetRequiredService(); + var second = services.GetRequiredService(); + + Assert.Same(first, second); + } + + [Fact] + public async Task AddServiceDiscovery_HttpClient_HandlerUsesSharedResolverSingleton() + { + var services = new ServiceCollection(); + services.AddHttpClient("test").AddServiceDiscovery(); + await using var provider = services.BuildServiceProvider(); + + using var handler = provider.GetRequiredService().CreateHandler("test"); + var resolvingHandler = FindHandler(handler); + Assert.NotNull(resolvingHandler); + + var resolverField = typeof(ResolvingHttpDelegatingHandler) + .GetField("_resolver", BindingFlags.Instance | BindingFlags.NonPublic); + var usedResolver = resolverField!.GetValue(resolvingHandler); + + Assert.Same(provider.GetRequiredService(), usedResolver); + } + + private static T? FindHandler(HttpMessageHandler handler) + where T : HttpMessageHandler + { + for (var current = handler; current is not null; current = (current as DelegatingHandler)?.InnerHandler) + { + if (current is T match) + { + return match; + } + } + + return null; + } +}