From 34a16dcf5043b4f02756e9d0727943c385cd2810 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 8 Mar 2023 16:27:53 -0600 Subject: [PATCH] Remove some Linq usages from Kestrel.Core (#47012) * Remove some Linq usages from Kestrel.Core --- .../Kestrel/Core/src/Internal/AddressBinder.cs | 5 +++-- .../Internal/Infrastructure/TransportManager.cs | 10 ++++++++-- .../Core/src/Internal/KestrelServerImpl.cs | 8 ++++++-- .../Core/src/KestrelConfigurationLoader.cs | 9 ++++++++- .../Kestrel/Core/src/KestrelServerOptions.cs | 15 ++++++++++++++- .../Kestrel/Core/test/AddressBinderTests.cs | 12 ++++++------ .../test/KestrelConfigurationLoaderTests.cs | 16 ++++++++-------- .../test/TransportTestHelpers/TestServer.cs | 2 +- .../test/BindTests/AddressRegistrationTests.cs | 2 +- 9 files changed, 55 insertions(+), 24 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs b/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs index 6f5b48723872..a2e872da3080 100644 --- a/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs +++ b/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs @@ -16,10 +16,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal; internal sealed class AddressBinder { - public static async Task BindAsync(IEnumerable listenOptions, AddressBindContext context, CancellationToken cancellationToken) + // note this doesn't copy the ListenOptions[], only call this with an array that isn't mutated elsewhere + public static async Task BindAsync(ListenOptions[] listenOptions, AddressBindContext context, CancellationToken cancellationToken) { var strategy = CreateStrategy( - listenOptions.ToArray(), + listenOptions, context.Addresses.ToArray(), context.ServerAddressesFeature.PreferHostingUrls); diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TransportManager.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TransportManager.cs index 3760612366e5..00505ae9e354 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TransportManager.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TransportManager.cs @@ -4,7 +4,6 @@ #nullable enable using System.IO.Pipelines; -using System.Linq; using System.Net; using System.Net.Security; using Microsoft.AspNetCore.Connections; @@ -179,7 +178,14 @@ private void StartAcceptLoop(IConnectionListener connectionListener, Func< public Task StopEndpointsAsync(List endpointsToStop, CancellationToken cancellationToken) { - var transportsToStop = _transports.Where(t => t.EndpointConfig != null && endpointsToStop.Contains(t.EndpointConfig)).ToList(); + var transportsToStop = new List(); + foreach (var t in _transports) + { + if (t.EndpointConfig is not null && endpointsToStop.Contains(t.EndpointConfig)) + { + transportsToStop.Add(t); + } + } return StopTransportsAsync(transportsToStop, cancellationToken); } diff --git a/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs b/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs index f2cc083d5c39..1861d46434f3 100644 --- a/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs +++ b/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs @@ -314,7 +314,7 @@ private async Task BindAsync(CancellationToken cancellationToken) Options.ConfigurationLoader?.Load(); - await AddressBinder.BindAsync(Options.ListenOptions, AddressBindContext!, cancellationToken).ConfigureAwait(false); + await AddressBinder.BindAsync(Options.GetListenOptions(), AddressBindContext!, cancellationToken).ConfigureAwait(false); _configChangedRegistration = reloadToken?.RegisterChangeCallback(TriggerRebind, this); } finally @@ -364,7 +364,11 @@ private async Task RebindAsync() // TODO: It would be nice to start binding to new endpoints immediately and reconfigured endpoints as soon // as the unbinding finished for the given endpoint rather than wait for all transports to unbind first. - var configsToStop = endpointsToStop.Select(lo => lo.EndpointConfig!).ToList(); + var configsToStop = new List(endpointsToStop.Count); + foreach (var lo in endpointsToStop) + { + configsToStop.Add(lo.EndpointConfig!); + } await _transportManager.StopEndpointsAsync(configsToStop, combinedCts.Token).ConfigureAwait(false); foreach (var listenOption in endpointsToStop) diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 297fd606148b..6b519fd56c5b 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -347,7 +347,14 @@ public void Load() // Now that defaults have been loaded, we can compare to the currently bound endpoints to see if the config changed. // There's no reason to rerun an EndpointConfigurations callback if nothing changed. - var matchingBoundEndpoints = endpointsToStop.Where(o => o.EndpointConfig == endpoint).ToList(); + var matchingBoundEndpoints = new List(); + foreach (var o in endpointsToStop) + { + if (o.EndpointConfig == endpoint) + { + matchingBoundEndpoints.Add(o); + } + } if (matchingBoundEndpoints.Count > 0) { diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index 3aa1ca76607d..6bfeaef62115 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -38,7 +38,20 @@ public class KestrelServerOptions // The following two lists configure the endpoints that Kestrel should listen to. If both lists are empty, the "urls" config setting (e.g. UseUrls) is used. internal List CodeBackedListenOptions { get; } = new List(); internal List ConfigurationBackedListenOptions { get; } = new List(); - internal IEnumerable ListenOptions => CodeBackedListenOptions.Concat(ConfigurationBackedListenOptions); + + internal ListenOptions[] GetListenOptions() + { + int resultCount = CodeBackedListenOptions.Count + ConfigurationBackedListenOptions.Count; + if (resultCount == 0) + { + return Array.Empty(); + } + + var result = new ListenOptions[resultCount]; + CodeBackedListenOptions.CopyTo(result); + ConfigurationBackedListenOptions.CopyTo(result, CodeBackedListenOptions.Count); + return result; + } // For testing and debugging. internal List OptionsInUse { get; } = new List(); diff --git a/src/Servers/Kestrel/Core/test/AddressBinderTests.cs b/src/Servers/Kestrel/Core/test/AddressBinderTests.cs index b36660bb95e7..4a0cd1602315 100644 --- a/src/Servers/Kestrel/Core/test/AddressBinderTests.cs +++ b/src/Servers/Kestrel/Core/test/AddressBinderTests.cs @@ -172,7 +172,7 @@ public async Task WrapsAddressInUseExceptionAsIOException() endpoint => throw new AddressInUseException("already in use")); await Assert.ThrowsAsync(() => - AddressBinder.BindAsync(options.ListenOptions, addressBindContext, CancellationToken.None)); + AddressBinder.BindAsync(options.GetListenOptions(), addressBindContext, CancellationToken.None)); } [Fact] @@ -193,7 +193,7 @@ public void LogsWarningWhenHostingAddressesAreOverridden() logger, endpoint => Task.CompletedTask); - var bindTask = AddressBinder.BindAsync(options.ListenOptions, addressBindContext, CancellationToken.None); + var bindTask = AddressBinder.BindAsync(options.GetListenOptions(), addressBindContext, CancellationToken.None); Assert.True(bindTask.IsCompletedSuccessfully); var log = Assert.Single(logger.Messages); @@ -221,7 +221,7 @@ public void LogsInformationWhenKestrelAddressesAreOverridden() addressBindContext.ServerAddressesFeature.PreferHostingUrls = true; - var bindTask = AddressBinder.BindAsync(options.ListenOptions, addressBindContext, CancellationToken.None); + var bindTask = AddressBinder.BindAsync(options.GetListenOptions(), addressBindContext, CancellationToken.None); Assert.True(bindTask.IsCompletedSuccessfully); var log = Assert.Single(logger.Messages); @@ -247,7 +247,7 @@ public async Task FlowsCancellationTokenToCreateBinddingCallback() }); await Assert.ThrowsAsync(() => - AddressBinder.BindAsync(options.ListenOptions, addressBindContext, new CancellationToken(true))); + AddressBinder.BindAsync(options.GetListenOptions(), addressBindContext, new CancellationToken(true))); } [Theory] @@ -284,7 +284,7 @@ public async Task FallbackToIPv4WhenIPv6AnyBindFails(string address) return Task.CompletedTask; }); - await AddressBinder.BindAsync(options.ListenOptions, addressBindContext, CancellationToken.None); + await AddressBinder.BindAsync(options.GetListenOptions(), addressBindContext, CancellationToken.None); Assert.True(ipV4Attempt, "Should have attempted to bind to IPAddress.Any"); Assert.True(ipV6Attempt, "Should have attempted to bind to IPAddress.IPv6Any"); @@ -315,7 +315,7 @@ public async Task DefaultAddressBinderBindsToHttpPort5000() return Task.CompletedTask; }); - await AddressBinder.BindAsync(options.ListenOptions, addressBindContext, CancellationToken.None); + await AddressBinder.BindAsync(options.GetListenOptions(), addressBindContext, CancellationToken.None); Assert.Contains(endpoints, e => e.IPEndPoint.Port == 5000 && !e.IsTls); } diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index 8f683984f907..c6a373cf7f88 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -42,7 +42,7 @@ public void ConfigureNamedEndpoint_OnlyRunForMatchingConfig() .Endpoint("NotFound", endpointOptions => throw new NotImplementedException()) .Load(); - Assert.Single(serverOptions.ListenOptions); + Assert.Single(serverOptions.GetListenOptions()); Assert.Equal(5001, serverOptions.ConfigurationBackedListenOptions[0].IPEndPoint.Port); Assert.True(found); @@ -56,11 +56,11 @@ public void ConfigureEndpoint_OnlyRunWhenBuildIsCalled() serverOptions.Configure() .LocalhostEndpoint(5001, endpointOptions => run = true); - Assert.Empty(serverOptions.ListenOptions); + Assert.Empty(serverOptions.GetListenOptions()); serverOptions.ConfigurationLoader.Load(); - Assert.Single(serverOptions.ListenOptions); + Assert.Single(serverOptions.GetListenOptions()); Assert.Equal(5001, serverOptions.CodeBackedListenOptions[0].IPEndPoint.Port); Assert.True(run); @@ -73,18 +73,18 @@ public void CallBuildTwice_OnlyRunsOnce() var builder = serverOptions.Configure() .LocalhostEndpoint(5001); - Assert.Empty(serverOptions.ListenOptions); + Assert.Empty(serverOptions.GetListenOptions()); Assert.Equal(builder, serverOptions.ConfigurationLoader); builder.Load(); - Assert.Single(serverOptions.ListenOptions); + Assert.Single(serverOptions.GetListenOptions()); Assert.Equal(5001, serverOptions.CodeBackedListenOptions[0].IPEndPoint.Port); Assert.NotNull(serverOptions.ConfigurationLoader); builder.Load(); - Assert.Single(serverOptions.ListenOptions); + Assert.Single(serverOptions.GetListenOptions()); Assert.Equal(5001, serverOptions.CodeBackedListenOptions[0].IPEndPoint.Port); Assert.NotNull(serverOptions.ConfigurationLoader); } @@ -101,7 +101,7 @@ public void Configure_IsReplaceable() serverOptions.Configure(config1) .LocalhostEndpoint(5001, endpointOptions => run1 = true); - Assert.Empty(serverOptions.ListenOptions); + Assert.Empty(serverOptions.GetListenOptions()); Assert.False(run1); var run2 = false; @@ -114,7 +114,7 @@ public void Configure_IsReplaceable() serverOptions.ConfigurationLoader.Load(); - Assert.Equal(2, serverOptions.ListenOptions.Count()); + Assert.Equal(2, serverOptions.GetListenOptions().Length); Assert.Equal(5002, serverOptions.ConfigurationBackedListenOptions[0].IPEndPoint.Port); Assert.Equal(5003, serverOptions.CodeBackedListenOptions[0].IPEndPoint.Port); diff --git a/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs b/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs index ecb9260802a6..5d73e667c5a3 100644 --- a/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs +++ b/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs @@ -78,7 +78,7 @@ public TestServer(RequestDelegate app, TestServiceContext context, Action { configureKestrel(options); - _listenOptions = options.ListenOptions.First(); + _listenOptions = options.GetListenOptions().First(); }) .ConfigureServices(services => { diff --git a/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs b/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs index 9c265e115a46..7e28c4622244 100644 --- a/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs +++ b/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs @@ -328,7 +328,7 @@ private async Task RegisterIPEndPoint_Success(IPEndPoint endPoint, string testUr var testUrlWithPort = $"{testUrl}:{(testPort == 0 ? host.GetPort() : testPort)}"; var options = ((IOptions)host.Services.GetService(typeof(IOptions))).Value; - Assert.Single(options.ListenOptions); + Assert.Single(options.GetListenOptions()); var response = await HttpClientSlim.GetStringAsync(testUrlWithPort, validateCertificate: false);