diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp index 28a5dde1c8..317a8a5d4b 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp @@ -155,8 +155,8 @@ APPLICATION_INFO::TryCreateApplication(IHttpContext& pHttpContext, const ShimOpt } LOG_LAST_ERROR_IF(WaitForSingleObject(eventHandle, INFINITE) != WAIT_OBJECT_0); } - } + RETURN_IF_FAILED(m_handlerResolver.GetApplicationFactory(*pHttpContext.GetApplication(), m_pApplicationFactory, options)); LOG_INFO(L"Creating handler application"); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/application.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/application.h index 36ca2f073d..ae30844330 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/application.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/application.h @@ -9,6 +9,7 @@ #include "SRWExclusiveLock.h" #include "SRWSharedLock.h" #include "exceptions.h" +#include "HandleWrapper.h" class APPLICATION : public IAPPLICATION { @@ -24,8 +25,10 @@ public: { *pRequestHandler = nullptr; - SRWSharedLock stopLock(m_stateLock); + SRWSharedLock stopLock(m_stopLock); + // If we have acquired the stopLock, we don't need to acquire the data lock + // as m_fStoppedCalled is only set by Stop. if (m_fStopCalled) { return S_FALSE; @@ -49,22 +52,25 @@ public: m_applicationConfigPath(pHttpApplication.GetAppConfigPath()), m_applicationId(pHttpApplication.GetApplicationId()) { - InitializeSRWLock(&m_stateLock); + InitializeSRWLock(&m_stopLock); + InitializeSRWLock(&m_dataLock); m_applicationVirtualPath = ToVirtualPath(m_applicationConfigPath); } - VOID Stop(bool fServerInitiated) override { - SRWExclusiveLock stopLock(m_stateLock); + SRWExclusiveLock stopLock(m_stopLock); - if (m_fStopCalled) { - return; - } + SRWExclusiveLock dataLock(m_dataLock); + if (m_fStopCalled) + { + return; + } - m_fStopCalled = true; + m_fStopCalled = true; + } StopInternal(fServerInitiated); } @@ -120,7 +126,8 @@ public: } protected: - SRWLOCK m_stateLock {}; + SRWLOCK m_stopLock{}; + SRWLOCK m_dataLock {}; bool m_fStopCalled; private: diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp index af1c3e2594..8719348843 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp @@ -20,7 +20,7 @@ IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( IHttpServer& pHttpServer, IHttpApplication& pApplication, std::unique_ptr pConfig, - APPLICATION_PARAMETER *pParameters, + APPLICATION_PARAMETER* pParameters, DWORD nParameters) : InProcessApplicationBase(pHttpServer, pApplication), m_Initialized(false), @@ -55,6 +55,7 @@ IN_PROCESS_APPLICATION::StopInternal(bool fServerInitiated) VOID IN_PROCESS_APPLICATION::StopClr() { + // This has the state lock around it. LOG_INFO(L"Stopping CLR"); if (!m_blockManagedCallbacks) @@ -69,11 +70,13 @@ IN_PROCESS_APPLICATION::StopClr() shutdownHandler(m_ShutdownHandlerContext); } + SRWSharedLock dataLock(m_dataLock); + auto requestCount = m_requestCount.load(); + if (requestCount == 0) { - LOG_INFO(L"Drained all requests, notifying managed."); - m_RequestsDrainedHandler(m_ShutdownHandlerContext); + CallRequestsDrained(); } } @@ -141,12 +144,6 @@ IN_PROCESS_APPLICATION::LoadManagedApplication() FALSE, // not set nullptr)); // name - THROW_LAST_ERROR_IF_NULL(m_pRequestDrainEvent = CreateEvent( - nullptr, // default security attributes - TRUE, // manual reset event - FALSE, // not set - nullptr)); // name - LOG_INFO(L"Waiting for initialization"); m_workerThread = std::thread([](std::unique_ptr application) @@ -532,8 +529,13 @@ IN_PROCESS_APPLICATION::CreateHandler( { try { + SRWSharedLock dataLock(m_dataLock); + DBG_ASSERT(!m_fStopCalled); m_requestCount++; + + LOG_TRACEF(L"Adding request. Total Request Count %d", m_requestCount.load()); + *pRequestHandler = new IN_PROCESS_HANDLER(::ReferenceApplication(this), pHttpContext, m_RequestHandler, m_RequestHandlerContext, m_DisconnectHandler, m_AsyncCompletionHandler); } CATCH_RETURN(); @@ -544,11 +546,24 @@ IN_PROCESS_APPLICATION::CreateHandler( void IN_PROCESS_APPLICATION::HandleRequestCompletion() { - SRWSharedLock lock(m_stateLock); - auto requestCount = m_requestCount--; - if (m_fStopCalled && requestCount == 0) + SRWSharedLock dataLock(m_dataLock); + + auto requestCount = --m_requestCount; + + LOG_TRACEF(L"Removing Request %d", requestCount); + + if (m_fStopCalled && requestCount == 0 && !m_blockManagedCallbacks) + { + CallRequestsDrained(); + } +} + +void IN_PROCESS_APPLICATION::CallRequestsDrained() +{ + if (m_RequestsDrainedHandler != nullptr) { LOG_INFO(L"Drained all requests, notifying managed."); m_RequestsDrainedHandler(m_ShutdownHandlerContext); + m_RequestsDrainedHandler = nullptr; } } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h index e992802806..e6f4c826bb 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h @@ -150,8 +150,6 @@ private: // The event that gets triggered when worker thread should exit HandleWrapper m_pShutdownEvent; - HandleWrapper m_pRequestDrainEvent; - // The request handler callback from managed code PFN_REQUEST_HANDLER m_RequestHandler; VOID* m_RequestHandlerContext; @@ -169,6 +167,7 @@ private: std::atomic_bool m_blockManagedCallbacks; bool m_Initialized; bool m_waitForShutdown; + std::atomic m_requestCount; std::unique_ptr m_pConfig; @@ -188,6 +187,9 @@ private: void StopClr(); + void + CallRequestsDrained(); + static void ClrThreadEntryPoint(const std::shared_ptr &context); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/outprocessapplication.cpp b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/outprocessapplication.cpp index 0a171e5f8d..7bd0a5fa83 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/outprocessapplication.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/outprocessapplication.cpp @@ -18,7 +18,7 @@ OUT_OF_PROCESS_APPLICATION::OUT_OF_PROCESS_APPLICATION( OUT_OF_PROCESS_APPLICATION::~OUT_OF_PROCESS_APPLICATION() { - SRWExclusiveLock lock(m_stateLock); + SRWExclusiveLock lock(m_stopLock); if (m_pProcessManager != NULL) { m_pProcessManager->Shutdown(); diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/AppOfflineTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/AppOfflineTests.cs index 73f7d59444..ab5e39748e 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/AppOfflineTests.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/AppOfflineTests.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.Logging; using Xunit; using Microsoft.AspNetCore.Server.IntegrationTesting.IIS; +using System.Collections.Generic; namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests { @@ -98,8 +99,8 @@ namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests await deploymentResult.AssertRecycledAsync(() => AssertAppOffline(deploymentResult)); } - [ConditionalFact(Skip = "https://github.com/aspnet/AspNetCore/issues/6555")] - [RequiresIIS(IISCapability.ShutdownToken)] + [ConditionalFact] + [RequiresNewHandler] public async Task AppOfflineDroppedWhileSiteStarting_SiteShutsDown_InProcess() { // This test often hits a race between debug logging and stdout redirection closing the handle @@ -131,19 +132,100 @@ namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests // if AssertAppOffline succeeded ANCM have picked up app_offline before starting the app // try again RemoveAppOffline(deploymentResult.ContentRoot); + + if (deploymentResult.DeploymentParameters.ServerType == ServerType.IIS) + { + deploymentResult.AssertWorkerProcessStop(); + return; + } } catch { + // For IISExpress, we need to catch the exception because IISExpress will not restart a process if it crashed. + // RemoveAppOffline will fail due to a bad request exception as the server is down. + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Drained all requests, notifying managed.")); deploymentResult.AssertWorkerProcessStop(); return; } } Assert.True(false); - } } + [ConditionalFact] + public async Task GracefulShutdownWorksWithMultipleRequestsInFlight_InProcess() + { + // The goal of this test is to have multiple requests currently in progress + // and for app offline to be dropped. We expect that all requests are eventually drained + // and graceful shutdown occurs. + var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.InProcessTestSite); + deploymentParameters.TransformArguments((a, _) => $"{a} IncreaseShutdownLimit"); + + var deploymentResult = await DeployAsync(deploymentParameters); + + var result = await deploymentResult.HttpClient.GetAsync("/HelloWorld"); + + // Send two requests that will hang until data is sent from the client. + var connectionList = new List(); + + for (var i = 0; i < 2; i++) + { + var connection = new TestConnection(deploymentResult.HttpClient.BaseAddress.Port); + await connection.Send( + "POST /ReadAndCountRequestBody HTTP/1.1", + "Content-Length: 1", + "Host: localhost", + "Connection: close", + "", + ""); + + await connection.Receive( + "HTTP/1.1 200 OK", ""); + await connection.ReceiveHeaders(); + await connection.Receive("1", $"{i + 1}"); + connectionList.Add(connection); + } + + // Send a request that will end once app lifetime is triggered (ApplicationStopping cts). + var statusConnection = new TestConnection(deploymentResult.HttpClient.BaseAddress.Port); + + await statusConnection.Send( + "GET /WaitForAppToStartShuttingDown HTTP/1.1", + "Host: localhost", + "Connection: close", + "", + ""); + + await statusConnection.Receive("HTTP/1.1 200 OK", + ""); + + await statusConnection.ReceiveHeaders(); + + // Receiving some data means we are currently waiting for IHostApplicationLifetime. + await statusConnection.Receive("5", + "test1", + ""); + + AddAppOffline(deploymentResult.ContentRoot); + + // Receive the rest of all open connections. + await statusConnection.Receive("5", "test2", ""); + + for (var i = 0; i < 2; i++) + { + await connectionList[i].Send("a", ""); + await connectionList[i].Receive("", "4", "done"); + connectionList[i].Dispose(); + } + + deploymentResult.AssertWorkerProcessStop(); + + // Shutdown should be graceful here! + EventLogHelpers.VerifyEventLogEvent(deploymentResult, + EventLogHelpers.InProcessShutdown()); + } + [ConditionalFact] public async Task AppOfflineDroppedWhileSiteRunning_SiteShutsDown_InProcess() { @@ -282,6 +364,5 @@ namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests File.Delete(file); } } - } } diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs index 0f0d556cdf..0bd9c196db 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs @@ -172,6 +172,11 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } } + public static string InProcessShutdown() + { + return "Application 'MACHINE/WEBROOT/APPHOST/.*?' has shutdown."; + } + public static string InProcessFailedToStop(IISDeploymentResult deploymentResult, string reason) { return "Failed to gracefully shutdown application 'MACHINE/WEBROOT/APPHOST/.*?'."; diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/IISCapability.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/IISCapability.cs index 0a080bb702..612b61d442 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/IISCapability.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Utilities/IISCapability.cs @@ -12,11 +12,10 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Websockets = 1, WindowsAuthentication = 2, PoolEnvironmentVariables = 4, - ShutdownToken = 8, - DynamicCompression = 16, - ApplicationInitialization = 32, - TracingModule = 64, - FailedRequestTracingModule = 128, - BasicAuthentication = 256 + DynamicCompression = 8, + ApplicationInitialization = 16, + TracingModule = 32, + FailedRequestTracingModule = 64, + BasicAuthentication = 128 } } diff --git a/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/RequiresIISAttribute.cs b/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/RequiresIISAttribute.cs index 8653c2b9ee..4f00a17531 100644 --- a/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/RequiresIISAttribute.cs +++ b/src/Servers/IIS/IIS/test/IIS.Shared.FunctionalTests/RequiresIISAttribute.cs @@ -124,12 +124,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } } - if (capabilities.HasFlag(IISCapability.ShutdownToken)) - { - IsMet = false; - SkipReason += "https://github.com/aspnet/IISIntegration/issues/1074"; - } - foreach (var module in Modules) { if (capabilities.HasFlag(module.Capability)) diff --git a/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Program.cs b/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Program.cs index c40448ee91..59e35c3a28 100644 --- a/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Program.cs +++ b/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Program.cs @@ -63,6 +63,18 @@ namespace TestSite Thread.Sleep(Timeout.Infinite); } + break; + case "IncreaseShutdownLimit": + { + var host = new WebHostBuilder() + .UseIIS() + .UseShutdownTimeout(TimeSpan.FromSeconds(120)) + .UseStartup() + .Build(); + + host.Run(); + } + break; case "CheckConsoleFunctions": // Call a bunch of console functions and make sure none return invalid handle. diff --git a/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs b/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs index f3b8d1e100..0e925106bf 100644 --- a/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs +++ b/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs @@ -284,6 +284,28 @@ namespace TestSite { result = await ctx.Request.Body.ReadAsync(readBuffer, 0, 1); } + + } + + private int _requestsInFlight = 0; + private async Task ReadAndCountRequestBody(HttpContext ctx) + { + Interlocked.Increment(ref _requestsInFlight); + await ctx.Response.WriteAsync(_requestsInFlight.ToString()); + + var readBuffer = new byte[1]; + await ctx.Request.Body.ReadAsync(readBuffer, 0, 1); + + await ctx.Response.WriteAsync("done"); + Interlocked.Decrement(ref _requestsInFlight); + } + + private async Task WaitForAppToStartShuttingDown(HttpContext ctx) + { + await ctx.Response.WriteAsync("test1"); + var lifetime = ctx.RequestServices.GetService(); + lifetime.ApplicationStopping.WaitHandle.WaitOne(); + await ctx.Response.WriteAsync("test2"); } private async Task ReadFullBody(HttpContext ctx) diff --git a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs index cd05e83de8..c98b6b83ed 100644 --- a/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs +++ b/src/Servers/IIS/IntegrationTesting.IIS/src/IISDeployer.cs @@ -24,12 +24,13 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS private const string DetailedErrorsEnvironmentVariable = "ASPNETCORE_DETAILEDERRORS"; private static readonly TimeSpan _timeout = TimeSpan.FromSeconds(60); - private static readonly TimeSpan _retryDelay = TimeSpan.FromMilliseconds(200); + private static readonly TimeSpan _retryDelay = TimeSpan.FromMilliseconds(100); private CancellationTokenSource _hostShutdownToken = new CancellationTokenSource(); private string _configPath; private string _debugLogFile; + private bool _disposed; public Process HostProcess { get; set; } @@ -45,6 +46,12 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS public override void Dispose() { + if (_disposed) + { + return; + } + + _disposed = true; Dispose(gracefulShutdown: false); } @@ -420,6 +427,7 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS List exceptions = null; var sw = Stopwatch.StartNew(); int retryCount = 0; + var delay = _retryDelay; while (sw.Elapsed < _timeout) { @@ -443,7 +451,8 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS } retryCount++; - Thread.Sleep(_retryDelay); + Thread.Sleep(delay); + delay *= 1.5; } throw new AggregateException($"Operation did not succeed after {retryCount} retries", exceptions.ToArray());