Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IIS recycle regressions #59998

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ APPLICATION_MANAGER::GetOrCreateApplicationInfo(
_Out_ std::shared_ptr<APPLICATION_INFO>& 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
Expand Down Expand Up @@ -123,7 +127,6 @@ APPLICATION_MANAGER::RecycleApplicationFromManager(
else
{
++itr;

}
}

Expand All @@ -149,8 +152,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -57,11 +58,22 @@ class APPLICATION_MANAGER
return m_handlerResolver.GetShutdownDelay() == std::chrono::milliseconds::zero();
}

bool IsIISExpress() const
{
return m_pHttpServer.IsCommandLineLaunch();
}

bool HasReceivedRequest() const
{
return m_hasStarted;
}

private:

std::unordered_map<std::wstring, std::shared_ptr<APPLICATION_INFO>> m_pApplicationInfoHash;
SRWLOCK m_srwLock {};
BOOL m_fDebugInitialize;
IHttpServer &m_pHttpServer;
HandlerResolver m_handlerResolver;
bool m_hasStarted;
};
22 changes: 14 additions & 8 deletions src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,18 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalApplicationStop(

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
// so we should make sure to shutdown the server in those cases
StartShutdown();
}
else
{
LOG_INFO(L"Ignoring OnGlobalApplicationStop, OnGlobalStopListening should be called shortly.");
}

return GL_NOTIFICATION_CONTINUE;
}
Expand All @@ -82,15 +87,16 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalConfigurationChange(

// Test for an error.
BrennanConroy marked this conversation as resolved.
Show resolved Hide resolved
if (nullptr != pwszChangePath &&
_wcsicmp(pwszChangePath, L"MACHINE") != 0 &&
_wcsicmp(pwszChangePath, L"MACHINE/WEBROOT") != 0)
{
_wcsicmp(pwszChangePath, L"MACHINE/WEBROOT") > 0 &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a prefix check shouldn't it be e.g.

_wcsnicmp(pwszChangePath, L"MACHINE/WEBROOT", wcslen(L"MACHINE/WEBROOT")) == 0

Why the >0 check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm curious why the previous code looked like it did if it was in fact intending to do a prefix check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm curious why the previous code looked like it did if it was in fact intending to do a prefix check.

That's fair, it is written oddly. I haven't seen any examples of a non "MACHINE/WEBROOT/..." value which is why I assumed it was a prefix check.

Your proposed code would be an exact check not a prefix check. The > 0 I'm using checks that it matches and contains more characters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the wcsicmp you're using will return a positive value for anything that's lexicographically greater, so even MACHINE/ZZZ will pass. It doesn't actually do a prefix check.

The wcsnicmp approach I suggested would be a prefix check since it also specifies the number of characters to compare.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to just check for empty path

// 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 for '%ls'", pwszChangePath);
}

// Return processing to the pipeline.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}";
}
}

Expand All @@ -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 '.*?'";
}
}

Expand All @@ -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}";
}
}

Expand All @@ -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.";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
}

Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ internal IISTestSiteFixture(Action<IISDeploymentParameters> 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"))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix local websocket tests.

{
DeploymentParameters.EnableModule("WebSocketModule", "%IIS_BIN%/iiswsock.dll");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
129 changes: 129 additions & 0 deletions src/Servers/IIS/IIS/test/Common.FunctionalTests/MultipleAppTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net;
using System.Net.Http;
using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities;
using Microsoft.AspNetCore.Server.IntegrationTesting;
using Microsoft.AspNetCore.InternalTesting;
using System.Globalization;

#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 StartupManyAppsSuccessful()
{
const int numApps = 10;

using (var deployers = new DisposableList<ApplicationDeployer>())
{
var deploymentResults = new List<DeploymentResult>();

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<Task<HttpResponseMessage>>();
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 RestartAppShouldNotAffectOtherApps()
{
const int numApps = 10;

using (var deployers = new DisposableList<ApplicationDeployer>())
{
var deploymentResults = new List<DeploymentResult>();

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<Task<HttpResponseMessage>>();
foreach (var deploymentResult in deploymentResults)
{
requestTasks.Add(deploymentResult.HttpClient.RetryRequestAsync("/ProcessId", r => r.IsSuccessStatusCode));
}

List<int> 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));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}
""";

Expand Down
Loading
Loading