Make KestrelConfigurationLoader.Load safe to re-call

src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs

@@ -8,6 +8,7 @@
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal;
 using Microsoft.AspNetCore.Server.Kestrel.Https;
 using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.Server.Kestrel;
 
@@ -18,7 +19,7 @@ public class KestrelConfigurationLoader
 {
     private readonly IHttpsConfigurationService _httpsConfigurationService;
 
-    private bool _loaded;
+    private IChangeToken? _reloadToken;
 
     internal KestrelConfigurationLoader(
         KestrelServerOptions options,
@@ -234,25 +235,27 @@ internal void ApplyHttpsDefaults(HttpsConnectionAdapterOptions httpsOptions)
     /// </summary>
     public void Load()
     {
-        if (_loaded)
+        if (_reloadToken is null || _reloadToken.HasChanged)
         {
-            // The loader has already been run.
-            return;
+            // Will update _reloadToken
+            _ = Reload();
         }
-        _loaded = true;
-
-        Reload();
 
         foreach (var action in EndpointsToAdd)
         {
             action();
         }
+
+        // If Load is called again, we don't want to rerun these
+        EndpointsToAdd.Clear();
     }
 
     // Adds endpoints from config to KestrelServerOptions.ConfigurationBackedListenOptions and configures some other options.
     // Any endpoints that were removed from the last time endpoints were loaded are returned.
     internal (List<ListenOptions>, List<ListenOptions>) Reload()
     {
+        _reloadToken = Configuration.GetReloadToken();
+
         var endpointsToStop = Options.ConfigurationBackedListenOptions.ToList();
         var endpointsToStart = new List<ListenOptions>();
         var endpointsToReuse = new List<ListenOptions>();

src/Servers/Kestrel/Core/test/KestrelServerTests.cs

@@ -958,8 +958,6 @@ public async Task DoesNotReloadOnConfigurationChangeByDefault()
         mockTransportFactory.Verify(f => f.BindAsync(new IPEndPoint(IPAddress.IPv6Any, 5000), It.IsAny<CancellationToken>()), Times.Once);
         mockTransportFactory.Verify(f => f.BindAsync(new IPEndPoint(IPAddress.IPv6Any, 5001), It.IsAny<CancellationToken>()), Times.Once);
 
-        mockConfig.Verify(c => c.GetReloadToken(), Times.Never);
-
         await server.StopAsync(CancellationToken.None).DefaultTimeout();
     }
 

src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs

@@ -14,6 +14,8 @@
 using Microsoft.Extensions.FileProviders;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Metrics;
+using Microsoft.Extensions.Primitives;
+using Moq;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Tests;
 
@@ -1224,6 +1226,125 @@ public void Reload_RerunsNamedEndpointConfigurationOnChange()
         Assert.Equal(1, foundUnchangedCount);
     }
 
+    [Fact]
+    public void MultipleLoads_ConfigureBetween()
+    {
+        var currentConfig = new ConfigurationBuilder().AddInMemoryCollection(new[]
+        {
+            new KeyValuePair<string, string>("Endpoints:A:Url", "http://*:5000"),
+            new KeyValuePair<string, string>("Endpoints:B:Url", "http://*:5001"),
+        }).Build();
+
+        var mockConfig = new Mock<IConfiguration>();
+        mockConfig.Setup(c => c.GetSection(It.IsAny<string>())).Returns<string>(currentConfig.GetSection);
+        mockConfig.Setup(c => c.GetChildren()).Returns(currentConfig.GetChildren);
+
+        var serverOptions = CreateServerOptions();
+        serverOptions.Configure(mockConfig.Object, reloadOnChange: false);
+        var oldConfigurationLoader = serverOptions.ConfigurationLoader;
+
+        serverOptions.ConfigurationLoader.Load();
+
+        mockConfig.Verify(c => c.GetSection(It.IsAny<string>()), Times.AtLeastOnce);
+
+        mockConfig.Invocations.Clear();
+
+        serverOptions.Configure(mockConfig.Object, reloadOnChange: false);
+        var newConfigurationLoader = serverOptions.ConfigurationLoader;
+        Assert.NotSame(oldConfigurationLoader, newConfigurationLoader);
+
+        serverOptions.ConfigurationLoader.Load();
+
+        // The change token is stored on the loader, so replacing the loader replaces the token.
+        mockConfig.Verify(c => c.GetSection(It.IsAny<string>()), Times.AtLeastOnce);
+    }
+
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public void MultipleLoads_ConfigurationChanges(bool changeConfiguration)
+    {
+        var currentConfig = new ConfigurationBuilder().AddInMemoryCollection(new[]
+        {
+            new KeyValuePair<string, string>("Endpoints:A:Url", "http://*:5000"),
+            new KeyValuePair<string, string>("Endpoints:B:Url", "http://*:5001"),
+        }).Build();
+
+        var mockReloadToken = new Mock<IChangeToken>();
+
+        var mockConfig = new Mock<IConfiguration>();
+        mockConfig.Setup(c => c.GetSection(It.IsAny<string>())).Returns<string>(currentConfig.GetSection);
+        mockConfig.Setup(c => c.GetChildren()).Returns(currentConfig.GetChildren);
+        mockConfig.Setup(c => c.GetReloadToken()).Returns(mockReloadToken.Object);
+
+        var serverOptions = CreateServerOptions();
+        serverOptions.Configure(mockConfig.Object, reloadOnChange: false);
+
+        serverOptions.ConfigurationLoader.Load();
+
+        mockReloadToken.VerifyGet(t => t.HasChanged, Times.Never);
+        mockConfig.Verify(c => c.GetSection(It.IsAny<string>()), Times.AtLeastOnce);
+
+        mockReloadToken.SetupGet(t => t.HasChanged).Returns(changeConfiguration);
+
+        mockReloadToken.Invocations.Clear();
+        mockConfig.Invocations.Clear();
+
+        serverOptions.ConfigurationLoader.Load();
+
+        mockReloadToken.VerifyGet(t => t.HasChanged, Times.AtLeastOnce);
+        mockConfig.Verify(c => c.GetSection(It.IsAny<string>()), changeConfiguration ? Times.AtLeastOnce : Times.Never);
+    }
+
+    [Theory]
+    [InlineData(0, 0)]
+    [InlineData(3, 0)]
+    [InlineData(2, 1)]
+    [InlineData(1, 2)]
+    [InlineData(0, 3)]
+    public void MultipleLoads_AddEndpoints(int beforeInitialLoadCount, int afterInitialLoadCount)
+    {
+        int _endpointAddedCount = 0;
+
+        var serverOptions = CreateServerOptions();
+        serverOptions.Configure();
+
+        for (int i = 0; i < beforeInitialLoadCount; i++)
+        {
+            serverOptions.ConfigurationLoader.LocalhostEndpoint(5000 + i, _ => _endpointAddedCount++);
+        }
+
+        serverOptions.ConfigurationLoader.Load();
+
+        Assert.Equal(beforeInitialLoadCount, _endpointAddedCount);
+        Assert.Equal(beforeInitialLoadCount, serverOptions.CodeBackedListenOptions.Count);
+        Assert.Empty(serverOptions.ConfigurationBackedListenOptions);
+
+        for (int i = 0; i < afterInitialLoadCount; i++)
+        {
+            serverOptions.ConfigurationLoader.LocalhostEndpoint(7000 + i, _ => _endpointAddedCount++);
+        }
+
+        serverOptions.ConfigurationLoader.Load();
+
+        Assert.Equal(beforeInitialLoadCount + afterInitialLoadCount, _endpointAddedCount);
+        Assert.Equal(beforeInitialLoadCount + afterInitialLoadCount, serverOptions.CodeBackedListenOptions.Count);
+        Assert.Empty(serverOptions.ConfigurationBackedListenOptions);
+    }
+
+    [Fact]
+    public void ReloadDoesNotAddEndpoints()
+    {
+        var serverOptions = CreateServerOptions();
+        serverOptions.Configure();
+
+        serverOptions.ConfigurationLoader.Load();
+
+        serverOptions.ConfigurationLoader.LocalhostEndpoint(7000, _ => Assert.Fail("New endpoints should not be added by Reload"));
+
+        _ = serverOptions.ConfigurationLoader.Reload();
+    }
+
     private static string GetCertificatePath()
     {
         var appData = Environment.GetEnvironmentVariable("APPDATA");