Remove some Linq usages from Kestrel.Core (#47012)

* Remove some Linq usages from Kestrel.Core
dotnet · Mar 8, 2023 · 34a16dc · 34a16dc
1 parent 4e9a3e8
commit 34a16dc
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 24 deletions.
diff --git 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> 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
@@ -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<T>(IConnectionListener<T> connectionListener, Func<
 
     public Task StopEndpointsAsync(List<EndpointConfig> endpointsToStop, CancellationToken cancellationToken)
     {
-        var transportsToStop = _transports.Where(t => t.EndpointConfig != null && endpointsToStop.Contains(t.EndpointConfig)).ToList();
+        var transportsToStop = new List<ActiveTransport>();
+        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
@@ -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<EndpointConfig>(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
@@ -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<ListenOptions>();
+            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
@@ -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<ListenOptions> CodeBackedListenOptions { get; } = new List<ListenOptions>();
     internal List<ListenOptions> ConfigurationBackedListenOptions { get; } = new List<ListenOptions>();
-    internal IEnumerable<ListenOptions> ListenOptions => CodeBackedListenOptions.Concat(ConfigurationBackedListenOptions);
+
+    internal ListenOptions[] GetListenOptions()
+    {
+        int resultCount = CodeBackedListenOptions.Count + ConfigurationBackedListenOptions.Count;
+        if (resultCount == 0)
+        {
+            return Array.Empty<ListenOptions>();
+        }
+
+        var result = new ListenOptions[resultCount];
+        CodeBackedListenOptions.CopyTo(result);
+        ConfigurationBackedListenOptions.CopyTo(result, CodeBackedListenOptions.Count);
+        return result;
+    }
 
     // For testing and debugging.
     internal List<ListenOptions> OptionsInUse { get; } = new List<ListenOptions>();

diff --git 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<IOException>(() =>
-            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<OperationCanceledException>(() =>
-            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
@@ -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
@@ -78,7 +78,7 @@ public TestServer(RequestDelegate app, TestServiceContext context, Action<Kestre
                     .UseKestrel(options =>
                     {
                         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
@@ -328,7 +328,7 @@ private async Task RegisterIPEndPoint_Success(IPEndPoint endPoint, string testUr
             var testUrlWithPort = $"{testUrl}:{(testPort == 0 ? host.GetPort() : testPort)}";
 
             var options = ((IOptions<KestrelServerOptions>)host.Services.GetService(typeof(IOptions<KestrelServerOptions>))).Value;
-            Assert.Single(options.ListenOptions);
+            Assert.Single(options.GetListenOptions());
 
             var response = await HttpClientSlim.GetStringAsync(testUrlWithPort, validateCertificate: false);