From ac7a6b56d47fa39030d772fc8a5153e685d1e1dc Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 10 Oct 2018 10:00:13 -0700 Subject: [PATCH 1/3] Reenable client disconnect tests (#1485) --- test/IIS.Tests/ClientDisconnectTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/IIS.Tests/ClientDisconnectTests.cs b/test/IIS.Tests/ClientDisconnectTests.cs index 9c8855be47..2731c5665e 100644 --- a/test/IIS.Tests/ClientDisconnectTests.cs +++ b/test/IIS.Tests/ClientDisconnectTests.cs @@ -127,7 +127,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal("The client has disconnected", exception.Message); } - [ConditionalFact(Skip = "See: https://github.com/aspnet/IISIntegration/issues/1075")] + [ConditionalFact] public async Task WriterThrowsCancelledException() { var requestStartedCompletionSource = CreateTaskCompletionSource(); @@ -136,7 +136,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Exception exception = null; var cancellationTokenSource = new CancellationTokenSource(); - var data = new byte[1024]; + var data = new byte[1]; using (var testServer = await TestServer.Create(async ctx => { requestStartedCompletionSource.SetResult(true); @@ -167,7 +167,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.IsType(exception); } } - [ConditionalFact(Skip = "See: https://github.com/aspnet/IISIntegration/issues/1075")] + + [ConditionalFact] public async Task ReaderThrowsCancelledException() { var requestStartedCompletionSource = CreateTaskCompletionSource(); From ab124fc344608ddd588738116d68a367c4b64a13 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 10 Oct 2018 12:41:11 -0700 Subject: [PATCH 2/3] Fix applicationInitialization tests and disconnect handler (#1484) --- .../AspNetCore/proxymodule.cpp | 13 +++-- .../Core/IISHttpContext.cs | 18 ++++++- .../NativeMethods.cs | 6 ++- test/IIS.FunctionalTests/ServicesTests.cs | 48 +++++++++++++------ test/WebSites/InProcessWebSite/Program.cs | 5 +- test/WebSites/OutOfProcessWebSite/Program.cs | 27 ++++++++--- .../shared/SharedStartup/Startup.shared.cs | 13 ++--- 7 files changed, 94 insertions(+), 36 deletions(-) diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp index b1fa5568a8..7467c81b21 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp @@ -159,9 +159,16 @@ REQUEST_NOTIFICATION_STATUS ASPNET_CORE_PROXY_MODULE::HandleNotificationStatus(R void ASPNET_CORE_PROXY_MODULE::SetupDisconnectHandler(IHttpContext * pHttpContext) { - auto moduleContainer = pHttpContext - ->GetConnection() - ->GetModuleContextContainer(); + auto connection = pHttpContext + ->GetConnection(); + + // connection might be null in when applicationInitialization is running + if (connection == nullptr) + { + return; + } + + auto moduleContainer = connection->GetModuleContextContainer(); #pragma warning( push ) #pragma warning ( disable : 26466 ) // Disable "Don't use static_cast downcasts". We build without RTTI support so dynamic_cast is not available diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs index 45848183ba..b824c67c6c 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpContext.cs @@ -120,7 +120,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core KnownMethod = VerbId; StatusCode = 200; - var originalPath = RequestUriBuilder.DecodeAndUnescapePath(GetRawUrlInBytes()); + var originalPath = GetOriginalPath(); if (KnownMethod == HttpApiTypes.HTTP_VERB.HttpVerbOPTIONS && string.Equals(RawTarget, "*", StringComparison.Ordinal)) { @@ -172,6 +172,22 @@ namespace Microsoft.AspNetCore.Server.IIS.Core _bodyOutput = new OutputProducer(pipe); } + private string GetOriginalPath() + { + // applicationInitialization request might have trailing \0 character included in the length + // check and skip it + var rawUrlInBytes = GetRawUrlInBytes(); + if (rawUrlInBytes.Length > 0 && rawUrlInBytes[rawUrlInBytes.Length - 1] == 0) + { + var newRawUrlInBytes = new byte[rawUrlInBytes.Length - 1]; + Array.Copy(rawUrlInBytes, newRawUrlInBytes, newRawUrlInBytes.Length); + rawUrlInBytes = newRawUrlInBytes; + } + + var originalPath = RequestUriBuilder.DecodeAndUnescapePath(rawUrlInBytes); + return originalPath; + } + public int StatusCode { get { return _statusCode; } diff --git a/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs b/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs index 51f4ad448f..4d24b3dd77 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs @@ -13,6 +13,7 @@ namespace Microsoft.AspNetCore.Server.IIS internal const int HR_OK = 0; internal const int ERROR_NOT_FOUND = unchecked((int)0x80070490); internal const int ERROR_OPERATION_ABORTED = unchecked((int)0x800703E3); + internal const int ERROR_INVALID_PARAMETER = unchecked((int)0x80070057); internal const int COR_E_IO = unchecked((int)0x80131620); private const string KERNEL32 = "kernel32.dll"; @@ -262,9 +263,10 @@ namespace Microsoft.AspNetCore.Server.IIS public static bool HttpTryCancelIO(IntPtr pInProcessHandler) { var hr = http_cancel_io(pInProcessHandler); - // Async operation finished + // ERROR_NOT_FOUND is expected if async operation finished // https://msdn.microsoft.com/en-us/library/windows/desktop/aa363792(v=vs.85).aspx - if (hr == ERROR_NOT_FOUND) + // ERROR_INVALID_PARAMETER is expected for "fake" requests like applicationInitialization ones + if (hr == ERROR_NOT_FOUND || hr == ERROR_INVALID_PARAMETER) { return false; } diff --git a/test/IIS.FunctionalTests/ServicesTests.cs b/test/IIS.FunctionalTests/ServicesTests.cs index 0fe723eae9..b91fc8aa20 100644 --- a/test/IIS.FunctionalTests/ServicesTests.cs +++ b/test/IIS.FunctionalTests/ServicesTests.cs @@ -1,7 +1,10 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.IO; using System.ServiceProcess; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; using Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests; @@ -27,24 +30,44 @@ namespace IIS.FunctionalTests [RequiresIIS(IISCapability.ApplicationInitialization)] [InlineData(HostingModel.InProcess)] [InlineData(HostingModel.OutOfProcess)] - public async Task ApplicationInitializationInitializedInProc(HostingModel hostingModel) + public async Task ApplicationPreloadStartsApp(HostingModel hostingModel) { - var baseDeploymentParameters = _fixture.GetBaseDeploymentParameters(hostingModel); - EnableAppInitialization(baseDeploymentParameters); + var baseDeploymentParameters = _fixture.GetBaseDeploymentParameters(hostingModel, publish: true); + baseDeploymentParameters.TransformArguments((args, contentRoot)=> $"{args} CreateFile \"{Path.Combine(contentRoot, "Started.txt")}\""); + EnablePreload(baseDeploymentParameters); var result = await DeployAsync(baseDeploymentParameters); - // Allow IIS a bit of time to complete starting before we start checking - await Task.Delay(100); - // There is always a race between which Init request arrives first - // retry couple times to see if we ever get the one comming from ApplicationInitialization module - await result.HttpClient.RetryRequestAsync("/ApplicationInitialization", async message => await message.Content.ReadAsStringAsync() == "True"); - + await Helpers.Retry(async () => await File.ReadAllTextAsync(Path.Combine(result.ContentRoot, "Started.txt")), 10, 200); StopServer(); EventLogHelpers.VerifyEventLogEvent(result, EventLogHelpers.Started(result)); } - private static void EnableAppInitialization(IISDeploymentParameters baseDeploymentParameters) + [ConditionalTheory] + [RequiresIIS(IISCapability.ApplicationInitialization)] + [InlineData(HostingModel.InProcess)] + [InlineData(HostingModel.OutOfProcess)] + public async Task ApplicationInitializationPageIsRequested(HostingModel hostingModel) + { + var baseDeploymentParameters = _fixture.GetBaseDeploymentParameters(hostingModel, publish: true); + EnablePreload(baseDeploymentParameters); + + baseDeploymentParameters.ServerConfigActionList.Add( + (config, _) => { + config + .RequiredElement("system.webServer") + .GetOrAdd("applicationInitialization") + .GetOrAdd("add", "initializationPage", "/CreateFile"); + }); + + var result = await DeployAsync(baseDeploymentParameters); + + await Helpers.Retry(async () => await File.ReadAllTextAsync(Path.Combine(result.ContentRoot, "Started.txt")), 10, 200); + StopServer(); + EventLogHelpers.VerifyEventLogEvent(result, EventLogHelpers.Started(result)); + } + + private static void EnablePreload(IISDeploymentParameters baseDeploymentParameters) { baseDeploymentParameters.ServerConfigActionList.Add( (config, _) => { @@ -60,11 +83,6 @@ namespace IIS.FunctionalTests .RequiredElement("site") .RequiredElement("application") .SetAttributeValue("preloadEnabled", true); - - config - .RequiredElement("system.webServer") - .GetOrAdd("applicationInitialization") - .GetOrAdd("add", "initializationPage", "/ApplicationInitialization?IISInit=true"); }); baseDeploymentParameters.EnableModule("ApplicationInitializationModule", "%IIS_BIN%\\warmup.dll"); diff --git a/test/WebSites/InProcessWebSite/Program.cs b/test/WebSites/InProcessWebSite/Program.cs index 43e9a563b2..c40448ee91 100644 --- a/test/WebSites/InProcessWebSite/Program.cs +++ b/test/WebSites/InProcessWebSite/Program.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO; using System.Linq; using System.Text; using System.Threading; @@ -21,7 +22,9 @@ namespace TestSite var mode = args.FirstOrDefault(); switch (mode) { - // Semicolons are appended to env variables; removing them. + case "CreateFile": + File.WriteAllText(args[1], ""); + return StartServer(); case "CheckLargeStdOutWrites": Console.WriteLine(new string('a', 30000)); break; diff --git a/test/WebSites/OutOfProcessWebSite/Program.cs b/test/WebSites/OutOfProcessWebSite/Program.cs index e5ce65cba1..90895237d2 100644 --- a/test/WebSites/OutOfProcessWebSite/Program.cs +++ b/test/WebSites/OutOfProcessWebSite/Program.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.IO; +using System.Linq; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Logging; @@ -9,14 +10,27 @@ namespace TestSite { public static class Program { - public static void Main(string[] args) + public static int Main(string[] args) + { + var mode = args.FirstOrDefault(); + switch (mode) + { + case "CreateFile": + File.WriteAllText(args[1], ""); + return StartServer(); + } + + return StartServer(); + } + + private static int StartServer() { var host = new WebHostBuilder() - .ConfigureLogging((_, factory) => - { - factory.AddConsole(); - factory.AddFilter("Console", level => level >= LogLevel.Information); - }) + .ConfigureLogging( + (_, factory) => { + factory.AddConsole(); + factory.AddFilter("Console", level => level >= LogLevel.Information); + }) .UseContentRoot(Directory.GetCurrentDirectory()) .UseIISIntegration() .UseStartup() @@ -24,6 +38,7 @@ namespace TestSite .Build(); host.Run(); + return 0; } } } diff --git a/test/WebSites/shared/SharedStartup/Startup.shared.cs b/test/WebSites/shared/SharedStartup/Startup.shared.cs index 375902faea..d119932255 100644 --- a/test/WebSites/shared/SharedStartup/Startup.shared.cs +++ b/test/WebSites/shared/SharedStartup/Startup.shared.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -72,15 +73,11 @@ namespace TestSite await context.Response.WriteAsync(_waitingRequestCount.ToString()); } - private static bool _applicationInitializationCalled; - - public async Task ApplicationInitialization(HttpContext context) + public Task CreateFile(HttpContext context) { - if (context.Request.Query["IISInit"] == "true") - { - _applicationInitializationCalled = true; - } - await context.Response.WriteAsync(_applicationInitializationCalled.ToString()); + var hostingEnv = context.RequestServices.GetService(); + File.WriteAllText(System.IO.Path.Combine(hostingEnv.ContentRootPath, "Started.txt"), ""); + return Task.CompletedTask; } public Task OverrideServer(HttpContext context) From bfa583a9aa2acae53c6f7b3d903f689710a6e0c3 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 10 Oct 2018 14:34:55 -0700 Subject: [PATCH 3/3] Guard OnAsyncCompletion from completing request before OnExecuteRequestHandler exits (#1489) --- src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp | 8 ++++++++ src/AspNetCoreModuleV2/AspNetCore/proxymodule.h | 1 + 2 files changed, 9 insertions(+) diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp index 7467c81b21..38eaf08c31 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp @@ -7,6 +7,7 @@ #include "applicationinfo.h" #include "exceptions.h" #include "DisconnectHandler.h" +#include "SRWExclusiveLock.h" extern BOOL g_fInShutdown; @@ -67,6 +68,7 @@ ASPNET_CORE_PROXY_MODULE::ASPNET_CORE_PROXY_MODULE(HTTP_MODULE_ID moduleId, std: m_moduleId(moduleId), m_pDisconnectHandler(nullptr) { + InitializeSRWLock(&m_requestLock); } ASPNET_CORE_PROXY_MODULE::~ASPNET_CORE_PROXY_MODULE() @@ -84,6 +86,9 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( HRESULT hr = S_OK; REQUEST_NOTIFICATION_STATUS retVal = RQ_NOTIFICATION_CONTINUE; + // We don't want OnAsyncCompletion to complete request before OnExecuteRequestHandler exits + auto lock = SRWExclusiveLock(m_requestLock); + try { if (g_fInShutdown) @@ -134,6 +139,9 @@ ASPNET_CORE_PROXY_MODULE::OnAsyncCompletion( IHttpCompletionInfo * pCompletionInfo ) { + // We don't want OnAsyncCompletion to complete request before OnExecuteRequestHandler exits + auto lock = SRWExclusiveLock(m_requestLock); + try { return HandleNotificationStatus(m_pHandler->OnAsyncCompletion( diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h index c5b290a36e..8f5b680e36 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h @@ -60,6 +60,7 @@ class ASPNET_CORE_PROXY_MODULE : NonCopyable, public CHttpModule std::unique_ptr m_pHandler; HTTP_MODULE_ID m_moduleId; DisconnectHandler * m_pDisconnectHandler; + SRWLOCK m_requestLock {}; }; class ASPNET_CORE_PROXY_MODULE_FACTORY : NonCopyable, public IHttpModuleFactory