From 88c90489f0f12c82228a9ed2954e3f2ccf277baa Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 5 Dec 2024 16:17:06 -0800 Subject: [PATCH 1/9] whip --- .../AspNetCore/HandlerResolver.cpp | 7 + .../AspNetCore/HandlerResolver.h | 1 + .../AspNetCore/applicationmanager.cpp | 5 +- .../AspNetCore/applicationmanager.h | 10 ++ .../AspNetCore/globalmodule.cpp | 5 +- .../Infrastructure/EventLogHelpers.cs | 22 +-- .../Infrastructure/Helpers.cs | 4 +- .../Infrastructure/IISTestSiteFixture.cs | 2 +- .../MultiApplicationTests.cs | 2 +- .../MultipleAppTests.cs | 136 ++++++++++++++++++ .../RequestResponseTests.cs | 4 +- .../ApplicationInitializationTests.cs | 11 +- .../OutOfProcess/MultipleAppTests.cs | 61 -------- .../IntegrationTesting.IIS/src/IISDeployer.cs | 34 ++++- .../src/IISDeployerBase.cs | 24 +++- .../src/IISDeploymentParameterExtensions.cs | 22 +-- .../src/IISExpressDeployer.cs | 5 +- 17 files changed, 246 insertions(+), 109 deletions(-) create mode 100644 src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs delete mode 100644 src/Servers/IIS/IIS/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp index fcd4205138c9..bcfe6d6288be 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp @@ -204,6 +204,13 @@ std::chrono::milliseconds HandlerResolver::GetShutdownDelay() const return m_shutdownDelay; } +bool HandlerResolver::IsSameApplication(PCWSTR application) +{ + SRWExclusiveLock lock(m_requestHandlerLoadLock); + + return m_loadedApplicationId == application; +} + HRESULT HandlerResolver::FindNativeAssemblyFromGlobalLocation( const ShimOptions& pConfiguration, diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h index 54121f072cac..ee776c8abea0 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h @@ -20,6 +20,7 @@ class HandlerResolver APP_HOSTING_MODEL GetHostingModel(); bool GetDisallowRotationOnConfigChange(); std::chrono::milliseconds GetShutdownDelay() const; + bool IsSameApplication(PCWSTR application); private: HRESULT LoadRequestHandlerAssembly(const IHttpApplication &pApplication, const std::filesystem::path& shadowCopyPath, const ShimOptions& pConfiguration, std::unique_ptr& pApplicationFactory, ErrorContext& errorContext); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp index 48855946d29a..5c4272504e7f 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp @@ -123,7 +123,6 @@ APPLICATION_MANAGER::RecycleApplicationFromManager( else { ++itr; - } } @@ -149,8 +148,10 @@ APPLICATION_MANAGER::RecycleApplicationFromManager( { try { - if (UseLegacyShutdown()) + if (m_handlerResolver.GetHostingModel() == APP_HOSTING_MODEL::HOSTING_OUT_PROCESS + || UseLegacyShutdown()) { + // Only shutdown the specific application when using OutOfProcess instead of the whole w3wp process application->ShutDownApplication(/* fServerInitiated */ false); } else diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index efc466dc7ca9..e8200c49e2ef 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -57,6 +57,16 @@ class APPLICATION_MANAGER return m_handlerResolver.GetShutdownDelay() == std::chrono::milliseconds::zero(); } + APP_HOSTING_MODEL GetAppHostingModel() + { + return m_handlerResolver.GetHostingModel(); + } + + bool IsSameApplication(PCWSTR application) + { + return m_handlerResolver.IsSameApplication(application); + } + private: std::unordered_map> m_pApplicationInfoHash; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index 9e69d586cb80..1b77a6ecb7e7 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. See License.txt in the project root for license information. #include "globalmodule.h" +#include extern BOOL g_fInShutdown; @@ -44,7 +45,9 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop( // If we're already cleaned up just return. // If user has opted out of the new shutdown behavior ignore this call as we never registered for it before - if (!m_pApplicationManager || m_pApplicationManager->UseLegacyShutdown()) + if (!m_pApplicationManager || m_pApplicationManager->UseLegacyShutdown() + /*|| m_pApplicationManager->GetAppHostingModel() == APP_HOSTING_MODEL::HOSTING_OUT_PROCESS*/) + /*!m_pApplicationManager->IsSameApplication(pProvider->GetApplication()->GetApplicationId()))*/ { return GL_NOTIFICATION_CONTINUE; } diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/EventLogHelpers.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/EventLogHelpers.cs index 0c2eabceb0e6..c57e85b353c4 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/EventLogHelpers.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/EventLogHelpers.cs @@ -161,18 +161,18 @@ public static string InProcessStarted(IISDeploymentResult deploymentResult) public static string OutOfProcessStarted(IISDeploymentResult deploymentResult) { - return $"Application '/LM/W3SVC/1/ROOT' started process '\\d+' successfully and process '\\d+' is listening on port '\\d+'."; + return $"Application '/LM/W3SVC/\\d+/ROOT' started process '\\d+' successfully and process '\\d+' is listening on port '\\d+'."; } public static string InProcessFailedToStart(IISDeploymentResult deploymentResult, string reason) { if (DeployerSelector.HasNewHandler) { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to load coreclr. Exception message:\r\n{reason}"; + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to load coreclr. Exception message:\r\n{reason}"; } else { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to load clr and managed application. {reason}"; + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to load clr and managed application. {reason}"; } } @@ -184,7 +184,7 @@ public static string ShutdownMessage(IISDeploymentResult deploymentResult) } else { - return "Application '/LM/W3SVC/1/ROOT' with physical root '.*?' shut down process with Id '.*?' listening on port '.*?'"; + return "Application '/LM/W3SVC/\\d+/ROOT' with physical root '.*?' shut down process with Id '.*?' listening on port '.*?'"; } } @@ -200,29 +200,29 @@ public static string InProcessFailedToStop(IISDeploymentResult deploymentResult, public static string InProcessThreadException(IISDeploymentResult deploymentResult, string reason) { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed exception{reason}"; + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed exception{reason}"; } public static string InProcessThreadExit(IISDeploymentResult deploymentResult, string code) { if (DeployerSelector.HasNewHandler) { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' has exited from Program.Main with exit code = '{code}'. Please check the stderr logs for more information."; + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' has exited from Program.Main with exit code = '{code}'. Please check the stderr logs for more information."; } else { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed background thread exit, exit code = '{code}'."; + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed background thread exit, exit code = '{code}'."; } } public static string InProcessThreadExitStdOut(IISDeploymentResult deploymentResult, string code, string output) { if (DeployerSelector.HasNewHandler) { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' has exited from Program.Main with exit code = '{code}'. First 30KB characters of captured stdout and stderr logs:\r\n{output}"; + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' has exited from Program.Main with exit code = '{code}'. First 30KB characters of captured stdout and stderr logs:\r\n{output}"; } else { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed background thread exit, exit code = '{code}'. Last 4KB characters of captured stdout and stderr logs:\r\n{output}"; + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed background thread exit, exit code = '{code}'. Last 4KB characters of captured stdout and stderr logs:\r\n{output}"; } } @@ -247,13 +247,13 @@ public static string OutOfProcessFailedToStart(IISDeploymentResult deploymentRes { if (DeployerSelector.HasNewShim) { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to start process with " + + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to start process with " + $"commandline '(.*)' with multiple retries. " + $"Failed to bind to port '(.*)'. First 30KB characters of captured stdout and stderr logs from multiple retries:\r\n{output}"; } else { - return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to start process with " + + return $"Application '/LM/W3SVC/\\d+/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to start process with " + $"commandline '(.*)' with multiple retries. " + $"The last try of listening port is '(.*)'. See previous warnings for details."; } diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/Helpers.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/Helpers.cs index 74f499ce488e..7c537166f57b 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/Helpers.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/Helpers.cs @@ -72,7 +72,7 @@ public static string GetFrebFolder(string folder, IISDeploymentResult result) } else { - return Path.Combine(folder, "W3SVC1"); + return Path.Combine(folder, $"W3SVC{result.DeploymentParameters.SiteName}"); } } @@ -256,7 +256,7 @@ public static string CreateEmptyApplication(XElement config, string contentRoot) var siteElement = config .RequiredElement("system.applicationHost") .RequiredElement("sites") - .RequiredElement("site"); + .Elements("site").Last(); var application = siteElement .RequiredElement("application"); diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/IISTestSiteFixture.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/IISTestSiteFixture.cs index ec04ad06ba84..6de947af7347 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/IISTestSiteFixture.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Infrastructure/IISTestSiteFixture.cs @@ -51,7 +51,7 @@ internal IISTestSiteFixture(Action configure) //DeploymentParameters.EnvironmentVariables.Add("ASPNETCORE_MODULE_DEBUG", "console"); // This queue does not have websockets enabled currently, adding the module will break all tests using this fixture. - if (!HelixHelper.GetTargetHelixQueue().ToLowerInvariant().Contains("windows.amd64.server2022")) + if (!(HelixHelper.GetTargetHelixQueue() ?? "").ToLowerInvariant().Contains("windows.amd64.server2022")) { DeploymentParameters.EnableModule("WebSocketModule", "%IIS_BIN%/iiswsock.dll"); } diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultiApplicationTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultiApplicationTests.cs index 891eafdacb6d..4ad541a6871c 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultiApplicationTests.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultiApplicationTests.cs @@ -114,7 +114,7 @@ private void DuplicateApplication(XElement config, string contentRoot) var siteElement = config .RequiredElement("system.applicationHost") .RequiredElement("sites") - .RequiredElement("site"); + .Elements("site").Last(); var application = siteElement .RequiredElement("application"); diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs new file mode 100644 index 000000000000..fda5a6075419 --- /dev/null +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs @@ -0,0 +1,136 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.IIS.FunctionalTests; +using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; +using Microsoft.AspNetCore.Server.IntegrationTesting; +using Microsoft.AspNetCore.InternalTesting; +using Xunit; +using System.Globalization; +using System.Diagnostics; +using System.IO; + +#if !IIS_FUNCTIONALS +using Microsoft.AspNetCore.Server.IIS.FunctionalTests; + +#if IISEXPRESS_FUNCTIONALS +namespace Microsoft.AspNetCore.Server.IIS.IISExpress.FunctionalTests; +#elif NEWHANDLER_FUNCTIONALS +namespace Microsoft.AspNetCore.Server.IIS.NewHandler.FunctionalTests; +#elif NEWSHIM_FUNCTIONALS +namespace Microsoft.AspNetCore.Server.IIS.NewShim.FunctionalTests; +#endif + +#else +namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests; +#endif + +[Collection(PublishedSitesCollection.Name)] +[SkipOnHelix("Unsupported queue", Queues = "Windows.Amd64.VS2022.Pre.Open;")] +public class MultipleAppTests : IISFunctionalTestBase +{ + public MultipleAppTests(PublishedSitesFixture fixture) : base(fixture) + { + } + + [ConditionalFact] + public async Task Startup() + { + const int numApps = 10; + + using (var deployers = new DisposableList()) + { + var deploymentResults = new List(); + + var deploymentParameters = Fixture.GetBaseDeploymentParameters(hostingModel: HostingModel.OutOfProcess); + var deployer = CreateDeployer(deploymentParameters); + deployers.Add(deployer); + + // Deploy all apps + for (var i = 0; i < numApps; i++) + { + deploymentResults.Add(await deployer.DeployAsync()); + } + + // Start all requests as quickly as possible, so apps are started as quickly as possible, + // to test possible race conditions when multiple apps start at the same time. + var requestTasks = new List>(); + foreach (var deploymentResult in deploymentResults) + { + requestTasks.Add(deploymentResult.HttpClient.RetryRequestAsync("/HelloWorld", r => r.IsSuccessStatusCode)); + } + + // Verify all apps started and return expected response + foreach (var requestTask in requestTasks) + { + var response = await requestTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var responseText = await response.Content.ReadAsStringAsync(); + Assert.Equal("Hello World", responseText); + } + } + } + + [ConditionalFact] + public async Task Restart() + { + const int numApps = 10; + + using (var deployers = new DisposableList()) + { + var deploymentResults = new List(); + + var deploymentParameters = Fixture.GetBaseDeploymentParameters(hostingModel: HostingModel.OutOfProcess); + var deployer = CreateDeployer(deploymentParameters); + deployers.Add(deployer); + + // Deploy all apps + for (var i = 0; i < numApps; i++) + { + deploymentResults.Add(await deployer.DeployAsync()); + } + + // Start all requests as quickly as possible, so apps are started as quickly as possible, + // to test possible race conditions when multiple apps start at the same time. + var requestTasks = new List>(); + foreach (var deploymentResult in deploymentResults) + { + requestTasks.Add(deploymentResult.HttpClient.RetryRequestAsync("/ProcessId", r => r.IsSuccessStatusCode)); + } + + List processIDs = new(); + // Verify all apps started and return expected response + foreach (var requestTask in requestTasks) + { + var response = await requestTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var responseText = await response.Content.ReadAsStringAsync(); + processIDs.Add(int.Parse(responseText, CultureInfo.InvariantCulture)); + } + + // Just "touching" web.config should be enough to restart the process + deploymentResults[0].ModifyWebConfig(_ => { }); + + // Need to give time for process to start and finish restarting + await deploymentResults[0].HttpClient.RetryRequestAsync("/ProcessId", + async r => int.Parse(await r.Content.ReadAsStringAsync(), CultureInfo.InvariantCulture) != processIDs[0]); + + // First process should have restarted + var res = await deploymentResults[0].HttpClient.RetryRequestAsync("/ProcessId", r => r.IsSuccessStatusCode); + + Assert.NotEqual(processIDs[0], int.Parse(await res.Content.ReadAsStringAsync(), CultureInfo.InvariantCulture)); + + // Every other process should stay the same + for (var i = 1; i < deploymentResults.Count; i++) + { + res = await deploymentResults[i].HttpClient.GetAsync("/ProcessId"); + Assert.Equal(processIDs[i], int.Parse(await res.Content.ReadAsStringAsync(), CultureInfo.InvariantCulture)); + } + } + } +} diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs index 4428a6d1a6dd..5c00a7398c49 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs @@ -620,13 +620,13 @@ public async Task IISEnvironmentFeatureIsAvailable(string endpoint) var expected = $""" IIS Version: 10.0 - ApplicationId: /LM/W3SVC/1/ROOT + ApplicationId: /LM/W3SVC/{siteName}/ROOT Application Path: {_fixture.DeploymentResult.ContentRoot}\ Application Virtual Path: / Application Config Path: MACHINE/WEBROOT/APPHOST/{siteName} AppPool ID: {_fixture.DeploymentResult.AppPoolName} AppPool Config File: {_fixture.DeploymentResult.DeploymentParameters.ServerConfigLocation} - Site ID: 1 + Site ID: {siteName} Site Name: {siteName} """; diff --git a/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/ApplicationInitializationTests.cs b/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/ApplicationInitializationTests.cs index 2a8f62afd5d4..861a1deb295b 100644 --- a/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/ApplicationInitializationTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/ApplicationInitializationTests.cs @@ -113,12 +113,15 @@ private static void EnablePreload(IISDeploymentParameters baseDeploymentParamete (config, _) => { - config + foreach (var site in config .RequiredElement("system.applicationHost") .RequiredElement("sites") - .RequiredElement("site") - .RequiredElement("application") - .SetAttributeValue("preloadEnabled", true); + .Elements("site")) + { + site + .RequiredElement("application") + .SetAttributeValue("preloadEnabled", true); + } }); baseDeploymentParameters.EnableModule("ApplicationInitializationModule", "%IIS_BIN%\\warmup.dll"); diff --git a/src/Servers/IIS/IIS/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs b/src/Servers/IIS/IIS/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs deleted file mode 100644 index 17b6bd47d615..000000000000 --- a/src/Servers/IIS/IIS/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs +++ /dev/null @@ -1,61 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Generic; -using System.Net; -using System.Net.Http; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.IIS.FunctionalTests; -using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; -using Microsoft.AspNetCore.Server.IntegrationTesting; -using Microsoft.AspNetCore.InternalTesting; -using Xunit; - -namespace Microsoft.AspNetCore.Server.IIS.IISExpress.FunctionalTests; - -[Collection(PublishedSitesCollection.Name)] -[SkipOnHelix("Unsupported queue", Queues = "Windows.Amd64.VS2022.Pre.Open;")] -public class MultipleAppTests : IISFunctionalTestBase -{ - public MultipleAppTests(PublishedSitesFixture fixture) : base(fixture) - { - } - - [ConditionalFact] - public async Task Startup() - { - const int numApps = 10; - - using (var deployers = new DisposableList()) - { - var deploymentResults = new List(); - - // Deploy all apps - for (var i = 0; i < numApps; i++) - { - var deploymentParameters = Fixture.GetBaseDeploymentParameters(hostingModel: IntegrationTesting.HostingModel.OutOfProcess); - var deployer = CreateDeployer(deploymentParameters); - deployers.Add(deployer); - deploymentResults.Add(await deployer.DeployAsync()); - } - - // Start all requests as quickly as possible, so apps are started as quickly as possible, - // to test possible race conditions when multiple apps start at the same time. - var requestTasks = new List>(); - foreach (var deploymentResult in deploymentResults) - { - requestTasks.Add(deploymentResult.HttpClient.GetAsync("/HelloWorld")); - } - - // Verify all apps started and return expected response - foreach (var requestTask in requestTasks) - { - var response = await requestTask; - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - var responseText = await response.Content.ReadAsStringAsync(); - Assert.Equal("Hello World", responseText); - } - } - } -} diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs index f3db8e12c866..69af33569557 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs @@ -30,6 +30,7 @@ public class IISDeployer : IISDeployerBase private string _debugLogFile; private string _appPoolName; private bool _disposed; + private int _siteId = 5; public Process HostProcess { get; set; } @@ -305,17 +306,35 @@ private static Site FindSite(ServerManager serverManager, string contentRoot) private void AddTemporaryAppHostConfig(string contentRoot, int port) { - _configPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("D")); - _applicationHostConfig = Path.Combine(_configPath, "applicationHost.config"); - Directory.CreateDirectory(_configPath); - var config = XDocument.Parse(DeploymentParameters.ServerConfigTemplateContent ?? File.ReadAllText("IIS.config")); + var multiSite = true; + XDocument config; + if (_applicationHostConfig is not null) + { + //Debugger.Launch(); + config = XDocument.Parse(File.ReadAllText(_applicationHostConfig)); + } + else + { + multiSite = false; + _configPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("D")); + _applicationHostConfig = Path.Combine(_configPath, "applicationHost.config"); + Directory.CreateDirectory(_configPath); + config = XDocument.Parse(DeploymentParameters.ServerConfigTemplateContent ?? File.ReadAllText("IIS.config")); + } - ConfigureAppHostConfig(config.Root, contentRoot, port); + ConfigureAppHostConfig(config.Root, contentRoot, port, _siteId); + _siteId++; config.Save(_applicationHostConfig); + if (multiSite) + { + return; + } + RetryServerManagerAction(serverManager => { + //Debugger.Launch(); var redirectionConfiguration = serverManager.GetRedirectionConfiguration(); var redirectionSection = redirectionConfiguration.GetSection("configurationRedirection"); @@ -342,9 +361,9 @@ private void AddTemporaryAppHostConfig(string contentRoot, int port) }); } - private void ConfigureAppHostConfig(XElement config, string contentRoot, int port) + private void ConfigureAppHostConfig(XElement config, string contentRoot, int port, int siteId) { - ConfigureModuleAndBinding(config, contentRoot, port); + ConfigureModuleAndBinding(config, contentRoot, port, siteId); // In IISExpress system.webServer/modules in under location element config @@ -521,6 +540,7 @@ private static string DumpServerManagerConfig() var configDump = new StringBuilder(); using (var serverManager = new ServerManager()) { + //Debugger.Launch(); foreach (var site in serverManager.Sites) { configDump.AppendLine(CultureInfo.InvariantCulture, $"Site Name:{site.Name} Id:{site.Id} State:{site.State}"); diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs index 7502bf95ab88..7b476dd54874 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Globalization; using System.Xml.Linq; using Microsoft.Extensions.Logging; @@ -136,23 +137,34 @@ private void AddHandlerSettings(XElement element, string contentRoot) } } - protected void ConfigureModuleAndBinding(XElement config, string contentRoot, int port) + protected void ConfigureModuleAndBinding(XElement config, string contentRoot, int port, int siteId) { var siteElement = config .RequiredElement("system.applicationHost") .RequiredElement("sites") .RequiredElement("site"); - siteElement - .RequiredElement("application") + var newSiteElement = new XElement(siteElement); + newSiteElement.SetAttributeValue("name", $"{siteId}"); + + newSiteElement + .SetAttributeValue("id", $"{siteId}"); + + newSiteElement + .Element("application") .RequiredElement("virtualDirectory") .SetAttributeValue("physicalPath", contentRoot); - siteElement - .RequiredElement("bindings") - .RequiredElement("binding") + newSiteElement + .GetOrAdd("bindings") + .GetOrAdd("binding", "protocol", "http") .SetAttributeValue("bindingInformation", $":{port}:localhost"); + config + .RequiredElement("system.applicationHost") + .RequiredElement("sites") + .Add(newSiteElement); + config .RequiredElement("system.webServer") .RequiredElement("globalModules") diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeploymentParameterExtensions.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeploymentParameterExtensions.cs index a2361943e4c5..261b0965c97a 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeploymentParameterExtensions.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeploymentParameterExtensions.cs @@ -28,13 +28,15 @@ public static void AddHttpsToServerConfig(this IISDeploymentParameters parameter parameters.AddServerConfigAction( element => { - element.Descendants("binding") - .Single() - .SetAttributeValue("protocol", "https"); + foreach (var binding in element.Descendants("binding")) + { + binding.SetAttributeValue("protocol", "https"); + } - element.Descendants("access") - .Single() - .SetAttributeValue("sslFlags", "None"); + foreach (var access in element.Descendants("access")) + { + access.SetAttributeValue("sslFlags", "None"); + } }); } @@ -43,9 +45,11 @@ public static void AddHttpsWithClientCertToServerConfig(this IISDeploymentParame parameters.AddServerConfigAction( element => { - element.Descendants("binding") - .Single() - .SetAttributeValue("protocol", "https"); + foreach (var binding in element.Descendants("binding")) + { + binding + .SetAttributeValue("protocol", "https"); + } element.Descendants("access") .Single() diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs index 17e30ee84205..e620e5f0c766 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs @@ -169,7 +169,8 @@ private string CheckIfPublishIsRequired() var parameters = string.IsNullOrEmpty(DeploymentParameters.ServerConfigLocation) ? string.Format(CultureInfo.InvariantCulture, "/port:{0} /path:\"{1}\" /trace:error /systray:false", uri.Port, contentRoot) : - string.Format(CultureInfo.InvariantCulture, "/site:{0} /config:{1} /trace:error /systray:false", DeploymentParameters.SiteName, DeploymentParameters.ServerConfigLocation); + string.Format(CultureInfo.InvariantCulture, "/site:{0} /config:{1} /trace:error /systray:false", "5", DeploymentParameters.ServerConfigLocation); + DeploymentParameters.SiteName = "5"; Logger.LogInformation("Executing command : {iisExpress} {parameters}", iisExpressPath, parameters); @@ -293,7 +294,7 @@ private void PrepareConfig(string contentRoot, int port) .RequiredElement("modules") .GetOrAdd("add", "name", AspNetCoreModuleV2ModuleName); - ConfigureModuleAndBinding(config.Root, contentRoot, port); + ConfigureModuleAndBinding(config.Root, contentRoot, port, 5); var webConfigPath = Path.Combine(contentRoot, "web.config"); if (!DeploymentParameters.PublishApplicationBeforeDeployment && !File.Exists(webConfigPath)) From e8c1033e705b68088032f82702c5fc6f6279dbce Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 6 Dec 2024 14:06:10 -0800 Subject: [PATCH 2/9] fixuo --- .../IIS/test/Common.LongTests/StartupTests.cs | 55 +++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/Servers/IIS/IIS/test/Common.LongTests/StartupTests.cs b/src/Servers/IIS/IIS/test/Common.LongTests/StartupTests.cs index e47c3bdf4ec9..c6b6f25743ad 100644 --- a/src/Servers/IIS/IIS/test/Common.LongTests/StartupTests.cs +++ b/src/Servers/IIS/IIS/test/Common.LongTests/StartupTests.cs @@ -1311,7 +1311,10 @@ public async Task ServerAddressesIncludesBaseAddress() deploymentParameters.AddServerConfigAction( (element, root) => { - element.Descendants("site").Single().Element("application").SetAttributeValue("path", "/" + appName); + foreach (var site in element.Descendants("site")) + { + site.Element("application").SetAttributeValue("path", "/" + appName); + } Helpers.CreateEmptyApplication(element, root); }); @@ -1330,10 +1333,12 @@ public async Task AncmHttpsPortCanBeOverriden() deploymentParameters.AddServerConfigAction( element => { - element.Descendants("bindings") - .Single() - .GetOrAdd("binding", "protocol", "https") - .SetAttributeValue("bindingInformation", $":{TestPortHelper.GetNextSSLPort()}:localhost"); + foreach (var binding in element.Descendants("bindings")) + { + binding + .GetOrAdd("binding", "protocol", "https") + .SetAttributeValue("bindingInformation", $":{TestPortHelper.GetNextSSLPort()}:localhost"); + } }); deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_ANCM_HTTPS_PORT"] = "123"; @@ -1357,14 +1362,18 @@ public async Task HttpsRedirectionWorksIn30AndNot22() deploymentParameters.AddServerConfigAction( element => { - element.Descendants("bindings") - .Single() - .AddAndGetInnerElement("binding", "protocol", "https") - .SetAttributeValue("bindingInformation", $":{port}:localhost"); - - element.Descendants("access") - .Single() - .SetAttributeValue("sslFlags", "None"); + foreach (var binding in element.Descendants("bindings")) + { + binding + .AddAndGetInnerElement("binding", "protocol", "https") + .SetAttributeValue("bindingInformation", $":{port}:localhost"); + } + + foreach (var access in element.Descendants("access")) + { + access + .SetAttributeValue("sslFlags", "None"); + } }); var deploymentResult = await DeployAsync(deploymentParameters); @@ -1404,15 +1413,17 @@ public async Task MultipleHttpsPortsProduceNoEnvVar() deploymentParameters.AddServerConfigAction( element => { - element.Descendants("bindings") - .Single() - .Add( - new XElement("binding", - new XAttribute("protocol", "https"), - new XAttribute("bindingInformation", $":{sslPort}:localhost")), - new XElement("binding", - new XAttribute("protocol", "https"), - new XAttribute("bindingInformation", $":{anotherSslPort}:localhost"))); + foreach (var binding in element.Descendants("bindings")) + { + binding + .Add( + new XElement("binding", + new XAttribute("protocol", "https"), + new XAttribute("bindingInformation", $":{sslPort}:localhost")), + new XElement("binding", + new XAttribute("protocol", "https"), + new XAttribute("bindingInformation", $":{anotherSslPort}:localhost"))); + } }); var deploymentResult = await DeployAsync(deploymentParameters); From 82bfbcaaa20180bedc9e4108aea3fbc3d68ade4a Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 23 Dec 2024 11:13:45 -0800 Subject: [PATCH 3/9] ignore config --- .../AspNetCore/applicationmanager.cpp | 4 ++++ .../AspNetCore/applicationmanager.h | 14 +++++++++++++- .../AspNetCoreModuleV2/AspNetCore/globalmodule.cpp | 11 +++++++---- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp index 5c4272504e7f..5f9224e39c76 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp @@ -22,6 +22,10 @@ APPLICATION_MANAGER::GetOrCreateApplicationInfo( _Out_ std::shared_ptr& ppApplicationInfo ) { + // GetOrCreateApplicationInfo is called from proxymodule when a request is received. + // Set this value to indicate that a request has been received so we can disable shutdown logic in OnGlobalApplicationStop + m_hasStarted = true; + auto &pApplication = *pHttpContext.GetApplication(); // The configuration path is unique for each application and is used for the diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index e8200c49e2ef..068d7096f3dc 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -36,7 +36,8 @@ class APPLICATION_MANAGER m_pApplicationInfoHash(NULL), m_fDebugInitialize(FALSE), m_pHttpServer(pHttpServer), - m_handlerResolver(hModule, pHttpServer) + m_handlerResolver(hModule, pHttpServer), + m_hasStarted(false) { InitializeSRWLock(&m_srwLock); } @@ -67,6 +68,16 @@ class APPLICATION_MANAGER return m_handlerResolver.IsSameApplication(application); } + bool IsIISExpress() const + { + return m_pHttpServer.IsCommandLineLaunch(); + } + + bool HasReceivedRequest() const + { + return m_hasStarted; + } + private: std::unordered_map> m_pApplicationInfoHash; @@ -74,4 +85,5 @@ class APPLICATION_MANAGER BOOL m_fDebugInitialize; IHttpServer &m_pHttpServer; HandlerResolver m_handlerResolver; + bool m_hasStarted; }; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index 1b77a6ecb7e7..d8c4fd7027b1 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -45,16 +45,15 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop( // If we're already cleaned up just return. // If user has opted out of the new shutdown behavior ignore this call as we never registered for it before - if (!m_pApplicationManager || m_pApplicationManager->UseLegacyShutdown() - /*|| m_pApplicationManager->GetAppHostingModel() == APP_HOSTING_MODEL::HOSTING_OUT_PROCESS*/) - /*!m_pApplicationManager->IsSameApplication(pProvider->GetApplication()->GetApplicationId()))*/ + if (!m_pApplicationManager || m_pApplicationManager->UseLegacyShutdown()) { return GL_NOTIFICATION_CONTINUE; } LOG_INFO(L"ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop"); - if (!g_fInShutdown && !m_shutdown.joinable()) + if (!g_fInShutdown && !m_shutdown.joinable() + /*&& (m_pApplicationManager->IsIISExpress() || !m_pApplicationManager->HasReceivedRequest())*/) { // Apps with preload + always running that don't receive a request before recycle/shutdown will never call OnGlobalStopListening // IISExpress can also close without calling OnGlobalStopListening which is where we usually would trigger shutdown @@ -95,6 +94,10 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalConfigurationChange( m_pApplicationManager->RecycleApplicationFromManager(pwszChangePath); } } + else + { + LOG_INFOF(L"Ignoring configuration change", pwszChangePath); + } // Return processing to the pipeline. return GL_NOTIFICATION_CONTINUE; From 4fddee1a649063ddbfe281cab13dbe09e0ca17ed Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 21 Jan 2025 16:10:26 -0800 Subject: [PATCH 4/9] Fixups --- .../AspNetCore/HandlerResolver.cpp | 7 ------ .../AspNetCore/HandlerResolver.h | 1 - .../AspNetCore/applicationmanager.h | 5 ---- .../AspNetCore/globalmodule.cpp | 20 ++++++++-------- .../test/Common.LongTests/ShutdownTests.cs | 24 ++++++++++++++----- .../IntegrationTesting.IIS/src/Http.config | 4 ++-- 6 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp index bcfe6d6288be..fcd4205138c9 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp @@ -204,13 +204,6 @@ std::chrono::milliseconds HandlerResolver::GetShutdownDelay() const return m_shutdownDelay; } -bool HandlerResolver::IsSameApplication(PCWSTR application) -{ - SRWExclusiveLock lock(m_requestHandlerLoadLock); - - return m_loadedApplicationId == application; -} - HRESULT HandlerResolver::FindNativeAssemblyFromGlobalLocation( const ShimOptions& pConfiguration, diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h index ee776c8abea0..54121f072cac 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h @@ -20,7 +20,6 @@ class HandlerResolver APP_HOSTING_MODEL GetHostingModel(); bool GetDisallowRotationOnConfigChange(); std::chrono::milliseconds GetShutdownDelay() const; - bool IsSameApplication(PCWSTR application); private: HRESULT LoadRequestHandlerAssembly(const IHttpApplication &pApplication, const std::filesystem::path& shadowCopyPath, const ShimOptions& pConfiguration, std::unique_ptr& pApplicationFactory, ErrorContext& errorContext); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index 068d7096f3dc..dab402a97e67 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -63,11 +63,6 @@ class APPLICATION_MANAGER return m_handlerResolver.GetHostingModel(); } - bool IsSameApplication(PCWSTR application) - { - return m_handlerResolver.IsSameApplication(application); - } - bool IsIISExpress() const { return m_pHttpServer.IsCommandLineLaunch(); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index d8c4fd7027b1..8f7131aa0985 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -2,7 +2,6 @@ // Licensed under the MIT License. See License.txt in the project root for license information. #include "globalmodule.h" -#include extern BOOL g_fInShutdown; @@ -53,13 +52,17 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop( LOG_INFO(L"ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop"); if (!g_fInShutdown && !m_shutdown.joinable() - /*&& (m_pApplicationManager->IsIISExpress() || !m_pApplicationManager->HasReceivedRequest())*/) + && (m_pApplicationManager->IsIISExpress() || !m_pApplicationManager->HasReceivedRequest())) { // Apps with preload + always running that don't receive a request before recycle/shutdown will never call OnGlobalStopListening // IISExpress can also close without calling OnGlobalStopListening which is where we usually would trigger shutdown // so we should make sure to shutdown the server in those cases StartShutdown(); } + else + { + LOG_INFOF(L"Ignoring OnGlobalApplicationStop, OnGlobalStopListening should be called shortly."); + } return GL_NOTIFICATION_CONTINUE; } @@ -84,19 +87,16 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalConfigurationChange( // Test for an error. if (nullptr != pwszChangePath && - _wcsicmp(pwszChangePath, L"MACHINE") != 0 && - _wcsicmp(pwszChangePath, L"MACHINE/WEBROOT") != 0) - { + _wcsicmp(pwszChangePath, L"MACHINE/WEBROOT") > 0 && // Configuration change recycling behavior can be turned off via setting disallowRotationOnConfigChange=true on the handler settings section. // We need this duplicate setting because the global module is unable to read the app settings disallowRotationOnConfigChange value. - if (m_pApplicationManager && m_pApplicationManager->ShouldRecycleOnConfigChange()) - { - m_pApplicationManager->RecycleApplicationFromManager(pwszChangePath); - } + m_pApplicationManager && m_pApplicationManager->ShouldRecycleOnConfigChange()) + { + m_pApplicationManager->RecycleApplicationFromManager(pwszChangePath); } else { - LOG_INFOF(L"Ignoring configuration change", pwszChangePath); + LOG_INFOF(L"Ignoring configuration change for '%ls'", pwszChangePath); } // Return processing to the pipeline. diff --git a/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs b/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs index a23279865f02..1a16588c980f 100644 --- a/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs +++ b/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs @@ -493,9 +493,15 @@ public async Task ConfigurationChangeCanBeIgnoredInProcess() // Just "touching" web.config should be enough deploymentResult.ModifyWebConfig(element => { }); - // Have to retry here to allow ANCM to receive notification and react to it - // Verify that worker process does not get restarted with new process id - await deploymentResult.HttpClient.RetryRequestAsync("/ProcessId", async r => await r.Content.ReadAsStringAsync() == processBefore); + // need some reasonable delay here to ensure the app has time to react to the file change notification + // In this case it should ignore it, but it's hard to test that the app doesn't do anything in reaction to the file change. + for (var i = 0; i < 2000; i++) + { + using var res = await deploymentResult.HttpClient.GetAsync("/ProcessId"); + await Task.Delay(1); + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + Assert.Equal(processBefore, await res.Content.ReadAsStringAsync()); + } } [ConditionalFact] @@ -513,9 +519,15 @@ public async Task AppHostConfigurationChangeIsIgnoredInProcess() // Just "touching" applicationHost.config should be enough _deployer.ModifyApplicationHostConfig(element => { }); - // Have to retry here to allow ANCM to receive notification and react to it - // Verify that worker process does not get restarted with new process id - await deploymentResult.HttpClient.RetryRequestAsync("/ProcessId", async r => await r.Content.ReadAsStringAsync() == processBefore); + // need some reasonable delay here to ensure the app has time to react to the file change notification + // In this case it should ignore it, but it's hard to test that the app doesn't do anything in reaction to the file change. + for (var i = 0; i < 2000; i++) + { + using var res = await deploymentResult.HttpClient.GetAsync("/ProcessId"); + await Task.Delay(1); + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + Assert.Equal(processBefore, await res.Content.ReadAsStringAsync()); + } } [ConditionalFact] diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/Http.config b/src/Servers/IIS/IntegrationTesting.IIS/src/Http.config index 8c74496e1224..7f14a0fc4a65 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/Http.config +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/Http.config @@ -164,8 +164,8 @@ - - + + From 127907b80fd3a06db80e523a98e0abca37f825f0 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 22 Jan 2025 10:41:00 -0800 Subject: [PATCH 5/9] cleanup --- .../IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h | 5 ----- .../IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp | 2 +- .../IIS/test/Common.FunctionalTests/MultipleAppTests.cs | 4 ++-- src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs | 7 +++---- .../IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs | 7 ++++--- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index dab402a97e67..dbfd7e96cb33 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -58,11 +58,6 @@ class APPLICATION_MANAGER return m_handlerResolver.GetShutdownDelay() == std::chrono::milliseconds::zero(); } - APP_HOSTING_MODEL GetAppHostingModel() - { - return m_handlerResolver.GetHostingModel(); - } - bool IsIISExpress() const { return m_pHttpServer.IsCommandLineLaunch(); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index 8f7131aa0985..a00ab1145d7d 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -61,7 +61,7 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop( } else { - LOG_INFOF(L"Ignoring OnGlobalApplicationStop, OnGlobalStopListening should be called shortly."); + LOG_INFO(L"Ignoring OnGlobalApplicationStop, OnGlobalStopListening should be called shortly."); } return GL_NOTIFICATION_CONTINUE; diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs index fda5a6075419..1893d3623b02 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs @@ -39,7 +39,7 @@ public MultipleAppTests(PublishedSitesFixture fixture) : base(fixture) } [ConditionalFact] - public async Task Startup() + public async Task StartupManyAppsSuccessful() { const int numApps = 10; @@ -77,7 +77,7 @@ public async Task Startup() } [ConditionalFact] - public async Task Restart() + public async Task RestartAppShouldNotAffectOtherApps() { const int numApps = 10; diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs index 69af33569557..3f56b6a7d151 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs @@ -30,7 +30,8 @@ public class IISDeployer : IISDeployerBase private string _debugLogFile; private string _appPoolName; private bool _disposed; - private int _siteId = 5; + // Start at 2 since 1 is used by the our default site in Http.config + private int _siteId = 2; public Process HostProcess { get; set; } @@ -310,7 +311,6 @@ private void AddTemporaryAppHostConfig(string contentRoot, int port) XDocument config; if (_applicationHostConfig is not null) { - //Debugger.Launch(); config = XDocument.Parse(File.ReadAllText(_applicationHostConfig)); } else @@ -327,6 +327,7 @@ private void AddTemporaryAppHostConfig(string contentRoot, int port) config.Save(_applicationHostConfig); + // Don't need to setup the config redirection since the first site configured already did it for this config file if (multiSite) { return; @@ -334,7 +335,6 @@ private void AddTemporaryAppHostConfig(string contentRoot, int port) RetryServerManagerAction(serverManager => { - //Debugger.Launch(); var redirectionConfiguration = serverManager.GetRedirectionConfiguration(); var redirectionSection = redirectionConfiguration.GetSection("configurationRedirection"); @@ -540,7 +540,6 @@ private static string DumpServerManagerConfig() var configDump = new StringBuilder(); using (var serverManager = new ServerManager()) { - //Debugger.Launch(); foreach (var site in serverManager.Sites) { configDump.AppendLine(CultureInfo.InvariantCulture, $"Site Name:{site.Name} Id:{site.Id} State:{site.State}"); diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs index e620e5f0c766..e4910d626d2e 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISExpressDeployer.cs @@ -167,10 +167,11 @@ private string CheckIfPublishIsRequired() Logger.LogInformation("Attempting to start IIS Express on port: {port}", port); PrepareConfig(contentRoot, port); + // Use 2 since 1 is used by the our default site in Http.config + DeploymentParameters.SiteName = "2"; var parameters = string.IsNullOrEmpty(DeploymentParameters.ServerConfigLocation) ? string.Format(CultureInfo.InvariantCulture, "/port:{0} /path:\"{1}\" /trace:error /systray:false", uri.Port, contentRoot) : - string.Format(CultureInfo.InvariantCulture, "/site:{0} /config:{1} /trace:error /systray:false", "5", DeploymentParameters.ServerConfigLocation); - DeploymentParameters.SiteName = "5"; + string.Format(CultureInfo.InvariantCulture, "/site:{0} /config:{1} /trace:error /systray:false", DeploymentParameters.SiteName, DeploymentParameters.ServerConfigLocation); Logger.LogInformation("Executing command : {iisExpress} {parameters}", iisExpressPath, parameters); @@ -294,7 +295,7 @@ private void PrepareConfig(string contentRoot, int port) .RequiredElement("modules") .GetOrAdd("add", "name", AspNetCoreModuleV2ModuleName); - ConfigureModuleAndBinding(config.Root, contentRoot, port, 5); + ConfigureModuleAndBinding(config.Root, contentRoot, port, siteId: 2); var webConfigPath = Path.Combine(contentRoot, "web.config"); if (!DeploymentParameters.PublishApplicationBeforeDeployment && !File.Exists(webConfigPath)) From 8c6e8e56c8b93e5f24fa682690d663fac85fbaa8 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 22 Jan 2025 11:19:19 -0800 Subject: [PATCH 6/9] using --- src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs index 7b476dd54874..8b3f32f61e96 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployerBase.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Globalization; using System.Xml.Linq; using Microsoft.Extensions.Logging; From 2324b9b3421504d3c766dbccfa7579b12e8cd2ed Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 22 Jan 2025 11:50:19 -0800 Subject: [PATCH 7/9] using --- .../IIS/test/Common.FunctionalTests/MultipleAppTests.cs | 7 ------- .../IIS/IntegrationTesting.IIS/src/Http.SubApp.config | 6 +++--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs index 1893d3623b02..3431f536c3db 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs @@ -1,19 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; using System.Net; using System.Net.Http; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.IIS.FunctionalTests; using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.AspNetCore.InternalTesting; -using Xunit; using System.Globalization; -using System.Diagnostics; -using System.IO; #if !IIS_FUNCTIONALS using Microsoft.AspNetCore.Server.IIS.FunctionalTests; diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/Http.SubApp.config b/src/Servers/IIS/IntegrationTesting.IIS/src/Http.SubApp.config index ffc83b434151..104e90f9fcc6 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/Http.SubApp.config +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/Http.SubApp.config @@ -167,8 +167,8 @@ - - + + @@ -1026,4 +1026,4 @@ - \ No newline at end of file + From bb29868a905c9502ab80fb4374455f2dbafd1b89 Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 23 Jan 2025 14:20:58 -0800 Subject: [PATCH 8/9] just empty path check --- .../IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index a00ab1145d7d..eeb30d71779a 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -85,9 +85,9 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalConfigurationChange( LOG_INFOF(L"ASPNET_CORE_GLOBAL_MODULE::OnGlobalConfigurationChange '%ls'", pwszChangePath); - // Test for an error. - if (nullptr != pwszChangePath && - _wcsicmp(pwszChangePath, L"MACHINE/WEBROOT") > 0 && + if (pwszChangePath != nullptr && pwszChangePath[0] != L'\0' && + _wcsicmp(pwszChangePath, L"MACHINE") != 0 && + _wcsicmp(pwszChangePath, L"MACHINE/WEBROOT") != 0 && // Configuration change recycling behavior can be turned off via setting disallowRotationOnConfigChange=true on the handler settings section. // We need this duplicate setting because the global module is unable to read the app settings disallowRotationOnConfigChange value. m_pApplicationManager && m_pApplicationManager->ShouldRecycleOnConfigChange()) From f7fbf14a9fd07cbc23a2e448d5126559fcc94ef8 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 4 Feb 2025 14:30:26 -0800 Subject: [PATCH 9/9] fixup --- .../AspNetCore/applicationmanager.cpp | 11 ++++++-- .../AspNetCore/globalmodule.cpp | 25 +++++++++++-------- .../IntegrationTesting.IIS/src/IISDeployer.cs | 3 +-- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp index 5f9224e39c76..ae56d4178f34 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp @@ -23,9 +23,16 @@ APPLICATION_MANAGER::GetOrCreateApplicationInfo( ) { // GetOrCreateApplicationInfo is called from proxymodule when a request is received. - // Set this value to indicate that a request has been received so we can disable shutdown logic in OnGlobalApplicationStop - m_hasStarted = true; + PCWSTR pszVariableValue = nullptr; + DWORD cbLength = 0; + // Check for preload or warmup request, part of the application initialization process, see comments in ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop for more info + if (FAILED(pHttpContext.GetServerVariable("PRELOAD_REQUEST", &pszVariableValue, &cbLength)) && + FAILED(pHttpContext.GetServerVariable("WARMUP_REQUEST", &pszVariableValue, &cbLength))) + { + // Set this value to indicate that a request has been received so we can disable shutdown logic in OnGlobalApplicationStop + m_hasStarted = true; + } auto &pApplication = *pHttpContext.GetApplication(); // The configuration path is unique for each application and is used for the diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index eeb30d71779a..e920072f7684 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -35,6 +35,9 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalStopListening( return GL_NOTIFICATION_CONTINUE; } +// We prefer shutting down from OnGlobalStopListening as it is called right before the IIS request handler is disabled, which means it'll start queueing requests +// But if we stopped in OnGlobalApplicationStop then we can start shutting down while the request handler is still active resulting in us returning 503's since we're shutting down. +// We still need to shutdown in specific cases where OnGlobalStopListening isn't called, like IISExpress or if the app never receives a request (app preload). GLOBAL_NOTIFICATION_STATUS ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop( IN IHttpApplicationStopProvider* pProvider @@ -51,17 +54,19 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop( LOG_INFO(L"ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop"); - if (!g_fInShutdown && !m_shutdown.joinable() - && (m_pApplicationManager->IsIISExpress() || !m_pApplicationManager->HasReceivedRequest())) + if (!g_fInShutdown && !m_shutdown.joinable()) { - // Apps with preload + always running that don't receive a request before recycle/shutdown will never call OnGlobalStopListening - // IISExpress can also close without calling OnGlobalStopListening which is where we usually would trigger shutdown - // so we should make sure to shutdown the server in those cases - StartShutdown(); - } - else - { - LOG_INFO(L"Ignoring OnGlobalApplicationStop, OnGlobalStopListening should be called shortly."); + if ((m_pApplicationManager->IsIISExpress() || !m_pApplicationManager->HasReceivedRequest())) + { + // Apps with preload + always running that don't receive a request before recycle/shutdown will never call OnGlobalStopListening + // IISExpress can also close without calling OnGlobalStopListening which is where we usually would trigger shutdown + // so we should make sure to shutdown the server in those cases + StartShutdown(); + } + else + { + LOG_INFO(L"Ignoring OnGlobalApplicationStop, OnGlobalStopListening has been called or should be called shortly."); + } } return GL_NOTIFICATION_CONTINUE; diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs index 3f56b6a7d151..400dc74287e9 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs @@ -307,7 +307,7 @@ private static Site FindSite(ServerManager serverManager, string contentRoot) private void AddTemporaryAppHostConfig(string contentRoot, int port) { - var multiSite = true; + var multiSite = _applicationHostConfig is not null; XDocument config; if (_applicationHostConfig is not null) { @@ -315,7 +315,6 @@ private void AddTemporaryAppHostConfig(string contentRoot, int port) } else { - multiSite = false; _configPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("D")); _applicationHostConfig = Path.Combine(_configPath, "applicationHost.config"); Directory.CreateDirectory(_configPath);