From a741af1e77a6b47cecc22c11fa25156037d45666 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 30 Aug 2018 09:07:02 -0700 Subject: [PATCH 01/18] Add header to sample (#1353) --- samples/IISSample/Startup.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/samples/IISSample/Startup.cs b/samples/IISSample/Startup.cs index 138f993d9e..f99b6ad729 100644 --- a/samples/IISSample/Startup.cs +++ b/samples/IISSample/Startup.cs @@ -1,3 +1,6 @@ +// 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.Linq; using Microsoft.AspNetCore.Authentication; From 922298bc902e4deccd766c0e3bed84b3b3e40b74 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 30 Aug 2018 12:00:13 -0700 Subject: [PATCH 02/18] Add rc file as an include for ANCMV1 (#1284) (#1344) --- src/AspNetCoreModuleV1/AspNetCore/aspnetcoremodule.rc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/AspNetCoreModuleV1/AspNetCore/aspnetcoremodule.rc b/src/AspNetCoreModuleV1/AspNetCore/aspnetcoremodule.rc index 8e06cd04f5..d37eb29238 100644 --- a/src/AspNetCoreModuleV1/AspNetCore/aspnetcoremodule.rc +++ b/src/AspNetCoreModuleV1/AspNetCore/aspnetcoremodule.rc @@ -3,6 +3,7 @@ #include #include "version.h" #include "resource.h" +#include "aspnetcore_msg.rc" ///////////////////////////////////////////////////////////////////////////// // English (United States) resources From e7f3c7c62ecaa5ed7d9519fcf5b017e384e47992 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 30 Aug 2018 12:18:30 -0700 Subject: [PATCH 03/18] Remove bad rc files (#1355) --- src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.rc | 2 -- src/AspNetCoreModuleV2/RequestHandlerLib/aspnetcore_msg.rc | 2 -- 2 files changed, 4 deletions(-) delete mode 100644 src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.rc delete mode 100644 src/AspNetCoreModuleV2/RequestHandlerLib/aspnetcore_msg.rc diff --git a/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.rc b/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.rc deleted file mode 100644 index 0abcb0fa2c..0000000000 --- a/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.rc +++ /dev/null @@ -1,2 +0,0 @@ -LANGUAGE 0x9,0x1 -1 11 "MSG00001.bin" diff --git a/src/AspNetCoreModuleV2/RequestHandlerLib/aspnetcore_msg.rc b/src/AspNetCoreModuleV2/RequestHandlerLib/aspnetcore_msg.rc deleted file mode 100644 index 0abcb0fa2c..0000000000 --- a/src/AspNetCoreModuleV2/RequestHandlerLib/aspnetcore_msg.rc +++ /dev/null @@ -1,2 +0,0 @@ -LANGUAGE 0x9,0x1 -1 11 "MSG00001.bin" From 729a98adfc03442b27b3f7953704e7022a9e1829 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 30 Aug 2018 12:20:53 -0700 Subject: [PATCH 04/18] Call AllocConsole (#1351) --- .../CommonLib/FileOutputManager.cpp | 23 ++++++++++++++- .../CommonLib/FileOutputManager.h | 1 + .../CommonLib/LoggingHelpers.cpp | 7 ++++- .../CommonLib/PipeOutputManager.cpp | 23 ++++++++++++++- .../CommonLib/PipeOutputManager.h | 1 + .../Inprocess/StartupExceptionTests.cs | 29 +++++++++++++------ .../StartupExceptionWebSite/Program.cs | 10 +++++++ 7 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp index 15bc54cbe6..66292196a6 100644 --- a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp @@ -20,7 +20,8 @@ FileOutputManager::FileOutputManager(bool fEnableNativeLogging) : m_disposed(false), stdoutWrapper(nullptr), stderrWrapper(nullptr), - m_fEnableNativeRedirection(fEnableNativeLogging) + m_fEnableNativeRedirection(fEnableNativeLogging), + m_fCreatedConsole(false) { InitializeSRWLock(&m_srwLock); } @@ -49,6 +50,21 @@ FileOutputManager::Start() STRU struPath; FILETIME processCreationTime; FILETIME dummyFileTime; + + // To make Console.* functions work, allocate a console + // in the current process. + if (!AllocConsole()) + { + // ERROR_ACCESS_DENIED means there is a console already present. + if (GetLastError() != ERROR_ACCESS_DENIED) + { + RETURN_LAST_ERROR(); + } + } + else + { + m_fCreatedConsole = true; + } // Concatenate the log file name and application path RETURN_IF_FAILED(FILE_UTILITY::ConvertPathToFullPath( @@ -129,6 +145,11 @@ FileOutputManager::Stop() m_disposed = true; + if (m_fCreatedConsole) + { + RETURN_LAST_ERROR_IF(!FreeConsole()); + } + if (m_hLogFileHandle == INVALID_HANDLE_VALUE) { return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); diff --git a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h index 94f6e19176..7cb3e452fa 100644 --- a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h +++ b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h @@ -35,6 +35,7 @@ private: STRA m_straFileContent; BOOL m_disposed; BOOL m_fEnableNativeRedirection; + BOOL m_fCreatedConsole; SRWLOCK m_srwLock{}; std::unique_ptr stdoutWrapper; std::unique_ptr stderrWrapper; diff --git a/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp b/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp index 69de017e7f..3d1aacf46d 100644 --- a/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp @@ -27,13 +27,18 @@ LoggingHelpers::CreateLoggingProvider( try { + // Check if there is an existing active console window before redirecting + // Window == IISExpress with active console window, don't redirect to a pipe + // if true. + CONSOLE_SCREEN_BUFFER_INFO dummy; + if (fIsLoggingEnabled) { auto manager = std::make_unique(fEnableNativeLogging); hr = manager->Initialize(pwzStdOutFileName, pwzApplicationPath); outputManager = std::move(manager); } - else if (!GetConsoleWindow()) + else if (!GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &dummy)) { outputManager = std::make_unique(fEnableNativeLogging); } diff --git a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp index 8ddfcb437f..20aafcc17e 100644 --- a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp @@ -24,7 +24,8 @@ PipeOutputManager::PipeOutputManager(bool fEnableNativeLogging) : m_disposed(FALSE), m_fEnableNativeRedirection(fEnableNativeLogging), stdoutWrapper(nullptr), - stderrWrapper(nullptr) + stderrWrapper(nullptr), + m_fCreatedConsole(false) { InitializeSRWLock(&m_srwLock); } @@ -43,6 +44,21 @@ HRESULT PipeOutputManager::Start() HANDLE hStdErrReadPipe; HANDLE hStdErrWritePipe; + // To make Console.* functions work, allocate a console + // in the current process. + if (!AllocConsole()) + { + // ERROR_ACCESS_DENIED means there is a console already present. + if (GetLastError() != ERROR_ACCESS_DENIED) + { + RETURN_LAST_ERROR(); + } + } + else + { + m_fCreatedConsole = true; + } + RETURN_LAST_ERROR_IF(!CreatePipe(&hStdErrReadPipe, &hStdErrWritePipe, &saAttr, 0 /*nSize*/)); m_hErrReadPipe = hStdErrReadPipe; @@ -92,6 +108,11 @@ HRESULT PipeOutputManager::Stop() m_disposed = true; + if (m_fCreatedConsole) + { + FreeConsole(); + } + // Both pipe wrappers duplicate the pipe writer handle // meaning we are fine to close the handle too. if (m_hErrWritePipe != INVALID_HANDLE_VALUE) diff --git a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h index 0b51ff4038..e41bee6d10 100644 --- a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h +++ b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h @@ -39,6 +39,7 @@ private: SRWLOCK m_srwLock {}; BOOL m_disposed; BOOL m_fEnableNativeRedirection; + BOOL m_fCreatedConsole; std::unique_ptr stdoutWrapper; std::unique_ptr stderrWrapper; }; diff --git a/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs b/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs index 1234d368fc..12eae66d01 100644 --- a/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs +++ b/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs @@ -35,13 +35,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_RANDOM_VALUE"] = randomNumberString; - var deploymentResult = await DeployAsync(deploymentParameters); - - var response = await deploymentResult.HttpClient.GetAsync(path); - - Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); - - StopServer(); + await AssertFailsToStart(path, deploymentParameters); Assert.Contains(TestSink.Writes, context => context.Message.Contains($"Random number: {randomNumberString}")); } @@ -56,6 +50,25 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; + await AssertFailsToStart(path, deploymentParameters); + + Assert.Contains(TestSink.Writes, context => context.Message.Contains(new string('a', 4096))); + } + + [ConditionalFact] + public async Task CheckValidConsoleFunctions() + { + var path = "CheckConsoleFunctions"; + var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); + deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; + + await AssertFailsToStart(path, deploymentParameters); + + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Is Console redirection: True")); + } + + private async Task AssertFailsToStart(string path, IntegrationTesting.IIS.IISDeploymentParameters deploymentParameters) + { var deploymentResult = await DeployAsync(deploymentParameters); var response = await deploymentResult.HttpClient.GetAsync(path); @@ -63,8 +76,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); StopServer(); - - Assert.Contains(TestSink.Writes, context => context.Message.Contains(new string('a', 4096))); } [ConditionalFact] diff --git a/test/WebSites/StartupExceptionWebSite/Program.cs b/test/WebSites/StartupExceptionWebSite/Program.cs index 283a0136ce..44c8b1f461 100644 --- a/test/WebSites/StartupExceptionWebSite/Program.cs +++ b/test/WebSites/StartupExceptionWebSite/Program.cs @@ -2,6 +2,8 @@ // 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.Text; namespace IISTestSite { @@ -41,6 +43,14 @@ namespace IISTestSite Console.Error.WriteLine(new string('a', 4096)); Console.Error.Flush(); } + else if (envVariable == "CheckConsoleFunctions") + { + // Call a bunch of console functions and make sure none return invalid handle. + Console.OutputEncoding = Encoding.UTF8; + Console.Title = "Test"; + Console.WriteLine($"Is Console redirection: {Console.IsOutputRedirected}"); + Console.BackgroundColor = ConsoleColor.Blue; + } } } } From db01ae37170c948bf9b6d61332d67c4d0bdb6e8d Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 31 Aug 2018 08:16:07 -0700 Subject: [PATCH 05/18] Remove FreeConsole calls. (#1358) --- .../CommonLib/FileOutputManager.cpp | 12 +----------- src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h | 1 - .../CommonLib/PipeOutputManager.cpp | 12 +----------- src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h | 1 - 4 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp index 66292196a6..69cc0fe76a 100644 --- a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp @@ -20,8 +20,7 @@ FileOutputManager::FileOutputManager(bool fEnableNativeLogging) : m_disposed(false), stdoutWrapper(nullptr), stderrWrapper(nullptr), - m_fEnableNativeRedirection(fEnableNativeLogging), - m_fCreatedConsole(false) + m_fEnableNativeRedirection(fEnableNativeLogging) { InitializeSRWLock(&m_srwLock); } @@ -61,10 +60,6 @@ FileOutputManager::Start() RETURN_LAST_ERROR(); } } - else - { - m_fCreatedConsole = true; - } // Concatenate the log file name and application path RETURN_IF_FAILED(FILE_UTILITY::ConvertPathToFullPath( @@ -145,11 +140,6 @@ FileOutputManager::Stop() m_disposed = true; - if (m_fCreatedConsole) - { - RETURN_LAST_ERROR_IF(!FreeConsole()); - } - if (m_hLogFileHandle == INVALID_HANDLE_VALUE) { return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); diff --git a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h index 7cb3e452fa..94f6e19176 100644 --- a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h +++ b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h @@ -35,7 +35,6 @@ private: STRA m_straFileContent; BOOL m_disposed; BOOL m_fEnableNativeRedirection; - BOOL m_fCreatedConsole; SRWLOCK m_srwLock{}; std::unique_ptr stdoutWrapper; std::unique_ptr stderrWrapper; diff --git a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp index 20aafcc17e..60c2723fcb 100644 --- a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp @@ -24,8 +24,7 @@ PipeOutputManager::PipeOutputManager(bool fEnableNativeLogging) : m_disposed(FALSE), m_fEnableNativeRedirection(fEnableNativeLogging), stdoutWrapper(nullptr), - stderrWrapper(nullptr), - m_fCreatedConsole(false) + stderrWrapper(nullptr) { InitializeSRWLock(&m_srwLock); } @@ -54,10 +53,6 @@ HRESULT PipeOutputManager::Start() RETURN_LAST_ERROR(); } } - else - { - m_fCreatedConsole = true; - } RETURN_LAST_ERROR_IF(!CreatePipe(&hStdErrReadPipe, &hStdErrWritePipe, &saAttr, 0 /*nSize*/)); @@ -108,11 +103,6 @@ HRESULT PipeOutputManager::Stop() m_disposed = true; - if (m_fCreatedConsole) - { - FreeConsole(); - } - // Both pipe wrappers duplicate the pipe writer handle // meaning we are fine to close the handle too. if (m_hErrWritePipe != INVALID_HANDLE_VALUE) diff --git a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h index e41bee6d10..0b51ff4038 100644 --- a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h +++ b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h @@ -39,7 +39,6 @@ private: SRWLOCK m_srwLock {}; BOOL m_disposed; BOOL m_fEnableNativeRedirection; - BOOL m_fCreatedConsole; std::unique_ptr stdoutWrapper; std::unique_ptr stderrWrapper; }; From 0459b6d0d48635e72939ecf789cad32fb41578ba Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 31 Aug 2018 09:05:46 -0700 Subject: [PATCH 06/18] Fix dotnet.exe abort exception and startup/shutdown timeouts (#1352) --- .../AspNetCore/HandlerResolver.cpp | 6 +- .../AspNetCore/applicationmanager.h | 2 - .../CommonLib/CommonLib.vcxproj | 3 +- .../CommonLib/ConfigurationSection.cpp | 10 + .../CommonLib/ConfigurationSection.h | 4 + .../CommonLib/InvalidOperationException.h | 22 + .../CommonLib/ModuleHelpers.h | 20 + .../WebConfigConfigurationSection.cpp | 11 + .../CommonLib/WebConfigConfigurationSection.h | 1 + .../CommonLib/application.h | 12 +- src/AspNetCoreModuleV2/CommonLib/exceptions.h | 12 +- .../CommonLib/hostfxr_utility.cpp | 20 +- .../CommonLib/hostfxr_utility.h | 16 +- .../CommonLib/hostfxroptions.cpp | 31 +- .../CommonLib/hostfxroptions.h | 8 +- src/AspNetCoreModuleV2/CommonLib/resources.h | 9 +- .../InProcessApplicationBase.cpp | 18 +- .../InProcessApplicationBase.h | 4 - .../InProcessOptions.cpp | 37 + .../InProcessOptions.h | 17 + .../StartupExceptionApplication.cpp | 2 +- .../StartupExceptionApplication.h | 23 - .../StartupExceptionHandler.cpp | 30 +- .../StartupExceptionHandler.h | 15 +- .../InProcessRequestHandler/dllmain.cpp | 36 +- .../inprocessapplication.cpp | 751 ++++++++---------- .../inprocessapplication.h | 155 ++-- .../inprocesshandler.h | 12 +- .../IISDeploymentParameterExtensions.cs | 20 + .../IISExpressDeployer.cs | 2 +- .../ConfigurationChangeTests.cs | 2 +- .../Inprocess/EventLogTests.cs | 4 +- .../Inprocess/StartupExceptionTests.cs | 28 +- .../MultiApplicationTests.cs | 4 +- .../Utilities/EventLogHelpers.cs | 107 ++- test/CommonLibTests/CommonLibTests.vcxproj | 8 +- test/CommonLibTests/hostfxr_utility_tests.cpp | 4 +- .../inprocess_application_tests.cpp | 2 +- .../Inprocess/StdOutRedirectionTests.cs | 10 +- .../InProcess/ShutdownTests.cs | 33 +- .../InProcess/StartupTests.cs | 114 ++- .../StartupExceptionWebSite/Program.cs | 93 ++- 42 files changed, 962 insertions(+), 756 deletions(-) create mode 100644 src/AspNetCoreModuleV2/CommonLib/InvalidOperationException.h create mode 100644 src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h diff --git a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp index 2c79f60a85..bd7dacfb34 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp @@ -56,10 +56,10 @@ HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, Shim std::unique_ptr outputManager; RETURN_IF_FAILED(HOSTFXR_OPTIONS::Create( - NULL, - pConfiguration.QueryProcessPath().c_str(), + L"", + pConfiguration.QueryProcessPath(), pApplication.GetApplicationPhysicalPath(), - pConfiguration.QueryArguments().c_str(), + pConfiguration.QueryArguments(), options)); location = options->GetDotnetExeLocation(); diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index 5cb341087d..ab3fbe9746 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -8,8 +8,6 @@ #include "exceptions.h" #include -#define DEFAULT_HASH_BUCKETS 17 - // // This class will manage the lifecycle of all Asp.Net Core applciation // It should be global singleton. diff --git a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj index ef9f1c1986..20e37c30f3 100644 --- a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj +++ b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj @@ -195,7 +195,6 @@ - @@ -211,9 +210,11 @@ + + diff --git a/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.cpp b/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.cpp index 77c3319dd1..11e790cff7 100644 --- a/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.cpp @@ -26,6 +26,16 @@ bool ConfigurationSection::GetRequiredBool(const std::wstring& name) const return result.value(); } +DWORD ConfigurationSection::GetRequiredLong(const std::wstring& name) const +{ + auto result = GetLong(name); + if (!result.has_value()) + { + ThrowRequiredException(name); + } + return result.value(); +} + DWORD ConfigurationSection::GetRequiredTimespan(const std::wstring& name) const { auto result = GetTimespan(name); diff --git a/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.h b/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.h index 163f50f8e5..f998478db2 100644 --- a/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.h +++ b/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.h @@ -17,6 +17,8 @@ #define CS_ASPNETCORE_PROCESS_EXE_PATH L"processPath" #define CS_ASPNETCORE_PROCESS_ARGUMENTS L"arguments" #define CS_ASPNETCORE_PROCESS_ARGUMENTS_DEFAULT L"" +#define CS_ASPNETCORE_PROCESS_STARTUP_TIME_LIMIT L"startupTimeLimit" +#define CS_ASPNETCORE_PROCESS_SHUTDOWN_TIME_LIMIT L"shutdownTimeLimit" #define CS_ASPNETCORE_HOSTING_MODEL_OUTOFPROCESS L"outofprocess" #define CS_ASPNETCORE_HOSTING_MODEL_INPROCESS L"inprocess" #define CS_ASPNETCORE_HOSTING_MODEL L"hostingModel" @@ -31,10 +33,12 @@ public: virtual ~ConfigurationSection() = default; virtual std::optional GetString(const std::wstring& name) const = 0; virtual std::optional GetBool(const std::wstring& name) const = 0; + virtual std::optional GetLong(const std::wstring& name) const = 0; virtual std::optional GetTimespan(const std::wstring& name) const = 0; std::wstring GetRequiredString(const std::wstring& name) const; bool GetRequiredBool(const std::wstring& name) const; + DWORD GetRequiredLong(const std::wstring& name) const; DWORD GetRequiredTimespan(const std::wstring& name) const; virtual std::vector> GetKeyValuePairs(const std::wstring& name) const = 0; diff --git a/src/AspNetCoreModuleV2/CommonLib/InvalidOperationException.h b/src/AspNetCoreModuleV2/CommonLib/InvalidOperationException.h new file mode 100644 index 0000000000..3963091911 --- /dev/null +++ b/src/AspNetCoreModuleV2/CommonLib/InvalidOperationException.h @@ -0,0 +1,22 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#pragma once +#include + +class InvalidOperationException: public std::runtime_error +{ + public: + InvalidOperationException(std::wstring msg) + : runtime_error("InvalidOperationException"), message(std::move(msg)) + { + } + + std::wstring as_wstring() const + { + return message; + } + + private: + std::wstring message; +}; diff --git a/src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h b/src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h new file mode 100644 index 0000000000..0ef9a7662d --- /dev/null +++ b/src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#pragma once +#include "HandleWrapper.h" +#include "exceptions.h" + +extern HMODULE g_hModule; + +class ModuleHelpers +{ +public: + static + void IncrementCurrentModuleRefCount(HandleWrapper &handle) + { + WCHAR path[MAX_PATH]; + THROW_LAST_ERROR_IF(!GetModuleFileName(g_hModule, path, sizeof(path))); + THROW_LAST_ERROR_IF(!GetModuleHandleEx(0, path, &handle)); + } +}; diff --git a/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.cpp b/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.cpp index aa2495359b..7003442f01 100644 --- a/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.cpp @@ -28,6 +28,17 @@ std::optional WebConfigConfigurationSection::GetBool(const std::wstring& n return std::make_optional(result); } +std::optional WebConfigConfigurationSection::GetLong(const std::wstring& name) const +{ + DWORD result; + if (FAILED_LOG(GetElementDWORDProperty(m_element, name.c_str(), &result))) + { + return std::nullopt; + } + + return std::make_optional(result); +} + std::optional WebConfigConfigurationSection::GetTimespan(const std::wstring& name) const { ULONGLONG result; diff --git a/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.h b/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.h index c2f87a9cfe..b206c02049 100644 --- a/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.h +++ b/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSection.h @@ -17,6 +17,7 @@ public: std::optional GetString(const std::wstring& name) const override; std::optional GetBool(const std::wstring& name) const override; + std::optional GetLong(const std::wstring& name) const override; std::optional GetTimespan(const std::wstring& name) const override; std::vector> GetKeyValuePairs(const std::wstring& name) const override; diff --git a/src/AspNetCoreModuleV2/CommonLib/application.h b/src/AspNetCoreModuleV2/CommonLib/application.h index 5e87e4ddf8..e99159d324 100644 --- a/src/AspNetCoreModuleV2/CommonLib/application.h +++ b/src/AspNetCoreModuleV2/CommonLib/application.h @@ -7,6 +7,7 @@ #include "iapplication.h" #include "ntassert.h" #include "SRWExclusiveLock.h" +#include "SRWSharedLock.h" class APPLICATION : public IAPPLICATION { @@ -15,11 +16,6 @@ public: APPLICATION(const APPLICATION&) = delete; const APPLICATION& operator=(const APPLICATION&) = delete; - APPLICATION_STATUS - QueryStatus() override - { - return m_fStopCalled ? APPLICATION_STATUS::RECYCLED : APPLICATION_STATUS::RUNNING; - } APPLICATION(const IHttpApplication& pHttpApplication) : m_fStopCalled(false), @@ -32,6 +28,12 @@ public: m_applicationVirtualPath = ToVirtualPath(m_applicationConfigPath); } + APPLICATION_STATUS + QueryStatus() override + { + SRWSharedLock stateLock(m_stateLock); + return m_fStopCalled ? APPLICATION_STATUS::RECYCLED : APPLICATION_STATUS::RUNNING; + } VOID Stop(bool fServerInitiated) override diff --git a/src/AspNetCoreModuleV2/CommonLib/exceptions.h b/src/AspNetCoreModuleV2/CommonLib/exceptions.h index 8d9a50ded3..0bf30d1bcc 100644 --- a/src/AspNetCoreModuleV2/CommonLib/exceptions.h +++ b/src/AspNetCoreModuleV2/CommonLib/exceptions.h @@ -3,10 +3,12 @@ #pragma once +#include #include #include "debugutil.h" #include "StringHelpers.h" +#include "InvalidOperationException.h" #define LOCATION_INFO_ENABLED TRUE @@ -41,10 +43,10 @@ #define FINISHED_LAST_ERROR_IF(condition) do { if (condition) { hr = LogLastError(LOCATION_INFO); goto Finished; }} while (0, 0) #define FINISHED_LAST_ERROR_IF_NULL(ptr) do { if ((ptr) == nullptr) { hr = LogLastError(LOCATION_INFO); goto Finished; }} while (0, 0) -#define THROW_LAST_ERROR() do { ThrowResultException(LogLastError(LOCATION_INFO)); } while (0, 0) +#define THROW_LAST_ERROR() do { ThrowResultException(LOCATION_INFO, LogLastError(LOCATION_INFO)); } while (0, 0) #define THROW_IF_FAILED(hr) do { HRESULT __hrRet = hr; if (FAILED(__hrRet)) { ThrowResultException(LOCATION_INFO, __hrRet); }} while (0, 0) -#define THROW_LAST_ERROR_IF(condition) do { if (condition) { ThrowResultException(LogLastError(LOCATION_INFO)); }} while (0, 0) -#define THROW_LAST_ERROR_IF_NULL(ptr) do { if ((ptr) == nullptr) { ThrowResultException(LogLastError(LOCATION_INFO)); }} while (0, 0) +#define THROW_LAST_ERROR_IF(condition) do { if (condition) { ThrowResultException(LOCATION_INFO, LogLastError(LOCATION_INFO)); }} while (0, 0) +#define THROW_LAST_ERROR_IF_NULL(ptr) do { if ((ptr) == nullptr) { ThrowResultException(LOCATION_INFO, LogLastError(LOCATION_INFO)); }} while (0, 0) #define THROW_IF_NULL_ALLOC(ptr) Throw_IfNullAlloc(ptr) @@ -152,3 +154,7 @@ template auto Throw_IfNullAlloc(PointerT pointer) } return pointer; } +__declspec(noinline) inline std::wstring GetUnexpectedExceptionMessage(std::runtime_error& ex) +{ + return format(L"Unexpected exception: %S", ex.what()); +} diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp index eb82490ced..bc27526a41 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp @@ -41,7 +41,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( } else if (!ends_with(expandedProcessPath, L".exe", true)) { - throw StartupParametersResolutionException(format(L"Process path '%s' doesn't have '.exe' extension.", expandedProcessPath.c_str())); + throw InvalidOperationException(format(L"Process path '%s' doesn't have '.exe' extension.", expandedProcessPath.c_str())); } // Check if the absolute path is to dotnet or not. @@ -51,7 +51,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( if (applicationArguments.empty()) { - throw StartupParametersResolutionException(L"Application arguments are empty."); + throw InvalidOperationException(L"Application arguments are empty."); } if (dotnetExePath.empty()) @@ -92,7 +92,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( LOG_INFOF(L"Checking application.dll at '%ls'", applicationDllPath.c_str()); if (!is_regular_file(applicationDllPath)) { - throw StartupParametersResolutionException(format(L"Application .dll was not found at %s", applicationDllPath.c_str())); + throw InvalidOperationException(format(L"Application .dll was not found at %s", applicationDllPath.c_str())); } hostFxrDllPath = executablePath.parent_path() / "hostfxr.dll"; @@ -131,7 +131,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( // If the processPath file does not exist and it doesn't include dotnet.exe or dotnet // then it is an invalid argument. // - throw StartupParametersResolutionException(format(L"Executable was not found at '%s'", executablePath.c_str())); + throw InvalidOperationException(format(L"Executable was not found at '%s'", executablePath.c_str())); } } } @@ -185,7 +185,7 @@ HOSTFXR_UTILITY::AppendArguments( auto pwzArgs = std::unique_ptr(CommandLineToArgvW(applicationArguments.c_str(), &argc)); if (!pwzArgs) { - throw StartupParametersResolutionException(format(L"Unable parse command line arguments '%s'", applicationArguments.c_str())); + throw InvalidOperationException(format(L"Unable parse command line arguments '%s'", applicationArguments.c_str())); } for (int intArgsProcessed = 0; intArgsProcessed < argc; intArgsProcessed++) @@ -246,7 +246,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( { LOG_INFOF(L"Absolute path to dotnet.exe was not found at '%ls'", requestedPath.c_str()); - throw StartupParametersResolutionException(format(L"Could not find dotnet.exe at '%s'", processPath.c_str())); + throw InvalidOperationException(format(L"Could not find dotnet.exe at '%s'", processPath.c_str())); } const auto dotnetViaWhere = InvokeWhereToFindDotnet(); @@ -266,7 +266,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( } LOG_INFOF(L"dotnet.exe not found"); - throw StartupParametersResolutionException(format( + throw InvalidOperationException(format( L"Could not find dotnet.exe at '%s' or using the system PATH environment variable." " Check that a valid path to dotnet is on the PATH and the bitness of dotnet matches the bitness of the IIS worker process.", processPath.c_str())); @@ -284,14 +284,14 @@ HOSTFXR_UTILITY::GetAbsolutePathToHostFxr( if (!is_directory(hostFxrBase)) { - throw StartupParametersResolutionException(format(L"Unable to find hostfxr directory at %s", hostFxrBase.c_str())); + throw InvalidOperationException(format(L"Unable to find hostfxr directory at %s", hostFxrBase.c_str())); } FindDotNetFolders(hostFxrBase, versionFolders); if (versionFolders.empty()) { - throw StartupParametersResolutionException(format(L"Hostfxr directory '%s' doesn't contain any version subdirectories", hostFxrBase.c_str())); + throw InvalidOperationException(format(L"Hostfxr directory '%s' doesn't contain any version subdirectories", hostFxrBase.c_str())); } const auto highestVersion = FindHighestDotNetVersion(versionFolders); @@ -299,7 +299,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToHostFxr( if (!is_regular_file(hostFxrPath)) { - throw StartupParametersResolutionException(format(L"hostfxr.dll not found at '%s'", hostFxrPath.c_str())); + throw InvalidOperationException(format(L"hostfxr.dll not found at '%s'", hostFxrPath.c_str())); } LOG_INFOF(L"hostfxr.dll located at '%ls'", hostFxrPath.c_str()); diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h index d13b7e139e..c015fadc88 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h @@ -29,20 +29,6 @@ public: std::vector &arguments ); - class StartupParametersResolutionException: public std::runtime_error - { - public: - StartupParametersResolutionException(std::wstring msg) - : runtime_error("Startup parameter resulution error occured"), message(std::move(msg)) - { - } - - std::wstring get_message() const { return message; } - - private: - std::wstring message; - }; - static void AppendArguments( @@ -96,7 +82,7 @@ private: struct LocalFreeDeleter { - void operator ()(LPWSTR* ptr) const + void operator ()(_In_ LPWSTR* ptr) const { LocalFree(ptr); } diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp index 9e1d8ab609..060224c60d 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp @@ -9,18 +9,19 @@ #include "EventLog.h" HRESULT HOSTFXR_OPTIONS::Create( - _In_ PCWSTR pcwzDotnetExePath, - _In_ PCWSTR pcwzProcessPath, - _In_ PCWSTR pcwzApplicationPhysicalPath, - _In_ PCWSTR pcwzArguments, + _In_ const std::wstring& pcwzDotnetExePath, + _In_ const std::wstring& pcwzProcessPath, + _In_ const std::wstring& pcwzApplicationPhysicalPath, + _In_ const std::wstring& pcwzArguments, _Out_ std::unique_ptr& ppWrapper) { std::filesystem::path knownDotnetLocation; - if (pcwzDotnetExePath != nullptr) + if (!pcwzDotnetExePath.empty()) { knownDotnetLocation = pcwzDotnetExePath; } + try { std::filesystem::path hostFxrDllPath; @@ -40,17 +41,25 @@ HRESULT HOSTFXR_OPTIONS::Create( } ppWrapper = std::make_unique(knownDotnetLocation, hostFxrDllPath, arguments); } - catch (HOSTFXR_UTILITY::StartupParametersResolutionException &resolutionException) + catch (InvalidOperationException &ex) { - OBSERVE_CAUGHT_EXCEPTION(); - EventLog::Error( ASPNETCORE_EVENT_INPROCESS_START_ERROR, ASPNETCORE_EVENT_INPROCESS_START_ERROR_MSG, - pcwzApplicationPhysicalPath, - resolutionException.get_message().c_str()); + pcwzApplicationPhysicalPath.c_str(), + ex.as_wstring().c_str()); - return E_FAIL; + RETURN_CAUGHT_EXCEPTION(); + } + catch (std::runtime_error &ex) + { + EventLog::Error( + ASPNETCORE_EVENT_INPROCESS_START_ERROR, + ASPNETCORE_EVENT_INPROCESS_START_ERROR_MSG, + pcwzApplicationPhysicalPath.c_str(), + GetUnexpectedExceptionMessage(ex).c_str()); + + RETURN_CAUGHT_EXCEPTION(); } CATCH_RETURN(); diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h index 50bdfe1ded..8c42d08c37 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h @@ -48,10 +48,10 @@ public: static HRESULT Create( - _In_ PCWSTR pcwzExeLocation, - _In_ PCWSTR pcwzProcessPath, - _In_ PCWSTR pcwzApplicationPhysicalPath, - _In_ PCWSTR pcwzArguments, + _In_ const std::wstring& pcwzExeLocation, + _In_ const std::wstring& pcwzProcessPath, + _In_ const std::wstring& pcwzApplicationPhysicalPath, + _In_ const std::wstring& pcwzArguments, _Out_ std::unique_ptr& ppWrapper); private: diff --git a/src/AspNetCoreModuleV2/CommonLib/resources.h b/src/AspNetCoreModuleV2/CommonLib/resources.h index c9cca2b8b6..790e849516 100644 --- a/src/AspNetCoreModuleV2/CommonLib/resources.h +++ b/src/AspNetCoreModuleV2/CommonLib/resources.h @@ -24,14 +24,14 @@ #define ASPNETCORE_EVENT_GRACEFUL_SHUTDOWN_FAILURE_MSG L"Failed to gracefully shutdown process '%d'." #define ASPNETCORE_EVENT_SENT_SHUTDOWN_HTTP_REQUEST_MSG L"Sent shutdown HTTP message to process '%d' and received http status '%d'." #define ASPNETCORE_EVENT_APP_SHUTDOWN_FAILURE_MSG L"Failed to gracefully shutdown application '%s'." -#define ASPNETCORE_EVENT_LOAD_CLR_FALIURE_MSG L"Application '%s' with physical root '%s' failed to load clr and managed application, ErrorCode = '0x%x." +#define ASPNETCORE_EVENT_LOAD_CLR_FALIURE_MSG L"Application '%s' with physical root '%s' failed to load clr and managed application. %s" #define ASPNETCORE_EVENT_APP_SHUTDOWN_SUCCESSFUL_MSG L"Application '%s' has shutdown." #define ASPNETCORE_EVENT_DUPLICATED_INPROCESS_APP_MSG L"Only one inprocess application is allowed per IIS application pool. Please assign the application '%s' to a different IIS application pool." #define ASPNETCORE_EVENT_MIXED_HOSTING_MODEL_ERROR_MSG L"Mixed hosting model is not supported. Application '%s' configured with different hostingModel value '%d' other than the one of running application(s)." #define ASPNETCORE_CONFIGURATION_LOAD_ERROR_MSG L"Configuration load error. %s" #define ASPNETCORE_EVENT_ADD_APPLICATION_ERROR_MSG L"Failed to start application '%s', ErrorCode '0x%x'." -#define ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_STDOUT_MSG L"Application '%s' with physical root '%s' hit unexpected managed background thread exit, ErrorCode = '0x%x'. Last 4KB characters of captured stdout and stderr logs:\r\n%s" -#define ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_MSG L"Application '%s' with physical root '%s' hit unexpected managed background thread exit, ErrorCode = '0x%x'. Please check the stderr logs for more information." +#define ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_STDOUT_MSG L"Application '%s' with physical root '%s' hit unexpected managed background thread exit, exit code = '%d'. Last 4KB characters of captured stdout and stderr logs:\r\n%s" +#define ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_MSG L"Application '%s' with physical root '%s' hit unexpected managed background thread exit, exit code = '%d'. Please check the stderr logs for more information." #define ASPNETCORE_EVENT_APP_IN_SHUTDOWN_MSG L"Application shutting down." #define ASPNETCORE_EVENT_RECYCLE_APPOFFLINE_MSG L"Application '%s' was recycled after detecting the app_offline file." #define ASPNETCORE_EVENT_MONITOR_APPOFFLINE_ERROR_MSG L"Monitoring app_offline.htm failed for application '%s', ErrorCode '0x%x'. " @@ -39,7 +39,8 @@ #define ASPNETCORE_EVENT_RECYCLE_FAILURE_CONFIGURATION_MSG L"Failed to recycle application due to a configuration change at '%s'. Recycling worker process." #define ASPNETCORE_EVENT_MODULE_DISABLED_MSG L"AspNetCore Module is disabled" #define ASPNETCORE_EVENT_HOSTFXR_DLL_INVALID_VERSION_MSG L"Hostfxr version used does not support 'hostfxr_get_native_search_directories', update the version of hostfxr to a higher version. Path to hostfxr: '%s'." -#define ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION_MSG L"Application '%s' with physical root '%s' hit unexpected managed exception, ErrorCode = '0x%x. Please check the stderr logs for more information." +#define ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION_MSG L"Application '%s' with physical root '%s' hit unexpected managed exception, exception code = '0x%x. Please check the stderr logs for more information." +#define ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION_STDOUT_MSG L"Application '%s' with physical root '%s' hit unexpected managed exception, exception code = '0x%x. Last 4KB characters of captured stdout and stderr logs:\r\n%s" #define ASPNETCORE_EVENT_INPROCESS_RH_ERROR_MSG L"Could not find the assembly '%s' for in-process application. Please confirm the Microsoft.AspNetCore.Server.IIS package is referenced in your application. Captured output: %s" #define ASPNETCORE_EVENT_OUT_OF_PROCESS_RH_MISSING_MSG L"Could not find the assembly '%s' for out-of-process application. Please confirm the assembly is installed correctly for IIS or IISExpress." #define ASPNETCORE_EVENT_INPROCESS_START_SUCCESS_MSG L"Application '%s' started the coreclr in-process successfully." diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.cpp index 0abdc42025..fb9fe8a015 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.cpp @@ -3,8 +3,6 @@ #include "InProcessApplicationBase.h" -hostfxr_main_fn InProcessApplicationBase::s_fMainCallback = NULL; - InProcessApplicationBase::InProcessApplicationBase( IHttpServer& pHttpServer, IHttpApplication& pHttpApplication) @@ -35,7 +33,21 @@ InProcessApplicationBase::StopInternal(bool fServerInitiated) } else { - exit(0); + // Send WM_QUIT to the main window to initiate graceful shutdown + EnumWindows([](HWND hwnd, LPARAM) -> BOOL + { + DWORD processId; + + if (GetWindowThreadProcessId(hwnd, &processId) && + processId == GetCurrentProcessId() && + GetConsoleWindow() != hwnd) + { + PostMessage(hwnd, WM_QUIT, 0, 0); + return false; + } + + return true; + }, 0); } } diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.h b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.h index bb343725f5..5bc2dd41e1 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.h +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessApplicationBase.h @@ -3,7 +3,6 @@ #pragma once -#include "application.h" #include "AppOfflineTrackingApplication.h" typedef INT(*hostfxr_main_fn) (CONST DWORD argc, CONST PCWSTR argv[]); // TODO these may need to be BSTRs @@ -23,8 +22,5 @@ public: protected: BOOL m_fRecycleCalled; IHttpServer& m_pHttpServer; - // Allows to override call to hostfxr_main with custome callback - // used in testing - static hostfxr_main_fn s_fMainCallback; }; diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.cpp index 6b26b3dcc9..1b3e22e1dc 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.cpp @@ -2,6 +2,41 @@ // Licensed under the MIT License. See License.txt in the project root for license information. #include "InProcessOptions.h" +#include "InvalidOperationException.h" +#include "EventLog.h" + +HRESULT InProcessOptions::Create( + IHttpServer& pServer, + IHttpApplication& pHttpApplication, + std::unique_ptr& options) +{ + try + { + const WebConfigConfigurationSource configurationSource(pServer.GetAdminManager(), pHttpApplication); + options = std::make_unique(configurationSource); + } + catch (InvalidOperationException& ex) + { + EventLog::Error( + ASPNETCORE_CONFIGURATION_LOAD_ERROR, + ASPNETCORE_CONFIGURATION_LOAD_ERROR_MSG, + ex.as_wstring().c_str()); + + RETURN_CAUGHT_EXCEPTION(); + } + catch (std::runtime_error& ex) + { + EventLog::Error( + ASPNETCORE_CONFIGURATION_LOAD_ERROR, + ASPNETCORE_CONFIGURATION_LOAD_ERROR_MSG, + GetUnexpectedExceptionMessage(ex).c_str()); + + RETURN_CAUGHT_EXCEPTION(); + } + CATCH_RETURN(); + + return S_OK; +} InProcessOptions::InProcessOptions(const ConfigurationSource &configurationSource) : m_fStdoutLogEnabled(false), @@ -18,6 +53,8 @@ InProcessOptions::InProcessOptions(const ConfigurationSource &configurationSourc m_struStdoutLogFile = aspNetCoreSection->GetRequiredString(CS_ASPNETCORE_STDOUT_LOG_FILE); m_fDisableStartUpErrorPage = aspNetCoreSection->GetRequiredBool(CS_ASPNETCORE_DISABLE_START_UP_ERROR_PAGE); m_environmentVariables = aspNetCoreSection->GetKeyValuePairs(CS_ASPNETCORE_ENVIRONMENT_VARIABLES); + m_dwStartupTimeLimitInMS = aspNetCoreSection->GetRequiredLong(CS_ASPNETCORE_PROCESS_STARTUP_TIME_LIMIT) * 1000; + m_dwShutdownTimeLimitInMS = aspNetCoreSection->GetRequiredLong(CS_ASPNETCORE_PROCESS_SHUTDOWN_TIME_LIMIT) * 1000; const auto basicAuthSection = configurationSource.GetSection(CS_BASIC_AUTHENTICATION_SECTION); m_fBasicAuthEnabled = basicAuthSection && basicAuthSection->GetBool(CS_ENABLED).value_or(false); diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.h b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.h index c0509b3003..e299c2033e 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.h +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.h @@ -5,6 +5,7 @@ #include #include "ConfigurationSource.h" +#include "WebConfigConfigurationSource.h" class InProcessOptions: NonCopyable { @@ -60,12 +61,22 @@ public: DWORD QueryStartupTimeLimitInMS() const { + if (IsDebuggerPresent()) + { + return INFINITE; + } + return m_dwStartupTimeLimitInMS; } DWORD QueryShutdownTimeLimitInMS() const { + if (IsDebuggerPresent()) + { + return INFINITE; + } + return m_dwShutdownTimeLimitInMS; } @@ -76,6 +87,12 @@ public: } InProcessOptions(const ConfigurationSource &configurationSource); + + static + HRESULT InProcessOptions::Create( + IHttpServer& pServer, + IHttpApplication& pHttpApplication, + std::unique_ptr& options); private: std::wstring m_strArguments; diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.cpp index fbb5350d62..b198e95909 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.cpp @@ -5,6 +5,6 @@ HRESULT StartupExceptionApplication::CreateHandler(IHttpContext *pHttpContext, IREQUEST_HANDLER ** pRequestHandler) { - *pRequestHandler = new StartupExceptionHandler(pHttpContext, m_disableLogs, this); + *pRequestHandler = new StartupExceptionHandler(pHttpContext, m_disableLogs); return S_OK; } diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h index 369c57adfe..d3505315be 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h @@ -16,29 +16,6 @@ public: : m_disableLogs(disableLogs), InProcessApplicationBase(pServer, pApplication) { - html500Page = std::string(" \ - \ - \ - \ - IIS 500.30 Error \ -
\ -

HTTP Error 500.30 - ANCM In-Process Start Failure

\ -
\ -

Common causes of this issue:

\ -
  • The application failed to start
    • \ -
  • The application started but then stopped
    • \ -
  • The application started but threw an exception during startup
\ -
\ -
\ -

Troubleshooting steps:

\ -
  • Check the system event log for error messages
    • \ -
  • Enable logging the application process' stdout messages
    • \ -
  • Attach a debugger to the application process and inspect
\ -

For more information visit: \ - https://go.microsoft.com/fwlink/?LinkID=808681

\ -
\ -
\ -
"); } ~StartupExceptionApplication() = default; diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.cpp index f8041a0f13..d180460898 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.cpp @@ -4,6 +4,30 @@ #include "StartupExceptionApplication.h" #include "StartupExceptionHandler.h" +std::string StartupExceptionHandler::s_html500Page = " \ + \ + \ + \ + IIS 500.30 Error \ +
\ +

HTTP Error 500.30 - ANCM In-Process Start Failure

\ +
\ +

Common causes of this issue:

\ +
  • The application failed to start
    • \ +
  • The application started but then stopped
    • \ +
  • The application started but threw an exception during startup
\ +
\ +
\ +

Troubleshooting steps:

\ +
  • Check the system event log for error messages
    • \ +
  • Enable logging the application process' stdout messages
    • \ +
  • Attach a debugger to the application process and inspect
\ +

For more information visit: \ + https://go.microsoft.com/fwlink/?LinkID=808681

\ +
\ +
\ +
"; + REQUEST_NOTIFICATION_STATUS StartupExceptionHandler::OnExecuteRequestHandler() { if (!m_disableLogs) @@ -16,11 +40,9 @@ REQUEST_NOTIFICATION_STATUS StartupExceptionHandler::OnExecuteRequestHandler() (USHORT)strlen("text/html"), FALSE ); - const std::string& html500Page = m_pApplication->GetStaticHtml500Content(); - DataChunk.DataChunkType = HttpDataChunkFromMemory; - DataChunk.FromMemory.pBuffer = (PVOID)html500Page.c_str(); - DataChunk.FromMemory.BufferLength = (ULONG)html500Page.size(); + DataChunk.FromMemory.pBuffer = (PVOID)s_html500Page.c_str(); + DataChunk.FromMemory.BufferLength = (ULONG)s_html500Page.size(); pResponse->WriteEntityChunkByReference(&DataChunk); } else diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.h b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.h index 9b5a9ba45d..266474e1b5 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.h +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionHandler.h @@ -3,6 +3,7 @@ #pragma once +#include #include "requesthandler.h" class StartupExceptionApplication; @@ -10,19 +11,23 @@ class StartupExceptionApplication; class StartupExceptionHandler : public REQUEST_HANDLER { public: - StartupExceptionHandler(IHttpContext* pContext, BOOL disableLogs, StartupExceptionApplication* pApplication) + StartupExceptionHandler(IHttpContext* pContext, BOOL disableLogs) : m_pContext(pContext), - m_disableLogs(disableLogs), - m_pApplication(pApplication) + m_disableLogs(disableLogs) { } - + ~StartupExceptionHandler() + { + + } REQUEST_NOTIFICATION_STATUS OnExecuteRequestHandler() override; private: IHttpContext * m_pContext; BOOL m_disableLogs; - StartupExceptionApplication* m_pApplication; + + static + std::string s_html500Page; }; diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp index 90779ce6d8..32a314dd50 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp @@ -110,39 +110,25 @@ CreateApplication( return S_OK; } - const WebConfigConfigurationSource configurationSource(pServer->GetAdminManager(), *pHttpApplication); - auto pConfig = std::make_unique(configurationSource); - - BOOL disableStartupPage = pConfig->QueryDisableStartUpErrorPage(); - - auto pApplication = std::make_unique(*pServer, *pHttpApplication, std::move(pConfig), pParameters, nParameters); - // never create two inprocess applications in one process g_fInProcessApplicationCreated = true; - if (FAILED_LOG(pApplication->LoadManagedApplication())) + + std::unique_ptr inProcessApplication; + if (!FAILED_LOG(IN_PROCESS_APPLICATION::Start(*pServer, *pHttpApplication, pParameters, nParameters, inProcessApplication))) { + *ppApplication = inProcessApplication.release(); + } + else + { + std::unique_ptr options; + THROW_IF_FAILED(InProcessOptions::Create(*pServer, *pHttpApplication, options)); // Set the currently running application to a fake application that returns startup exceptions. - auto pErrorApplication = std::make_unique(*pServer, *pHttpApplication, disableStartupPage); + auto pErrorApplication = std::make_unique(*pServer, *pHttpApplication, options->QueryDisableStartUpErrorPage()); RETURN_IF_FAILED(pErrorApplication->StartMonitoringAppOffline()); *ppApplication = pErrorApplication.release(); } - else - { - RETURN_IF_FAILED(pApplication->StartMonitoringAppOffline()); - *ppApplication = pApplication.release(); - } - } - catch(ConfigurationLoadException &ex) - { - EventLog::Error( - ASPNETCORE_CONFIGURATION_LOAD_ERROR, - ASPNETCORE_CONFIGURATION_LOAD_ERROR_MSG, - ex.get_message().c_str()); - - RETURN_HR(E_FAIL); + return S_OK; } CATCH_RETURN(); - - return S_OK; } diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp index 5f43952287..942bfe44b0 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp @@ -6,13 +6,11 @@ #include "hostfxroptions.h" #include "requesthandler_config.h" #include "environmentvariablehelpers.h" -#include "SRWExclusiveLock.h" #include "exceptions.h" #include "LoggingHelpers.h" #include "resources.h" #include "EventLog.h" - -const LPCSTR IN_PROCESS_APPLICATION::s_exeLocationParameterName = "InProcessExeLocation"; +#include "ModuleHelpers.h" IN_PROCESS_APPLICATION* IN_PROCESS_APPLICATION::s_Application = NULL; @@ -23,10 +21,9 @@ IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( APPLICATION_PARAMETER *pParameters, DWORD nParameters) : InProcessApplicationBase(pHttpServer, pApplication), - m_ProcessExitCode(0), - m_fBlockCallbacksIntoManaged(FALSE), - m_fShutdownCalledFromNative(FALSE), - m_fShutdownCalledFromManaged(FALSE), + m_Initialized(false), + m_blockManagedCallbacks(true), + m_waitForShutdown(true), m_pConfig(std::move(pConfig)) { DBG_ASSERT(m_pConfig); @@ -35,161 +32,54 @@ IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( { if (_stricmp(pParameters[i].pzName, s_exeLocationParameterName) == 0) { - m_struExeLocation.Copy(reinterpret_cast(pParameters[i].pValue)); + m_dotnetExeKnownLocation = reinterpret_cast(pParameters[i].pValue); } } - - m_status = MANAGED_APPLICATION_STATUS::STARTING; } IN_PROCESS_APPLICATION::~IN_PROCESS_APPLICATION() { - s_Application = NULL; + s_Application = nullptr; } -//static -DWORD WINAPI -IN_PROCESS_APPLICATION::DoShutDown( - LPVOID lpParam -) -{ - IN_PROCESS_APPLICATION* pApplication = static_cast(lpParam); - DBG_ASSERT(pApplication); - pApplication->ShutDownInternal(); - return 0; -} - -__override VOID IN_PROCESS_APPLICATION::StopInternal(bool fServerInitiated) -{ - UNREFERENCED_PARAMETER(fServerInitiated); - HRESULT hr = S_OK; - CHandle hThread; - DWORD dwThreadStatus = 0; - - DWORD dwTimeout = m_pConfig->QueryShutdownTimeLimitInMS(); - - if (IsDebuggerPresent()) - { - dwTimeout = INFINITE; - } - - hThread.Attach(CreateThread( - NULL, // default security attributes - 0, // default stack size - (LPTHREAD_START_ROUTINE)DoShutDown, - this, // thread function arguments - 0, // default creation flags - NULL)); // receive thread identifier - - if ((HANDLE)hThread == NULL) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - goto Finished; - } - - if (WaitForSingleObject(hThread, dwTimeout) != WAIT_OBJECT_0) - { - // if the thread is still running, we need kill it first before exit to avoid AV - if (GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && dwThreadStatus == STILL_ACTIVE) - { - // Calling back into managed at this point is prone to have AVs - // Calling terminate thread here may be our best solution. - TerminateThread(hThread, STATUS_CONTROL_C_EXIT); - hr = HRESULT_FROM_WIN32(ERROR_TIMEOUT); - } - } - -Finished: - - if (FAILED(hr)) - { - EventLog::Warn( - ASPNETCORE_EVENT_GRACEFUL_SHUTDOWN_FAILURE, - ASPNETCORE_EVENT_APP_SHUTDOWN_FAILURE_MSG, - QueryConfigPath().c_str()); - } - else - { - EventLog::Info( - ASPNETCORE_EVENT_APP_SHUTDOWN_SUCCESSFUL, - ASPNETCORE_EVENT_APP_SHUTDOWN_SUCCESSFUL_MSG, - QueryConfigPath().c_str()); - } - +{ + StopClr(); InProcessApplicationBase::StopInternal(fServerInitiated); } VOID -IN_PROCESS_APPLICATION::ShutDownInternal() -{ - DWORD dwThreadStatus = 0; - DWORD dwTimeout = m_pConfig->QueryShutdownTimeLimitInMS(); +IN_PROCESS_APPLICATION::StopClr() +{ + LOG_INFO(L"Stopping CLR"); - if (IsDebuggerPresent()) + if (!m_blockManagedCallbacks) { - dwTimeout = INFINITE; - } + // We cannot call into managed if the dll is detaching from the process. + // Calling into managed code when the dll is detaching is strictly a bad idea, + // and usually results in an AV saying "The string binding is invalid" + const auto shutdownHandler = m_ShutdownHandler; - if (m_fShutdownCalledFromNative || - m_status == MANAGED_APPLICATION_STATUS::STARTING || - m_status == MANAGED_APPLICATION_STATUS::FAIL) - { - return; - } - - { - if (m_fShutdownCalledFromNative || - m_status == MANAGED_APPLICATION_STATUS::STARTING || - m_status == MANAGED_APPLICATION_STATUS::FAIL) + if (!g_fProcessDetach && shutdownHandler != nullptr) { - return; - } - - // We need to keep track of when both managed and native initiate shutdown - // to avoid AVs. If shutdown has already been initiated in managed, we don't want to call into - // managed. We still need to wait on main exiting no matter what. m_fShutdownCalledFromNative - // is used for detecting redundant calls and blocking more requests to OnExecuteRequestHandler. - m_fShutdownCalledFromNative = TRUE; - m_status = MANAGED_APPLICATION_STATUS::SHUTDOWN; - - if (!m_fShutdownCalledFromManaged) - { - // We cannot call into managed if the dll is detaching from the process. - // Calling into managed code when the dll is detaching is strictly a bad idea, - // and usually results in an AV saying "The string binding is invalid" - if (!g_fProcessDetach) - { - m_ShutdownHandler(m_ShutdownHandlerContext); - m_ShutdownHandler = NULL; - } - } - - // Release the lock before we wait on the thread to exit. - } - - if (!m_fShutdownCalledFromManaged) - { - if (m_hThread != NULL && - GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && - dwThreadStatus == STILL_ACTIVE) - { - // wait for graceful shutdown, i.e., the exit of the background thread or timeout - if (WaitForSingleObject(m_hThread, dwTimeout) != WAIT_OBJECT_0) - { - // if the thread is still running, we need kill it first before exit to avoid AV - if (GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && dwThreadStatus == STILL_ACTIVE) - { - // Calling back into managed at this point is prone to have AVs - // Calling terminate thread here may be our best solution. - TerminateThread(m_hThread, STATUS_CONTROL_C_EXIT); - } - } + shutdownHandler(m_ShutdownHandlerContext); } } - s_Application = NULL; + // Signal shutdown + if (m_pShutdownEvent != nullptr) + { + LOG_IF_FAILED(SetEvent(m_pShutdownEvent)); + } + + if (m_workerThread.joinable()) + { + // Worker thread would wait for clr to finish and log error if required + m_workerThread.join(); + } + + s_Application = nullptr; } VOID @@ -201,169 +91,320 @@ IN_PROCESS_APPLICATION::SetCallbackHandles( _In_ VOID* pvShutdownHandlerContext ) { + LOG_INFO(L"In-process callbacks set"); + m_RequestHandler = request_handler; m_RequestHandlerContext = pvRequstHandlerContext; m_ShutdownHandler = shutdown_handler; m_ShutdownHandlerContext = pvShutdownHandlerContext; m_AsyncCompletionHandler = async_completion_handler; + m_blockManagedCallbacks = false; + m_Initialized = true; + // Can't check the std err handle as it isn't a critical error // Initialization complete EventLog::Info( ASPNETCORE_EVENT_INPROCESS_START_SUCCESS, ASPNETCORE_EVENT_INPROCESS_START_SUCCESS_MSG, QueryApplicationPhysicalPath().c_str()); - SetEvent(m_pInitalizeEvent); - m_fInitialized = TRUE; + + SetEvent(m_pInitializeEvent); } -// Will be called by the inprocesshandler HRESULT -IN_PROCESS_APPLICATION::LoadManagedApplication -( - VOID -) +IN_PROCESS_APPLICATION::LoadManagedApplication() { - HRESULT hr = S_OK; - DWORD dwTimeout; - DWORD dwResult; + THROW_LAST_ERROR_IF_NULL(m_pInitializeEvent = CreateEvent( + nullptr, // default security attributes + TRUE, // manual reset event + FALSE, // not set + nullptr)); // name - ReferenceApplication(); + THROW_LAST_ERROR_IF_NULL(m_pShutdownEvent = CreateEvent( + nullptr, // default security attributes + TRUE, // manual reset event + FALSE, // not set + nullptr)); // name - if (m_status != MANAGED_APPLICATION_STATUS::STARTING) + m_workerThread = std::thread([](std::unique_ptr application) { - // Core CLR has already been loaded. - // Cannot load more than once even there was a failure - if (m_status == MANAGED_APPLICATION_STATUS::FAIL) - { - hr = E_APPLICATION_ACTIVATION_EXEC_FAILURE; - } - else if (m_status == MANAGED_APPLICATION_STATUS::SHUTDOWN) - { - hr = HRESULT_FROM_WIN32(ERROR_SHUTDOWN_IS_SCHEDULED); - } + LOG_INFO(L"Starting in-process worker thread"); + application->ExecuteApplication(); + LOG_INFO(L"Stopping in-process worker thread"); + }, ::ReferenceApplication(this)); - goto Finished; + LOG_INFO(L"Waiting for initialization"); + + const HANDLE waitHandles[2] = { m_pInitializeEvent, m_workerThread.native_handle() }; + + // Wait for shutdown request + const auto waitResult = WaitForMultipleObjects(2, waitHandles, FALSE, m_pConfig->QueryStartupTimeLimitInMS()); + THROW_LAST_ERROR_IF(waitResult == WAIT_FAILED); + + if (waitResult == WAIT_TIMEOUT) + { + // If server wasn't initialized in time shut application down without waiting for CLR thread to exit + m_waitForShutdown = false; + StopClr(); + throw InvalidOperationException(format(L"Managed server didn't initialize after %u ms.", m_pConfig->QueryStartupTimeLimitInMS())); } + // WAIT_OBJECT_0 + 1 is the worker thead handle + if (waitResult == WAIT_OBJECT_0 + 1) { - SRWExclusiveLock lock(m_stateLock); + // Worker thread exited stop + StopClr(); + throw InvalidOperationException(format(L"CLR worker thread exited prematurely")); + } - if (m_status != MANAGED_APPLICATION_STATUS::STARTING) + THROW_IF_FAILED(StartMonitoringAppOffline()); + + return S_OK; +} + + +void +IN_PROCESS_APPLICATION::ExecuteApplication() +{ + try + { + std::unique_ptr hostFxrOptions; + + auto context = std::make_shared(); + + auto pProc = s_fMainCallback; + if (pProc == nullptr) { - if (m_status == MANAGED_APPLICATION_STATUS::FAIL) + HMODULE hModule; + // hostfxr should already be loaded by the shim. If not, then we will need + // to load it ourselves by finding hostfxr again. + THROW_LAST_ERROR_IF_NULL(hModule = GetModuleHandle(L"hostfxr.dll")); + + // Get the entry point for main + pProc = reinterpret_cast(GetProcAddress(hModule, "hostfxr_main")); + THROW_LAST_ERROR_IF_NULL(pProc); + + THROW_IF_FAILED(HOSTFXR_OPTIONS::Create( + m_dotnetExeKnownLocation, + m_pConfig->QueryProcessPath(), + QueryApplicationPhysicalPath(), + m_pConfig->QueryArguments(), + hostFxrOptions + )); + + hostFxrOptions->GetArguments(context->m_argc, context->m_argv); + THROW_IF_FAILED(SetEnvironmentVariablesOnWorkerProcess()); + } + context->m_pProc = pProc; + + if (m_pLoggerProvider == nullptr) + { + THROW_IF_FAILED(LoggingHelpers::CreateLoggingProvider( + m_pConfig->QueryStdoutLogEnabled(), + !m_pHttpServer.IsCommandLineLaunch(), + m_pConfig->QueryStdoutLogFile().c_str(), + QueryApplicationPhysicalPath().c_str(), + m_pLoggerProvider)); + + LOG_IF_FAILED(m_pLoggerProvider->Start()); + } + + // There can only ever be a single instance of .NET Core + // loaded in the process but we need to get config information to boot it up in the + // first place. This is happening in an execute request handler and everyone waits + // until this initialization is done. + // We set a static so that managed code can call back into this instance and + // set the callbacks + s_Application = this; + + //Start CLR thread + m_clrThread = std::thread(ClrThreadEntryPoint, context); + + // Wait for thread exit or shutdown event + const HANDLE waitHandles[2] = { m_pShutdownEvent, m_clrThread.native_handle() }; + + // Wait for shutdown request + const auto waitResult = WaitForMultipleObjects(2, waitHandles, FALSE, INFINITE); + THROW_LAST_ERROR_IF(waitResult == WAIT_FAILED); + + LOG_INFOF(L"Starting shutdown sequence %d", waitResult); + + bool clrThreadExited = waitResult == (WAIT_OBJECT_0 + 1); + // shutdown was signaled + // only wait for shutdown in case of successful startup + if (m_waitForShutdown) + { + const auto clrWaitResult = WaitForSingleObject(m_clrThread.native_handle(), m_pConfig->QueryShutdownTimeLimitInMS()); + THROW_LAST_ERROR_IF(waitResult == WAIT_FAILED); + + clrThreadExited = clrWaitResult != WAIT_TIMEOUT; + } + + LOG_INFOF(L"Clr thread wait ended: clrThreadExited: %d", clrThreadExited); + + // At this point CLR thread either finished or timed out, abandon it. + m_clrThread.detach(); + + LOG_IF_FAILED(m_pLoggerProvider->Stop()); + + if (m_fStopCalled) + { + if (clrThreadExited) { - hr = E_APPLICATION_ACTIVATION_EXEC_FAILURE; + EventLog::Info( + ASPNETCORE_EVENT_APP_SHUTDOWN_SUCCESSFUL, + ASPNETCORE_EVENT_APP_SHUTDOWN_SUCCESSFUL_MSG, + QueryConfigPath().c_str()); } - else if (m_status == MANAGED_APPLICATION_STATUS::SHUTDOWN) + else { - hr = HRESULT_FROM_WIN32(ERROR_SHUTDOWN_IS_SCHEDULED); + EventLog::Warn( + ASPNETCORE_EVENT_GRACEFUL_SHUTDOWN_FAILURE, + ASPNETCORE_EVENT_APP_SHUTDOWN_FAILURE_MSG, + QueryConfigPath().c_str()); } - - goto Finished; - } - m_hThread = CreateThread( - NULL, // default security attributes - 0, // default stack size - (LPTHREAD_START_ROUTINE)ExecuteAspNetCoreProcess, - this, // thread function arguments - 0, // default creation flags - NULL); // receive thread identifier - - if (m_hThread == NULL) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - goto Finished; - } - - m_pInitalizeEvent = CreateEvent( - NULL, // default security attributes - TRUE, // manual reset event - FALSE, // not set - NULL); // name - - if (m_pInitalizeEvent == NULL) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - } - - // If the debugger is attached, never timeout - if (IsDebuggerPresent()) - { - dwTimeout = INFINITE; } else { - dwTimeout = m_pConfig->QueryStartupTimeLimitInMS(); + if (clrThreadExited) + { + UnexpectedThreadExit(*context); + // If the inprocess server was initialized, we need to cause recycle to be called on the worker process. + // in case when it was not initialized we need to keep server running to serve 502 page + if (m_Initialized) + { + QueueStop(); + } + } } - - const HANDLE pHandles[2]{ m_hThread, m_pInitalizeEvent }; - - // Wait on either the thread to complete or the event to be set - dwResult = WaitForMultipleObjects(2, pHandles, FALSE, dwTimeout); - - // It all timed out - if (dwResult == WAIT_TIMEOUT) - { - // kill the backend thread as loading dotnet timedout - TerminateThread(m_hThread, 0); - hr = HRESULT_FROM_WIN32(dwResult); - goto Finished; - } - else if (dwResult == WAIT_FAILED) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - goto Finished; - } - - // The thread ended it means that something failed - if (dwResult == WAIT_OBJECT_0) - { - hr = E_APPLICATION_ACTIVATION_EXEC_FAILURE; - goto Finished; - } - - m_status = MANAGED_APPLICATION_STATUS::RUNNING_MANAGED; } -Finished: - - if (FAILED(hr)) + catch (InvalidOperationException& ex) { - m_status = MANAGED_APPLICATION_STATUS::FAIL; - EventLog::Error( ASPNETCORE_EVENT_LOAD_CLR_FALIURE, ASPNETCORE_EVENT_LOAD_CLR_FALIURE_MSG, QueryApplicationId().c_str(), QueryApplicationPhysicalPath().c_str(), - hr); - } - DereferenceApplication(); + ex.as_wstring().c_str()); - return hr; + OBSERVE_CAUGHT_EXCEPTION(); + } + catch (std::runtime_error& ex) + { + EventLog::Error( + ASPNETCORE_EVENT_LOAD_CLR_FALIURE, + ASPNETCORE_EVENT_LOAD_CLR_FALIURE_MSG, + QueryApplicationId().c_str(), + QueryApplicationPhysicalPath().c_str(), + GetUnexpectedExceptionMessage(ex).c_str()); + + OBSERVE_CAUGHT_EXCEPTION(); + } } -// static -VOID -IN_PROCESS_APPLICATION::ExecuteAspNetCoreProcess( - _In_ LPVOID pContext -) +void IN_PROCESS_APPLICATION::QueueStop() { - HRESULT hr = S_OK; - IN_PROCESS_APPLICATION *pApplication = (IN_PROCESS_APPLICATION*)pContext; - DBG_ASSERT(pApplication != NULL); - hr = pApplication->ExecuteApplication(); - // - // no need to log the error here as if error happened, the thread will exit - // the error will ba catched by caller LoadManagedApplication which will log an error - // + if (m_fStopCalled) + { + return; + } + LOG_INFO(L"Queueing in-process stop thread"); + + std::thread stoppingThread([](std::unique_ptr application) + { + LOG_INFO(L"Starting in-process stop thread"); + application->Stop(false); + LOG_INFO(L"Stopping in-process stop thread"); + }, ::ReferenceApplication(this)); + + stoppingThread.detach(); +} + +HRESULT IN_PROCESS_APPLICATION::Start( + IHttpServer& pServer, + IHttpApplication& pHttpApplication, + APPLICATION_PARAMETER* pParameters, + DWORD nParameters, + std::unique_ptr& application) +{ + try + { + std::unique_ptr options; + THROW_IF_FAILED(InProcessOptions::Create(pServer, pHttpApplication, options)); + application = std::unique_ptr( + new IN_PROCESS_APPLICATION(pServer, pHttpApplication, std::move(options), pParameters, nParameters)); + THROW_IF_FAILED(application->LoadManagedApplication()); + return S_OK; + } + catch (InvalidOperationException& ex) + { + EventLog::Error( + ASPNETCORE_EVENT_LOAD_CLR_FALIURE, + ASPNETCORE_EVENT_LOAD_CLR_FALIURE_MSG, + pHttpApplication.GetApplicationId(), + pHttpApplication.GetApplicationPhysicalPath(), + ex.as_wstring().c_str()); + + RETURN_CAUGHT_EXCEPTION(); + } + catch (std::runtime_error& ex) + { + EventLog::Error( + ASPNETCORE_EVENT_LOAD_CLR_FALIURE, + ASPNETCORE_EVENT_LOAD_CLR_FALIURE_MSG, + pHttpApplication.GetApplicationId(), + pHttpApplication.GetApplicationPhysicalPath(), + GetUnexpectedExceptionMessage(ex).c_str()); + + RETURN_CAUGHT_EXCEPTION(); + } + CATCH_RETURN(); +} + +// Required because __try and objects with destructors can not be mixed +void +IN_PROCESS_APPLICATION::ExecuteClr(const std::shared_ptr& context) +{ + __try + { + auto const exitCode = context->m_pProc(context->m_argc, context->m_argv.get()); + + LOG_INFOF(L"Managed application exited with code %d", exitCode); + + context->m_exitCode = exitCode; + } + __except(GetExceptionCode() != 0) + { + LOG_INFOF(L"Managed threw an exception %d", GetExceptionCode()); + + context->m_exceptionCode = GetExceptionCode(); + } +} + +// +// Calls hostfxr_main with the hostfxr and application as arguments. +// This method should not access IN_PROCESS_APPLICATION instance as it may be already freed +// in case of startup timeout +// +VOID +IN_PROCESS_APPLICATION::ClrThreadEntryPoint(const std::shared_ptr &context) +{ + LOG_INFO(L"Starting CLR thread"); + + // Keep aspnetcorev2_inprocess.dll loaded while this thread is running + // this is required because thread might be abandoned + HandleWrapper moduleHandle; + ModuleHelpers::IncrementCurrentModuleRefCount(moduleHandle); + + ExecuteClr(context); + + FreeLibraryAndExitThread(moduleHandle.release(), 0); } HRESULT -IN_PROCESS_APPLICATION::SetEnvironementVariablesOnWorkerProcess( - VOID -) +IN_PROCESS_APPLICATION::SetEnvironmentVariablesOnWorkerProcess() { auto variables = m_pConfig->QueryEnvironmentVariables(); auto inputTable = std::unique_ptr(new ENVIRONMENT_VAR_HASH()); @@ -397,128 +438,53 @@ IN_PROCESS_APPLICATION::SetEnvironementVariablesOnWorkerProcess( return S_OK; } -HRESULT -IN_PROCESS_APPLICATION::ExecuteApplication( - VOID -) -{ - HRESULT hr; - HMODULE hModule = nullptr; - hostfxr_main_fn pProc; - std::unique_ptr hostFxrOptions = NULL; - DWORD hostfxrArgc = 0; - std::unique_ptr hostfxrArgv; - DBG_ASSERT(m_status == MANAGED_APPLICATION_STATUS::STARTING); - - pProc = s_fMainCallback; - if (pProc == nullptr) - { - // hostfxr should already be loaded by the shim. If not, then we will need - // to load it ourselves by finding hostfxr again. - hModule = LoadLibraryW(L"hostfxr.dll"); - - if (hModule == NULL) - { - // .NET Core not installed (we can log a more detailed error message here) - hr = LOG_IF_FAILED(ERROR_BAD_ENVIRONMENT); - goto Finished; - } - - // Get the entry point for main - pProc = (hostfxr_main_fn)GetProcAddress(hModule, "hostfxr_main"); - if (pProc == NULL) - { - hr = LOG_IF_FAILED(ERROR_BAD_ENVIRONMENT); - goto Finished; - } - - FINISHED_IF_FAILED(hr = HOSTFXR_OPTIONS::Create( - m_struExeLocation.QueryStr(), - m_pConfig->QueryProcessPath().c_str(), - QueryApplicationPhysicalPath().c_str(), - m_pConfig->QueryArguments().c_str(), - hostFxrOptions - )); - hostFxrOptions->GetArguments(hostfxrArgc, hostfxrArgv); - FINISHED_IF_FAILED(SetEnvironementVariablesOnWorkerProcess()); - } - - LOG_INFO(L"Starting managed application"); - - if (m_pLoggerProvider == NULL) - { - FINISHED_IF_FAILED(hr = LoggingHelpers::CreateLoggingProvider( - m_pConfig->QueryStdoutLogEnabled(), - !m_pHttpServer.IsCommandLineLaunch(), - m_pConfig->QueryStdoutLogFile().c_str(), - QueryApplicationPhysicalPath().c_str(), - m_pLoggerProvider)); - - LOG_IF_FAILED(m_pLoggerProvider->Start()); - } - - // There can only ever be a single instance of .NET Core - // loaded in the process but we need to get config information to boot it up in the - // first place. This is happening in an execute request handler and everyone waits - // until this initialization is done. - // We set a static so that managed code can call back into this instance and - // set the callbacks - s_Application = this; - - hr = RunDotnetApplication(hostfxrArgc, hostfxrArgv.get(), pProc); - -Finished: - - // - // this method is called by the background thread and should never exit unless shutdown - // If main returned and shutdown was not called in managed, we want to block native from calling into - // managed. To do this, we can say that shutdown was called from managed. - // Don't bother locking here as there will always be a race between receiving a native shutdown - // notification and unexpected managed exit. - // - m_status = MANAGED_APPLICATION_STATUS::SHUTDOWN; - m_fShutdownCalledFromManaged = TRUE; - - m_pLoggerProvider->Stop(); - - if (!m_fShutdownCalledFromNative) - { - LogErrorsOnMainExit(hr); - if (m_fInitialized) - { - // - // If the inprocess server was initialized, we need to cause recycle to be called on the worker process. - // - Stop(/*fServerInitiated*/ false); - } - } - - return hr; -} - VOID -IN_PROCESS_APPLICATION::LogErrorsOnMainExit( - HRESULT hr -) +IN_PROCESS_APPLICATION::UnexpectedThreadExit(const ExecuteClrContext& context) const { + STRA straStdErrOutput; + STRU struStdMsg; + auto hasStdOut = m_pLoggerProvider->GetStdOutContent(&straStdErrOutput) && + SUCCEEDED(struStdMsg.CopyA(straStdErrOutput.QueryStr())); + + if (context.m_exceptionCode != 0) + { + if (hasStdOut) + { + EventLog::Error( + ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION, + ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION_STDOUT_MSG, + QueryApplicationId().c_str(), + QueryApplicationPhysicalPath().c_str(), + context.m_exceptionCode, + struStdMsg.QueryStr()); + } + else + { + EventLog::Error( + ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION, + ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION_MSG, + QueryApplicationId().c_str(), + QueryApplicationPhysicalPath().c_str(), + context.m_exceptionCode + ); + } + return; + } + // // Ungraceful shutdown, try to log an error message. // This will be a common place for errors as it means the hostfxr_main returned // or there was an exception. // - STRA straStdErrOutput; - STRU struStdMsg; - if (m_pLoggerProvider->GetStdOutContent(&straStdErrOutput)) + if (hasStdOut) { - if (SUCCEEDED(struStdMsg.CopyA(straStdErrOutput.QueryStr()))) { - EventLog::Error( - ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_STDOUT, - ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_STDOUT_MSG, - QueryApplicationId().c_str(), - QueryApplicationPhysicalPath().c_str(), - hr, - struStdMsg.QueryStr()); - } + EventLog::Error( + ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_STDOUT, + ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_STDOUT_MSG, + QueryApplicationId().c_str(), + QueryApplicationPhysicalPath().c_str(), + context.m_exitCode, + struStdMsg.QueryStr()); } else { @@ -527,53 +493,20 @@ IN_PROCESS_APPLICATION::LogErrorsOnMainExit( ASPNETCORE_EVENT_INPROCESS_THREAD_EXIT_MSG, QueryApplicationId().c_str(), QueryApplicationPhysicalPath().c_str(), - hr); + context.m_exitCode); } } -// -// Calls hostfxr_main with the hostfxr and application as arguments. -// -HRESULT -IN_PROCESS_APPLICATION::RunDotnetApplication(DWORD argc, CONST PCWSTR* argv, hostfxr_main_fn pProc) -{ - HRESULT hr = S_OK; - - __try - { - m_ProcessExitCode = pProc(argc, argv); - if (m_ProcessExitCode != 0) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - } - - LOG_INFOF(L"Managed application exited with code %d", m_ProcessExitCode); - } - __except(GetExceptionCode() != 0) - { - - LOG_INFOF(L"Managed threw an exception %d", GetExceptionCode()); - hr = HRESULT_FROM_WIN32(GetLastError()); - } - - return hr; -} - HRESULT IN_PROCESS_APPLICATION::CreateHandler( _In_ IHttpContext *pHttpContext, _Out_ IREQUEST_HANDLER **pRequestHandler) { - HRESULT hr = S_OK; - IREQUEST_HANDLER* pHandler = NULL; - - pHandler = new IN_PROCESS_HANDLER(::ReferenceApplication(this), pHttpContext, m_RequestHandler, m_RequestHandlerContext, m_AsyncCompletionHandler); - - if (pHandler == NULL) + try { - hr = HRESULT_FROM_WIN32(ERROR_OUTOFMEMORY); + *pRequestHandler = new IN_PROCESS_HANDLER(::ReferenceApplication(this), pHttpContext, m_RequestHandler, m_RequestHandlerContext, m_AsyncCompletionHandler); } - - *pRequestHandler = pHandler; - return hr; + CATCH_RETURN(); + + return S_OK; } diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h index 7183ad86b3..cab7a49e9b 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h @@ -3,6 +3,7 @@ #pragma once +#include #include "InProcessApplicationBase.h" #include "IOutputManager.h" #include "InProcessOptions.h" @@ -45,35 +46,26 @@ public: override; // Executes the .NET Core process + void + ExecuteApplication(); + HRESULT - ExecuteApplication( - VOID - ); + LoadManagedApplication(); - HRESULT - LoadManagedApplication( - VOID - ); - VOID - LogErrorsOnMainExit( - HRESULT hr - ); - - VOID - StopCallsIntoManaged( - VOID - ) + void + QueueStop(); + + void + StopIncomingRequests() { - m_fBlockCallbacksIntoManaged = TRUE; + QueueStop(); } - VOID - StopIncomingRequests( - VOID - ) + void + StopCallsIntoManaged() { - m_fShutdownCalledFromManaged = TRUE; + m_blockManagedCallbacks = true; } static @@ -84,44 +76,64 @@ public: static IN_PROCESS_APPLICATION* - GetInstance( - VOID - ) + GetInstance() { return s_Application; } - PCWSTR - QueryExeLocation() + const std::wstring& + QueryExeLocation() const { - return m_struExeLocation.QueryStr(); + return m_dotnetExeKnownLocation; } const InProcessOptions& QueryConfig() const { - return *m_pConfig.get(); + return *m_pConfig; } bool QueryBlockCallbacksIntoManaged() const { - return m_fBlockCallbacksIntoManaged; + return m_blockManagedCallbacks; } + static + HRESULT Start( + IHttpServer& pServer, + IHttpApplication& pHttpApplication, + APPLICATION_PARAMETER* pParameters, + DWORD nParameters, + std::unique_ptr& application); + private: - - enum MANAGED_APPLICATION_STATUS + struct ExecuteClrContext: std::enable_shared_from_this { - UNKNOWN = 0, - STARTING, - RUNNING_MANAGED, - SHUTDOWN, - FAIL - }; + ExecuteClrContext(): + m_argc(0), + m_pProc(nullptr), + m_exitCode(0), + m_exceptionCode(0) + { + } - // Thread executing the .NET Core process - HandleWrapper m_hThread; + DWORD m_argc; + std::unique_ptr m_argv; + hostfxr_main_fn m_pProc; + + int m_exitCode; + int m_exceptionCode; + }; + + // Thread executing the .NET Core process this might be abandoned in timeout cases + std::thread m_clrThread; + // Thread tracking the CLR thread, this one is always joined on shutdown + std::thread m_workerThread; + // The event that gets triggered when managed initialization is complete + HandleWrapper m_pInitializeEvent; + // The event that gets triggered when worker thread should exit + HandleWrapper m_pShutdownEvent; // The request handler callback from managed code PFN_REQUEST_HANDLER m_RequestHandler; @@ -133,53 +145,38 @@ private: PFN_ASYNC_COMPLETION_HANDLER m_AsyncCompletionHandler; - // The event that gets triggered when managed initialization is complete - HandleWrapper m_pInitalizeEvent; + std::wstring m_dotnetExeKnownLocation; - STRU m_struExeLocation; + std::atomic_bool m_blockManagedCallbacks; + bool m_Initialized; + bool m_waitForShutdown; - // The exit code of the .NET Core process - INT m_ProcessExitCode; - - volatile BOOL m_fBlockCallbacksIntoManaged; - volatile BOOL m_fShutdownCalledFromNative; - volatile BOOL m_fShutdownCalledFromManaged; - BOOL m_fInitialized; - MANAGED_APPLICATION_STATUS m_status; std::unique_ptr m_pConfig; static IN_PROCESS_APPLICATION* s_Application; std::unique_ptr m_pLoggerProvider; - static const LPCSTR s_exeLocationParameterName; - - static - VOID - ExecuteAspNetCoreProcess( - _In_ LPVOID pContext - ); - - HRESULT - SetEnvironementVariablesOnWorkerProcess( - VOID - ); - - HRESULT - RunDotnetApplication( - DWORD argc, - CONST PCWSTR* argv, - hostfxr_main_fn pProc - ); - - static - DWORD WINAPI - DoShutDown( - LPVOID lpParam - ); + inline static const LPCSTR s_exeLocationParameterName = "InProcessExeLocation"; VOID - ShutDownInternal( - VOID - ); + UnexpectedThreadExit(const ExecuteClrContext& context) const; + + HRESULT + SetEnvironmentVariablesOnWorkerProcess(); + + void + StopClr(); + + static + void + ClrThreadEntryPoint(const std::shared_ptr &context); + + static + void + ExecuteClr(const std::shared_ptr &context); + + // Allows to override call to hostfxr_main with custom callback + // used in testing + inline static hostfxr_main_fn s_fMainCallback = nullptr; }; diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.h b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.h index ac6aa24789..6089adc3ca 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.h +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.h @@ -40,9 +40,7 @@ public: ) override; IHttpContext* - QueryHttpContext( - VOID - ) const + QueryHttpContext() const { return m_pW3Context; } @@ -53,9 +51,7 @@ public: ); VOID - IndicateManagedRequestComplete( - VOID - ); + IndicateManagedRequestComplete(); VOID SetAsyncCompletionStatus( @@ -68,11 +64,11 @@ public: static HRESULT - StaticInitialize(VOID); + StaticInitialize(); static void - StaticTerminate(VOID); + StaticTerminate(); private: REQUEST_NOTIFICATION_STATUS diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs index 485c61cf4c..3cb2d40c3a 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs @@ -60,5 +60,25 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS deploymentParameters.WebConfigActionList.Add( WebConfigHelpers.AddOrModifyAspNetCoreSection("stdoutLogFile", Path.Combine(path, "std"))); } + + public static void TransformPath(this IISDeploymentParameters parameters, Func transformation) + { + parameters.WebConfigActionList.Add( + (config, contentRoot) => + { + var aspNetCoreElement = config.Descendants("aspNetCore").Single(); + aspNetCoreElement.SetAttributeValue("processPath", transformation((string)aspNetCoreElement.Attribute("processPath"), contentRoot)); + }); + } + + public static void TransformArguments(this IISDeploymentParameters parameters, Func transformation) + { + parameters.WebConfigActionList.Add( + (config, contentRoot) => + { + var aspNetCoreElement = config.Descendants("aspNetCore").Single(); + aspNetCoreElement.SetAttributeValue("arguments", transformation((string)aspNetCoreElement.Attribute("arguments"), contentRoot)); + }); + } } } diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs index 8464088f48..4604fb5652 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs @@ -483,7 +483,7 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS } else { - throw new InvalidOperationException($"iisexpress Process {hostProcess.Id} crashed before shutdown was triggered."); + throw new InvalidOperationException($"iisexpress Process {hostProcess?.Id} crashed before shutdown was triggered."); } } } diff --git a/test/Common.FunctionalTests/ConfigurationChangeTests.cs b/test/Common.FunctionalTests/ConfigurationChangeTests.cs index d1e5f62ff1..014e2520c4 100644 --- a/test/Common.FunctionalTests/ConfigurationChangeTests.cs +++ b/test/Common.FunctionalTests/ConfigurationChangeTests.cs @@ -83,7 +83,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests await deploymentResult.HttpClient.RetryRequestAsync("/HelloWorld", r => r.StatusCode == HttpStatusCode.InternalServerError); StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Could not find the assembly 'aspnetcorev2_inprocess.dll'"); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, "Could not find the assembly 'aspnetcorev2_inprocess.dll'"); } [ConditionalTheory] diff --git a/test/Common.FunctionalTests/Inprocess/EventLogTests.cs b/test/Common.FunctionalTests/Inprocess/EventLogTests.cs index 296d99f789..1df7f3c077 100644 --- a/test/Common.FunctionalTests/Inprocess/EventLogTests.cs +++ b/test/Common.FunctionalTests/Inprocess/EventLogTests.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Application '.+' started the coreclr in-process successfully."); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, "Application '.+' started the coreclr in-process successfully."); } [ConditionalFact] @@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Application '.+' has shutdown."); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, "Application '.+' has shutdown."); } } } diff --git a/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs b/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs index 12eae66d01..dbbf09f1ac 100644 --- a/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs +++ b/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs @@ -5,6 +5,7 @@ using System; using System.Net; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; +using Microsoft.AspNetCore.Server.IntegrationTesting.IIS; using Microsoft.AspNetCore.Testing.xunit; using Xunit; @@ -23,19 +24,14 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests [ConditionalTheory] [InlineData("CheckLogFile")] [InlineData("CheckErrLogFile")] - public async Task CheckStdoutWithRandomNumber(string path) + public async Task CheckStdoutWithRandomNumber(string mode) { - // Forcing publish for now to have parity between IIS and IISExpress - // Reason is because by default for IISExpress, we expect there to not be a web.config file. - // However, for IIS, we need a web.config file because the default on generated on publish - // doesn't include V2. We can remove the publish flag once IIS supports non-publish running var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); var randomNumberString = new Random(Guid.NewGuid().GetHashCode()).Next(10000000).ToString(); - deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; - deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_RANDOM_VALUE"] = randomNumberString; + deploymentParameters.TransformArguments((a, _) => $"{a} {mode} {randomNumberString}"); - await AssertFailsToStart(path, deploymentParameters); + await AssertFailsToStart(deploymentParameters); Assert.Contains(TestSink.Writes, context => context.Message.Contains($"Random number: {randomNumberString}")); } @@ -45,12 +41,12 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests [InlineData("CheckLargeStdOutWrites")] [InlineData("CheckOversizedStdErrWrites")] [InlineData("CheckOversizedStdOutWrites")] - public async Task CheckStdoutWithLargeWrites(string path) + public async Task CheckStdoutWithLargeWrites(string mode) { var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); - deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; + deploymentParameters.TransformArguments((a, _) => $"{a} {mode}"); - await AssertFailsToStart(path, deploymentParameters); + await AssertFailsToStart(deploymentParameters); Assert.Contains(TestSink.Writes, context => context.Message.Contains(new string('a', 4096))); } @@ -58,20 +54,19 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests [ConditionalFact] public async Task CheckValidConsoleFunctions() { - var path = "CheckConsoleFunctions"; var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); - deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; + deploymentParameters.TransformArguments((a, _) => $"{a} CheckConsoleFunctions"); - await AssertFailsToStart(path, deploymentParameters); + await AssertFailsToStart(deploymentParameters); Assert.Contains(TestSink.Writes, context => context.Message.Contains("Is Console redirection: True")); } - private async Task AssertFailsToStart(string path, IntegrationTesting.IIS.IISDeploymentParameters deploymentParameters) + private async Task AssertFailsToStart(IntegrationTesting.IIS.IISDeploymentParameters deploymentParameters) { var deploymentResult = await DeployAsync(deploymentParameters); - var response = await deploymentResult.HttpClient.GetAsync(path); + var response = await deploymentResult.HttpClient.GetAsync("/"); Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); @@ -92,5 +87,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests var responseText = await response.Content.ReadAsStringAsync(); Assert.Contains("500.30 - ANCM In-Process Start Failure", responseText); } + } } diff --git a/test/Common.FunctionalTests/MultiApplicationTests.cs b/test/Common.FunctionalTests/MultiApplicationTests.cs index e0adccfc54..3723369c89 100644 --- a/test/Common.FunctionalTests/MultiApplicationTests.cs +++ b/test/Common.FunctionalTests/MultiApplicationTests.cs @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal(200, (int)result1.StatusCode); Assert.Equal(500, (int)result2.StatusCode); StopServer(); - EventLogHelpers.VerifyEventLogEvent(result, TestSink, "Only one inprocess application is allowed per IIS application pool"); + EventLogHelpers.VerifyEventLogEvent(result, "Only one inprocess application is allowed per IIS application pool"); } [ConditionalTheory] @@ -72,7 +72,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal(200, (int)result1.StatusCode); Assert.Equal(500, (int)result2.StatusCode); StopServer(); - EventLogHelpers.VerifyEventLogEvent(result, TestSink, "Mixed hosting model is not supported."); + EventLogHelpers.VerifyEventLogEvent(result, "Mixed hosting model is not supported."); } private void SetHostingModel(string directory, HostingModel model) diff --git a/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs b/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs index 6d2393f187..4fb129feaf 100644 --- a/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs +++ b/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs @@ -1,54 +1,64 @@ // 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.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Text; using System.Text.RegularExpressions; using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.AspNetCore.Server.IntegrationTesting.IIS; -using Microsoft.Extensions.Logging.Testing; using Xunit; -using Xunit.Sdk; namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { public class EventLogHelpers { - private static readonly Regex EventLogRegex = new Regex("Event Log: (?.+?)End Event Log Message.", RegexOptions.Singleline | RegexOptions.Compiled); - - public static void VerifyEventLogEvent(IISDeploymentResult deploymentResult, ITestSink testSink, string expectedRegexMatchString) + public static void VerifyEventLogEvent(IISDeploymentResult deploymentResult, string expectedRegexMatchString) { Assert.True(deploymentResult.HostProcess.HasExited); - var builder = new StringBuilder(); + var entries = GetEntries(deploymentResult); + AssertSingleEntry(expectedRegexMatchString, entries); + } + + public static void VerifyEventLogEvents(IISDeploymentResult deploymentResult, params string[] expectedRegexMatchString) + { + Assert.True(deploymentResult.HostProcess.HasExited); - foreach (var context in testSink.Writes) + var entries = GetEntries(deploymentResult).ToList(); + foreach (var regexString in expectedRegexMatchString) { - builder.Append(context.Message); - } + var matchedEntries = AssertSingleEntry(regexString, entries); - var count = 0; - var expectedRegex = new Regex(expectedRegexMatchString, RegexOptions.Singleline); - foreach (Match match in EventLogRegex.Matches(builder.ToString())) - { - var eventLogText = match.Groups["EventLogMessage"].Value; - if (expectedRegex.IsMatch(eventLogText)) + foreach (var matchedEntry in matchedEntries) { - count++; + entries.Remove(matchedEntry); } } + + Assert.True(0 == entries.Count, $"Some entries were not matched by any regex {FormatEntries(entries)}"); + } - Assert.True(count > 0, $"'{expectedRegexMatchString}' didn't match any event log messaged"); - Assert.True(count < 2, $"'{expectedRegexMatchString}' matched more then one event log message"); + private static EventLogEntry[] AssertSingleEntry(string regexString, IEnumerable entries) + { + var expectedRegex = new Regex(regexString, RegexOptions.Singleline); + var matchedEntries = entries.Where(entry => expectedRegex.IsMatch(entry.Message)).ToArray(); + Assert.True(matchedEntries.Length > 0, $"No entries matched by '{regexString}'"); + Assert.True(matchedEntries.Length < 2, $"Multiple entries matched by '{regexString}': {FormatEntries(matchedEntries)}"); + return matchedEntries; + } + private static string FormatEntries(IEnumerable entries) + { + return string.Join(",", entries.Select(e => e.Message)); + } + + private static IEnumerable GetEntries(IISDeploymentResult deploymentResult) + { var eventLog = new EventLog("Application"); // Eventlog is already sorted based on time of event in ascending time. // Check results in reverse order. - var expectedRegexEventLog = new Regex(expectedRegexMatchString); var processIdString = $"Process Id: {deploymentResult.HostProcess.Id}."; // Event log messages round down to the nearest second, so subtract a second @@ -67,18 +77,15 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { continue; } - + // ReplacementStings == EventData collection in EventLog // This is unaffected if event providers are not registered correctly if (eventLogEntry.Source == AncmVersionToMatch(deploymentResult) && - processIdString == eventLogEntry.ReplacementStrings[1] && - expectedRegex.IsMatch(eventLogEntry.ReplacementStrings[0])) + processIdString == eventLogEntry.ReplacementStrings[1]) { - return; + yield return eventLogEntry; } } - - Assert.True(false, $"'{expectedRegexMatchString}' didn't match any event log messaged."); } private static string AncmVersionToMatch(IISDeploymentResult deploymentResult) @@ -88,5 +95,51 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests "AspNetCore Module" + (deploymentResult.DeploymentParameters.AncmVersion == AncmVersion.AspNetCoreModuleV2 ? " V2" : ""); } + + + public static string InProcessStarted(IISDeploymentResult deploymentResult) + { + return $"Application '{EscapedContentRoot(deploymentResult)}' started the coreclr in-process successfully"; + } + + public static string InProcessFailedToStart(IISDeploymentResult deploymentResult, string reason) + { + return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' failed to load clr and managed application. {reason}"; + } + + public static string InProcessFailedToStop(IISDeploymentResult deploymentResult, string reason) + { + return "Failed to gracefully shutdown application 'MACHINE/WEBROOT/APPHOST/HTTPTESTSITE'."; + } + + public static string InProcessThreadException(IISDeploymentResult deploymentResult, string reason) + { + return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed exception{reason}"; + } + + public static string InProcessThreadExit(IISDeploymentResult deploymentResult, string code) + { + return $"Application '/LM/W3SVC/1/ROOT' with physical root '{EscapedContentRoot(deploymentResult)}' hit unexpected managed background thread exit, exit code = '{code}'."; + } + + public static string FailedToStartApplication(IISDeploymentResult deploymentResult, string code) + { + return $"Failed to start application '/LM/W3SVC/1/ROOT', ErrorCode '{code}'."; + } + + public static string ConfigurationLoadError(IISDeploymentResult deploymentResult, string reason) + { + return $"Configuration load error. {reason}"; + } + + private static string EscapedContentRoot(IISDeploymentResult deploymentResult) + { + var contentRoot = deploymentResult.ContentRoot; + if (!contentRoot.EndsWith('\\')) + { + contentRoot += '\\'; + } + return Regex.Escape(contentRoot); + } } } diff --git a/test/CommonLibTests/CommonLibTests.vcxproj b/test/CommonLibTests/CommonLibTests.vcxproj index ef1cf020d5..87dbd16675 100644 --- a/test/CommonLibTests/CommonLibTests.vcxproj +++ b/test/CommonLibTests/CommonLibTests.vcxproj @@ -101,7 +101,7 @@ true Console ..\..\src\AspNetCoreModuleV2\InProcessRequestHandler\$(Configuration)\; - kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;%(AdditionalDependencies) + kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;inprocessoptions.obj;%(AdditionalDependencies) UseLinkTimeCodeGeneration @@ -128,7 +128,7 @@ true Console ..\..\src\AspNetCoreModuleV2\InProcessRequestHandler\x64\$(Configuration)\; - kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;%(AdditionalDependencies) + kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;inprocessoptions.obj;%(AdditionalDependencies) UseLinkTimeCodeGeneration @@ -156,7 +156,7 @@ true true ..\..\src\AspNetCoreModuleV2\InProcessRequestHandler\$(Configuration)\; - kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;%(AdditionalDependencies) + kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;inprocessoptions.obj;%(AdditionalDependencies) UseLinkTimeCodeGeneration @@ -184,7 +184,7 @@ true true ..\..\src\AspNetCoreModuleV2\InProcessRequestHandler\x64\$(Configuration)\; - kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;%(AdditionalDependencies) + kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;inprocessapplication.obj;inprocesshandler.obj;ahadmin.lib;Rpcrt4.lib;inprocessapplicationbase.obj;stdafx.obj;version.lib;inprocessoptions.obj;%(AdditionalDependencies) UseLinkTimeCodeGeneration diff --git a/test/CommonLibTests/hostfxr_utility_tests.cpp b/test/CommonLibTests/hostfxr_utility_tests.cpp index a9beb49676..01c9541429 100644 --- a/test/CommonLibTests/hostfxr_utility_tests.cpp +++ b/test/CommonLibTests/hostfxr_utility_tests.cpp @@ -64,7 +64,7 @@ TEST(ParseHostFxrArguments, ProvideNoArgs_InvalidArgs) struHostFxrDllLocation, struExeLocation, bstrArray), // args array. - HOSTFXR_UTILITY::StartupParametersResolutionException); + InvalidOperationException); } TEST(GetAbsolutePathToDotnetFromProgramFiles, BackupWorks) @@ -118,5 +118,5 @@ TEST(GetHostFxrArguments, InvalidParams) struHostFxrDllLocation, struExeLocation, bstrArray), // args array. - HOSTFXR_UTILITY::StartupParametersResolutionException); + InvalidOperationException); } diff --git a/test/CommonLibTests/inprocess_application_tests.cpp b/test/CommonLibTests/inprocess_application_tests.cpp index 06be5493bd..d2ec985723 100644 --- a/test/CommonLibTests/inprocess_application_tests.cpp +++ b/test/CommonLibTests/inprocess_application_tests.cpp @@ -40,7 +40,7 @@ namespace InprocessTests IN_PROCESS_APPLICATION *app = new IN_PROCESS_APPLICATION(server, application, std::move(requestHandlerConfig), parameters.data(), 1); - ASSERT_STREQ(app->QueryExeLocation(), L"hello"); + ASSERT_STREQ(app->QueryExeLocation().c_str(), L"hello"); } TEST(InProcessTest, GeneratesVirtualPath) diff --git a/test/IIS.FunctionalTests/Inprocess/StdOutRedirectionTests.cs b/test/IIS.FunctionalTests/Inprocess/StdOutRedirectionTests.cs index 49088180c1..fea6d3b5fb 100644 --- a/test/IIS.FunctionalTests/Inprocess/StdOutRedirectionTests.cs +++ b/test/IIS.FunctionalTests/Inprocess/StdOutRedirectionTests.cs @@ -51,7 +51,7 @@ namespace IIS.FunctionalTests.Inprocess StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, + EventLogHelpers.VerifyEventLogEvent(deploymentResult, "The specified framework 'Microsoft.NETCore.App', version '2.9.9' was not found."); } @@ -75,7 +75,7 @@ namespace IIS.FunctionalTests.Inprocess var contents = File.ReadAllText(Helpers.GetExpectedLogName(deploymentResult, _logFolderPath)); var expectedString = "The specified framework 'Microsoft.NETCore.App', version '2.9.9' was not found."; - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, expectedString); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, expectedString); Assert.Contains(expectedString, contents); } @@ -99,7 +99,7 @@ namespace IIS.FunctionalTests.Inprocess var fileInDirectory = Directory.GetFiles(_logFolderPath).Single(); var contents = File.ReadAllText(fileInDirectory); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Invoked hostfxr"); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, "Invoked hostfxr"); Assert.Contains("Invoked hostfxr", contents); } @@ -124,7 +124,7 @@ namespace IIS.FunctionalTests.Inprocess StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Invoked hostfxr"); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, "Invoked hostfxr"); } [ConditionalTheory] @@ -153,7 +153,7 @@ namespace IIS.FunctionalTests.Inprocess var fileInDirectory = Directory.GetFiles(_logFolderPath).First(); var contents = File.ReadAllText(fileInDirectory); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Invoked hostfxr"); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, "Invoked hostfxr"); Assert.Contains("Invoked hostfxr", contents); } diff --git a/test/IISExpress.FunctionalTests/InProcess/ShutdownTests.cs b/test/IISExpress.FunctionalTests/InProcess/ShutdownTests.cs index 1e5af7c13f..caaf728c64 100644 --- a/test/IISExpress.FunctionalTests/InProcess/ShutdownTests.cs +++ b/test/IISExpress.FunctionalTests/InProcess/ShutdownTests.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Net.Http; +using System.Net.Sockets; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; using Microsoft.AspNetCore.Server.IntegrationTesting; @@ -26,16 +27,42 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests public async Task ServerShutsDownWhenMainExits() { var parameters = _fixture.GetBaseDeploymentParameters(publish: true); - var result = await DeployAsync(parameters); + var deploymentResult = await DeployAsync(parameters); try { - await result.HttpClient.GetAsync("/Shutdown"); + await deploymentResult.HttpClient.GetAsync("/Shutdown"); } catch (HttpRequestException ex) when (ex.InnerException is IOException) { // Server might close a connection before request completes } - Assert.True(result.HostShutdownToken.WaitHandle.WaitOne(TimeoutExtensions.DefaultTimeout)); + + deploymentResult.AssertWorkerProcessStop(); + } + + + [ConditionalFact] + public async Task ServerShutsDownWhenMainExitsStress() + { + var parameters = _fixture.GetBaseDeploymentParameters(publish: true); + var deploymentResult = await StartAsync(parameters); + + var load = Helpers.StressLoad(deploymentResult.HttpClient, "/HelloWorld", response => { + var statusCode = (int)response.StatusCode; + Assert.True(statusCode == 200 || statusCode == 503, "Status code was " + statusCode); + }); + + try + { + await deploymentResult.HttpClient.GetAsync("/Shutdown"); + await load; + } + catch (HttpRequestException ex) when (ex.InnerException is IOException | ex.InnerException is SocketException) + { + // Server might close a connection before request completes + } + + deploymentResult.AssertWorkerProcessStop(); } [ConditionalFact] diff --git a/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs b/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs index cab621a3e6..4c288ef90e 100644 --- a/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs +++ b/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, $@"Application '{Regex.Escape(deploymentResult.ContentRoot)}\\' wasn't able to start. {subError}"); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, $@"Application '{Regex.Escape(deploymentResult.ContentRoot)}\\' wasn't able to start. {subError}"); } [ConditionalFact] @@ -137,11 +137,67 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { var deploymentResult = await DeployAsync(_fixture.GetBaseDeploymentParameters(_fixture.OverriddenServerWebSite, publish: true)); var response = await deploymentResult.HttpClient.GetAsync("/"); - Assert.False(response.IsSuccessStatusCode); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + + StopServer(); + + EventLogHelpers.VerifyEventLogEvents(deploymentResult, + EventLogHelpers.InProcessFailedToStart(deploymentResult, "CLR worker thread exited prematurely"), + EventLogHelpers.InProcessThreadException(deploymentResult, ".*?Application is running inside IIS process but is not configured to use IIS server")); + } + + [ConditionalFact] + public async Task LogsUnexpectedThreadExitError() + { + var deploymentResult = await DeployAsync(_fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true)); + + var response = await deploymentResult.HttpClient.GetAsync("/"); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); StopServer(); - Assert.Contains(TestSink.Writes, context => context.Message.Contains("Application is running inside IIS process but is not configured to use IIS server")); + EventLogHelpers.VerifyEventLogEvents(deploymentResult, + EventLogHelpers.InProcessFailedToStart(deploymentResult, "CLR worker thread exited prematurely"), + EventLogHelpers.InProcessThreadExit(deploymentResult, "12")); + } + + [ConditionalFact] + public async Task StartupTimeoutIsApplied() + { + var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); + deploymentParameters.TransformArguments((a, _) => $"{a} Hang"); + deploymentParameters.WebConfigActionList.Add( + WebConfigHelpers.AddOrModifyAspNetCoreSection("startupTimeLimit", "1")); + + var deploymentResult = await DeployAsync(deploymentParameters); + + var response = await deploymentResult.HttpClient.GetAsync("/"); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + + StopServer(); + + EventLogHelpers.VerifyEventLogEvents(deploymentResult, + EventLogHelpers.InProcessFailedToStart(deploymentResult, "Managed server didn't initialize after 1000 ms.") + ); + } + + [ConditionalFact] + public async Task ShutdownTimeoutIsApplied() + { + var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); + deploymentParameters.TransformArguments((a, _) => $"{a} HangOnStop"); + deploymentParameters.WebConfigActionList.Add( + WebConfigHelpers.AddOrModifyAspNetCoreSection("shutdownTimeLimit", "1")); + + var deploymentResult = await DeployAsync(deploymentParameters); + + Assert.Equal("OK", await deploymentResult.HttpClient.GetStringAsync("/")); + + StopServer(); + + EventLogHelpers.VerifyEventLogEvents(deploymentResult, + EventLogHelpers.InProcessStarted(deploymentResult), + EventLogHelpers.InProcessFailedToStop(deploymentResult, "")); } [ConditionalFact] @@ -158,10 +214,12 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Unknown hosting model 'bogus'. Please specify either hostingModel=\"inprocess\" or hostingModel=\"outofprocess\" in the web.config file."); + EventLogHelpers.VerifyEventLogEvents(deploymentResult, + EventLogHelpers.FailedToStartApplication(deploymentResult, "0x80004005"), + EventLogHelpers.ConfigurationLoadError(deploymentResult, "Unknown hosting model 'bogus'. Please specify either hostingModel=\"inprocess\" or hostingModel=\"outofprocess\" in the web.config file.") + ); } - private static Dictionary)> InvalidConfigTransformations = InitInvalidConfigTransformations(); public static IEnumerable InvalidConfigTransformationsScenarios => InvalidConfigTransformations.ToTheoryData(); @@ -177,7 +235,10 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal(HttpStatusCode.InternalServerError, result.StatusCode); StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Configuration load error. " + expectedError); + EventLogHelpers.VerifyEventLogEvents(deploymentResult, + EventLogHelpers.FailedToStartApplication(deploymentResult, "0x80004005"), + EventLogHelpers.ConfigurationLoadError(deploymentResult, expectedError) + ); } public static Dictionary)> InitInvalidConfigTransformations() @@ -223,42 +284,42 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests dictionary.Add("App in bin subdirectory full path to dll using exec and quotes", parameters => { MoveApplication(parameters, "bin"); - TransformArguments(parameters, (arguments, root) => "exec " + Path.Combine(root, "bin", arguments)); + parameters.TransformArguments((arguments, root) => "exec " + Path.Combine(root, "bin", arguments)); return ""; }); dictionary.Add("App in subdirectory with space", parameters => { MoveApplication(parameters, pathWithSpace); - TransformArguments(parameters, (arguments, root) => Path.Combine(pathWithSpace, arguments)); + parameters.TransformArguments((arguments, root) => Path.Combine(pathWithSpace, arguments)); return ""; }); dictionary.Add("App in subdirectory with space and full path to dll", parameters => { MoveApplication(parameters, pathWithSpace); - TransformArguments(parameters, (arguments, root) => Path.Combine(root, pathWithSpace, arguments)); + parameters.TransformArguments((arguments, root) => Path.Combine(root, pathWithSpace, arguments)); return ""; }); dictionary.Add("App in bin subdirectory with space full path to dll using exec and quotes", parameters => { MoveApplication(parameters, pathWithSpace); - TransformArguments(parameters, (arguments, root) => "exec \"" + Path.Combine(root, pathWithSpace, arguments) + "\" extra arguments"); + parameters.TransformArguments((arguments, root) => "exec \"" + Path.Combine(root, pathWithSpace, arguments) + "\" extra arguments"); return "extra|arguments"; }); dictionary.Add("App in bin subdirectory and quoted argument", parameters => { MoveApplication(parameters, "bin"); - TransformArguments(parameters, (arguments, root) => Path.Combine("bin", arguments) + " \"extra argument\""); + parameters.TransformArguments((arguments, root) => Path.Combine("bin", arguments) + " \"extra argument\""); return "extra argument"; }); dictionary.Add("App in bin subdirectory full path to dll", parameters => { MoveApplication(parameters, "bin"); - TransformArguments(parameters, (arguments, root) => Path.Combine(root, "bin", arguments) + " extra arguments"); + parameters.TransformArguments((arguments, root) => Path.Combine(root, "bin", arguments) + " extra arguments"); return "extra|arguments"; }); return dictionary; @@ -288,16 +349,16 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests dictionary.Add("App in subdirectory", parameters => { MoveApplication(parameters, pathWithSpace); - TransformPath(parameters, (path, root) => Path.Combine(pathWithSpace, path)); - TransformArguments(parameters, (arguments, root) => "\"additional argument\""); + parameters.TransformPath((path, root) => Path.Combine(pathWithSpace, path)); + parameters.TransformArguments((arguments, root) => "\"additional argument\""); return "additional argument"; }); dictionary.Add("App in bin subdirectory full path", parameters => { MoveApplication(parameters, pathWithSpace); - TransformPath(parameters, (path, root) => Path.Combine(root, pathWithSpace, path)); - TransformArguments(parameters, (arguments, root) => "additional arguments"); + parameters.TransformPath((path, root) => Path.Combine(root, pathWithSpace, path)); + parameters.TransformArguments((arguments, root) => "additional arguments"); return "additional|arguments"; }); @@ -322,26 +383,5 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } }); } - - private static void TransformPath(IISDeploymentParameters parameters, Func transformation) - { - parameters.WebConfigActionList.Add( - (config, contentRoot) => - { - var aspNetCoreElement = config.Descendants("aspNetCore").Single(); - aspNetCoreElement.SetAttributeValue("processPath", transformation((string)aspNetCoreElement.Attribute("processPath"), contentRoot)); - }); - } - - private static void TransformArguments(IISDeploymentParameters parameters, Func transformation) - { - parameters.WebConfigActionList.Add( - (config, contentRoot) => - { - var aspNetCoreElement = config.Descendants("aspNetCore").Single(); - aspNetCoreElement.SetAttributeValue("arguments", transformation((string)aspNetCoreElement.Attribute("arguments"), contentRoot)); - }); - } - } } diff --git a/test/WebSites/StartupExceptionWebSite/Program.cs b/test/WebSites/StartupExceptionWebSite/Program.cs index 44c8b1f461..750f247481 100644 --- a/test/WebSites/StartupExceptionWebSite/Program.cs +++ b/test/WebSites/StartupExceptionWebSite/Program.cs @@ -2,54 +2,75 @@ // 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; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; namespace IISTestSite { public static class Program { - public static void Main(string[] args) + public static int Main(string[] args) { - var envVariable = Environment.GetEnvironmentVariable("ASPNETCORE_INPROCESS_STARTUP_VALUE"); - var randomNumber = Environment.GetEnvironmentVariable("ASPNETCORE_INPROCESS_RANDOM_VALUE"); + var mode = args.FirstOrDefault(); - // Semicolons are appended to env variables; removing them. - if (envVariable == "CheckLargeStdOutWrites") + switch (mode) { - Console.WriteLine(new string('a', 4096)); - } - else if (envVariable == "CheckLargeStdErrWrites") - { - Console.Error.WriteLine(new string('a', 4096)); - Console.Error.Flush(); - } - else if (envVariable == "CheckLogFile") - { - Console.WriteLine($"Random number: {randomNumber}"); - } - else if (envVariable == "CheckErrLogFile") - { - Console.Error.WriteLine($"Random number: {randomNumber}"); - Console.Error.Flush(); - } - else if (envVariable == "CheckOversizedStdErrWrites") - { - Console.WriteLine(new string('a', 5000)); + // Semicolons are appended to env variables; removing them. + case "CheckLargeStdOutWrites": + Console.WriteLine(new string('a', 4096)); + break; + case "CheckLargeStdErrWrites": + Console.Error.WriteLine(new string('a', 4096)); + Console.Error.Flush(); + break; + case "CheckLogFile": + Console.WriteLine($"Random number: {args[1]}"); + break; + case "CheckErrLogFile": + Console.Error.WriteLine($"Random number: {args[1]}"); + Console.Error.Flush(); + break; + case "CheckOversizedStdErrWrites": + Console.WriteLine(new string('a', 5000)); + break; + case "CheckOversizedStdOutWrites": + Console.Error.WriteLine(new string('a', 4096)); + Console.Error.Flush(); + break; + case "Hang": + Thread.Sleep(Timeout.Infinite); + break; + case "HangOnStop": + + var host = new WebHostBuilder() + .UseIIS() + .UseStartup() + .Build(); + host.Run(); + Thread.Sleep(Timeout.Infinite); + break; + case "CheckConsoleFunctions": + // Call a bunch of console functions and make sure none return invalid handle. + Console.OutputEncoding = Encoding.UTF8; + Console.Title = "Test"; + Console.WriteLine($"Is Console redirection: {Console.IsOutputRedirected}"); + Console.BackgroundColor = ConsoleColor.Blue; + break; } - else if (envVariable == "CheckOversizedStdOutWrites") + + return 12; + } + + public partial class Startup + { + public void Configure(IApplicationBuilder app) { - Console.Error.WriteLine(new string('a', 4096)); - Console.Error.Flush(); - } - else if (envVariable == "CheckConsoleFunctions") - { - // Call a bunch of console functions and make sure none return invalid handle. - Console.OutputEncoding = Encoding.UTF8; - Console.Title = "Test"; - Console.WriteLine($"Is Console redirection: {Console.IsOutputRedirected}"); - Console.BackgroundColor = ConsoleColor.Blue; + app.Run(async context => await context.Response.WriteAsync("OK")); } } } From d22e54244923a22d0b09e09b7e7cc0c80758c8fc Mon Sep 17 00:00:00 2001 From: "ASP.NET CI" Date: Sun, 2 Sep 2018 12:15:01 -0700 Subject: [PATCH 07/18] Update dependencies.props [auto-updated: dependencies] --- build/dependencies.props | 52 ++++++++++++++++++++-------------------- korebuild-lock.txt | 4 ++-- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 87d39807ff..3ecafbac3f 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -4,38 +4,38 @@ 0.10.13 - 2.2.0-preview1-20180807.2 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 0.6.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 + 2.2.0-preview1-20180821.1 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 0.6.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 15.6.82 15.6.82 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 - 2.2.0-preview1-34967 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 + 2.2.0-preview2-35143 2.0.9 2.1.2 2.2.0-preview1-26618-02 1.0.1 - 2.2.0-preview1-34967 + 2.2.0-preview2-35143 15.6.1 11.1.0 2.0.3 diff --git a/korebuild-lock.txt b/korebuild-lock.txt index 4a53936b7b..524a2323d0 100644 --- a/korebuild-lock.txt +++ b/korebuild-lock.txt @@ -1,2 +1,2 @@ -version:2.2.0-preview1-20180816.6 -commithash:226c8adaefd0d19db0d9d11d911ba7d95644b224 +version:2.2.0-preview1-20180821.1 +commithash:c8d0cc52cd1abb697be24e288ffd54f8fae8bf17 From b5ff991eb95c71915aff1eb6283388dc02e9c67e Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 4 Sep 2018 14:05:54 -0700 Subject: [PATCH 08/18] Fix incorrect version in version.props file (#1363) --- version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.props b/version.props index aff221a96a..e897351a24 100644 --- a/version.props +++ b/version.props @@ -5,7 +5,7 @@ 2 $(DotNetMajorVersion).$(DotNetMinorVersion).$(DotNetPatchVersion) 12 - $(DotNetMajorVersion) + $(DotNetMinorVersion) $(DotNetPatchVersion) rtm $(VersionPrefix) From 9b5e34e197f2c614573884f9b6c4e0c48fe2717a Mon Sep 17 00:00:00 2001 From: "ASP.NET CI" Date: Wed, 5 Sep 2018 16:34:49 -0700 Subject: [PATCH 09/18] Update branding to 2.2.0-preview3 --- version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.props b/version.props index 623841625a..378470ee63 100644 --- a/version.props +++ b/version.props @@ -7,7 +7,7 @@ 12 $(DotNetMinorVersion) $(DotNetPatchVersion) - preview2 + preview3 $(VersionPrefix) $(VersionPrefix)-$(VersionSuffix)-final t000 From 2f4172e7e70b18da3b1a5d8a22a6fb397ca3b837 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 5 Sep 2018 16:43:09 -0700 Subject: [PATCH 10/18] Reduce probability of MultipleAppTests failure (#1365) - Decrease false-positive failure rate from 1 in 1,000 to 1 in 1,000,000 - Addresses #1350 --- .../OutOfProcess/MultipleAppTests.cs | 81 +++++++++++++------ 1 file changed, 58 insertions(+), 23 deletions(-) diff --git a/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs b/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs index cf647f46e7..cc270d9afa 100644 --- a/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs +++ b/test/IISExpress.FunctionalTests/OutOfProcess/MultipleAppTests.cs @@ -1,6 +1,7 @@ // 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.Collections.Generic; using System.Net; using System.Net.Http; @@ -25,42 +26,76 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests [ConditionalTheory] [InlineData(AncmVersion.AspNetCoreModule)] [InlineData(AncmVersion.AspNetCoreModuleV2)] - public async Task Startup(AncmVersion ancmVersion) + public Task Startup(AncmVersion ancmVersion) { - const int numApps = 10; + // ANCM v1 currently does *not* retry if an app fails to start the first time due to a port collision. + // So, this test is expected to fail on v1 approximately 1 in 1,000 times (probably of at least one collision + // when 10 sites choose a random port from the range 1025-48000). Adding one retry should reduce the failure + // rate from 1 in 1,000 to 1 in 1,000,000. The previous product code (with "srand(GetTickCount())") should still + // fail the test reliably. + // https://github.com/aspnet/IISIntegration/issues/1350 + // + // ANCM v2 does retry on port collisions, so no retries should be required. + var attempts = (ancmVersion == AncmVersion.AspNetCoreModule) ? 2 : 1; - using (var deployers = new DisposableList()) + return Retry(async () => { - var deploymentResults = new List(); + const int numApps = 10; - // Deploy all apps - for (var i = 0; i < numApps; i++) + using (var deployers = new DisposableList()) { - var deploymentParameters = _fixture.GetBaseDeploymentParameters(hostingModel: IntegrationTesting.HostingModel.OutOfProcess); - deploymentParameters.AncmVersion = ancmVersion; + var deploymentResults = new List(); - var deployer = CreateDeployer(deploymentParameters); - deployers.Add(deployer); - deploymentResults.Add(await deployer.DeployAsync()); + // Deploy all apps + for (var i = 0; i < numApps; i++) + { + var deploymentParameters = _fixture.GetBaseDeploymentParameters(hostingModel: IntegrationTesting.HostingModel.OutOfProcess); + deploymentParameters.AncmVersion = ancmVersion; + + 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); + } } + }, + attempts: attempts); + } - // 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) + private async Task Retry(Func func, int attempts) + { + var exceptions = new List(); + + for (var attempt = 0; attempt < attempts; attempt++) + { + try { - requestTasks.Add(deploymentResult.HttpClient.GetAsync("/HelloWorld")); + await func(); + return; } - - // Verify all apps started and return expected response - foreach (var requestTask in requestTasks) + catch (Exception e) { - var response = await requestTask; - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - var responseText = await response.Content.ReadAsStringAsync(); - Assert.Equal("Hello World", responseText); + exceptions.Add(e); } } + + throw new AggregateException(exceptions); } } } From 58df4ef34df0ccab5768c4abe457ce78a6a05d7d Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 5 Sep 2018 16:47:11 -0700 Subject: [PATCH 11/18] AppVeyor: Reset dynamic port range to Windows default (#1368) - Prevents conflicts between ANCM random ports and Windows dynamic ports - Addresses #1269 --- .appveyor.yml | 6 ++++++ IISIntegration.sln | 1 + 2 files changed, 7 insertions(+) diff --git a/.appveyor.yml b/.appveyor.yml index f46dcc270f..2bc93c45a8 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -1,4 +1,10 @@ init: + # Reset dynamic port range from AppVeyor setting (1025-65535) to Windows default (49152-65535), + # to prevent conflicts with ANCM random ports (1025-48000). + - netsh int ipv4 show dynamicport tcp + - netsh int ipv4 set dynamic tcp start=49152 num=16384 + - netsh int ipv4 show dynamicport tcp + - git config --global core.autocrlf true branches: only: diff --git a/IISIntegration.sln b/IISIntegration.sln index 549f17f75b..b47fed50fa 100644 --- a/IISIntegration.sln +++ b/IISIntegration.sln @@ -10,6 +10,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{04B1EDB6-E96 EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{0EF45656-B25D-40D8-959C-726EAF185E60}" ProjectSection(SolutionItems) = preProject + .appveyor.yml = .appveyor.yml .editorconfig = .editorconfig Directory.Build.props = Directory.Build.props Directory.Build.targets = Directory.Build.targets From 613fbcc349a6e2f43aa65e28722e9054d41c04aa Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 5 Sep 2018 16:47:51 -0700 Subject: [PATCH 12/18] Enable code analysis on shim project (#1361) --- build/Build.Settings | 5 + .../AspNetCore/AppOfflineApplication.cpp | 9 +- .../AspNetCore/AppOfflineApplication.h | 2 +- .../AspNetCore/AppOfflineHandler.cpp | 4 +- .../AspNetCore/AppOfflineHandler.h | 4 +- .../AspNetCore/ApplicationFactory.h | 9 +- .../AspNetCore/AspNetCore.vcxproj | 4 +- .../AspNetCore/HandlerResolver.cpp | 25 +- .../AspNetCore/HandlerResolver.h | 12 +- .../PollingAppOfflineApplication.cpp | 4 +- .../AspNetCore/PollingAppOfflineApplication.h | 4 +- .../AspNetCore/ServerErrorApplication.h | 7 +- .../AspNetCore/ServerErrorHandler.h | 6 +- .../AspNetCore/ShimOptions.h | 12 +- .../AspNetCore/applicationinfo.cpp | 17 +- .../AspNetCore/applicationinfo.h | 14 +- .../AspNetCore/applicationmanager.cpp | 3 - .../AspNetCore/applicationmanager.h | 33 +- src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp | 71 +-- .../AspNetCore/globalmodule.cpp | 16 +- .../AspNetCore/globalmodule.h | 18 +- .../AspNetCore/proxymodule.cpp | 37 +- .../AspNetCore/proxymodule.h | 24 +- src/AspNetCoreModuleV2/CommonLib/EventLog.cpp | 2 +- src/AspNetCoreModuleV2/CommonLib/EventLog.h | 24 +- .../CommonLib/FileOutputManager.cpp | 2 +- .../CommonLib/HandleWrapper.h | 26 +- .../CommonLib/IOutputManager.h | 2 +- .../CommonLib/ModuleHelpers.h | 21 +- .../CommonLib/NonCopyable.h | 4 +- .../CommonLib/ResultException.h | 19 + .../CommonLib/StdWrapper.cpp | 16 +- .../CommonLib/StringHelpers.h | 30 +- .../CommonLib/WebConfigConfigurationSource.h | 4 +- .../CommonLib/application.h | 12 +- src/AspNetCoreModuleV2/CommonLib/exceptions.h | 51 ++- src/AspNetCoreModuleV2/CommonLib/fx_ver.h | 14 +- .../CommonLib/hostfxroptions.h | 8 +- .../CommonLib/iapplication.h | 2 +- .../CommonLib/requesthandler.h | 4 +- src/AspNetCoreModuleV2/DefaultRules.ruleset | 432 ++++++++++++++++++ src/AspNetCoreModuleV2/IISLib/acache.cpp | 7 +- src/AspNetCoreModuleV2/IISLib/buffer.h | 5 + src/AspNetCoreModuleV2/IISLib/multisz.h | 7 +- src/AspNetCoreModuleV2/IISLib/percpu.h | 7 +- src/AspNetCoreModuleV2/IISLib/stringa.h | 6 + src/AspNetCoreModuleV2/IISLib/stringu.cpp | 22 +- src/AspNetCoreModuleV2/IISLib/stringu.h | 8 +- .../managedexports.cpp | 2 +- .../AppOfflineTrackingApplication.cpp | 2 +- 50 files changed, 793 insertions(+), 286 deletions(-) create mode 100644 src/AspNetCoreModuleV2/CommonLib/ResultException.h create mode 100644 src/AspNetCoreModuleV2/DefaultRules.ruleset diff --git a/build/Build.Settings b/build/Build.Settings index 9daccf4aaa..d60b07c269 100644 --- a/build/Build.Settings +++ b/build/Build.Settings @@ -20,6 +20,11 @@ false true + ..\DefaultRules.ruleset + false + + true + $(RunCodeAnalysis) diff --git a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp index 49b2b07993..abae1963d5 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp @@ -11,7 +11,8 @@ HRESULT AppOfflineApplication::CreateHandler(IHttpContext* pHttpContext, IREQUES { try { - *pRequestHandler = new AppOfflineHandler(pHttpContext, m_strAppOfflineContent); + auto handler = std::make_unique(*pHttpContext, m_strAppOfflineContent); + *pRequestHandler = handler.release(); } CATCH_RETURN(); @@ -44,9 +45,9 @@ HRESULT AppOfflineApplication::OnAppOfflineFound() if (li.LowPart > 0) { DWORD bytesRead = 0; - std::string pszBuff(li.LowPart + 1, '\0'); + std::string pszBuff(static_cast(li.LowPart) + 1, '\0'); - RETURN_LAST_ERROR_IF(!ReadFile(handle, pszBuff.data(), li.LowPart, &bytesRead, NULL)); + RETURN_LAST_ERROR_IF(!ReadFile(handle, pszBuff.data(), li.LowPart, &bytesRead, nullptr)); pszBuff.resize(bytesRead); m_strAppOfflineContent = pszBuff; @@ -55,7 +56,7 @@ HRESULT AppOfflineApplication::OnAppOfflineFound() return S_OK; } -bool AppOfflineApplication::ShouldBeStarted(IHttpApplication& pApplication) +bool AppOfflineApplication::ShouldBeStarted(const IHttpApplication& pApplication) { return FileExists(GetAppOfflineLocation(pApplication)); } diff --git a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.h b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.h index 71f79e5294..14399edaaa 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.h +++ b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.h @@ -20,7 +20,7 @@ public: HRESULT OnAppOfflineFound() override; - static bool ShouldBeStarted(IHttpApplication& pApplication); + static bool ShouldBeStarted(const IHttpApplication& pApplication); private: std::string m_strAppOfflineContent; diff --git a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.cpp b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.cpp index 3efeb74463..f5098f71f6 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.cpp @@ -7,8 +7,8 @@ REQUEST_NOTIFICATION_STATUS AppOfflineHandler::OnExecuteRequestHandler() { - HTTP_DATA_CHUNK DataChunk; - IHttpResponse* pResponse = m_pContext->GetResponse(); + HTTP_DATA_CHUNK DataChunk {}; + auto pResponse = m_pContext.GetResponse(); DBG_ASSERT(pResponse); diff --git a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.h b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.h index 784a470915..e0ffd5d7cc 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.h +++ b/src/AspNetCoreModuleV2/AspNetCore/AppOfflineHandler.h @@ -9,7 +9,7 @@ class AppOfflineHandler: public REQUEST_HANDLER { public: - AppOfflineHandler(IHttpContext* pContext, const std::string appOfflineContent) + AppOfflineHandler(IHttpContext& pContext, const std::string appOfflineContent) : m_pContext(pContext), m_strAppOfflineContent(appOfflineContent) { @@ -18,6 +18,6 @@ public: REQUEST_NOTIFICATION_STATUS OnExecuteRequestHandler() override; private: - IHttpContext* m_pContext; + IHttpContext& m_pContext; std::string m_strAppOfflineContent; }; diff --git a/src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h b/src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h index 380b0de6cc..9d310438ef 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h +++ b/src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h @@ -5,6 +5,7 @@ #include #include +#include #include "iapplication.h" #include "HandleWrapper.h" @@ -21,9 +22,9 @@ HRESULT class ApplicationFactory { public: - ApplicationFactory(HMODULE hRequestHandlerDll, std::wstring location, PFN_ASPNETCORE_CREATE_APPLICATION pfnAspNetCoreCreateApplication): + ApplicationFactory(HMODULE hRequestHandlerDll, std::wstring location, PFN_ASPNETCORE_CREATE_APPLICATION pfnAspNetCoreCreateApplication) noexcept: m_pfnAspNetCoreCreateApplication(pfnAspNetCoreCreateApplication), - m_location(location), + m_location(std::move(location)), m_hRequestHandlerDll(hRequestHandlerDll) { } @@ -31,10 +32,10 @@ public: HRESULT Execute( _In_ IHttpServer *pServer, _In_ IHttpApplication *pHttpApplication, - _Out_ IAPPLICATION **pApplication) const + _Outptr_ IAPPLICATION **pApplication) const noexcept { std::array parameters { - {"InProcessExeLocation", reinterpret_cast(m_location.data())} + {"InProcessExeLocation", m_location.data()} }; return m_pfnAspNetCoreCreateApplication(pServer, pHttpApplication, parameters.data(), static_cast(parameters.size()), pApplication); } diff --git a/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj b/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj index d700f7ed1b..b71ab41c83 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj +++ b/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj @@ -1,6 +1,5 @@ - + - Debug @@ -56,6 +55,7 @@ Unicode + diff --git a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp index bd7dacfb34..4c98e60f11 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp @@ -14,11 +14,12 @@ #include "resources.h" #include "ConfigurationLoadException.h" #include "WebConfigConfigurationSource.h" +#include "ModuleHelpers.h" const PCWSTR HandlerResolver::s_pwzAspnetcoreInProcessRequestHandlerName = L"aspnetcorev2_inprocess.dll"; const PCWSTR HandlerResolver::s_pwzAspnetcoreOutOfProcessRequestHandlerName = L"aspnetcorev2_outofprocess.dll"; -HandlerResolver::HandlerResolver(HMODULE hModule, IHttpServer &pServer) +HandlerResolver::HandlerResolver(HMODULE hModule, const IHttpServer &pServer) : m_hModule(hModule), m_pServer(pServer), m_loadedApplicationHostingModel(HOSTING_UNKNOWN) @@ -27,11 +28,11 @@ HandlerResolver::HandlerResolver(HMODULE hModule, IHttpServer &pServer) } HRESULT -HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, ShimOptions& pConfiguration, std::unique_ptr& pApplicationFactory) +HandlerResolver::LoadRequestHandlerAssembly(const IHttpApplication &pApplication, const ShimOptions& pConfiguration, std::unique_ptr& pApplicationFactory) { - HRESULT hr; - PCWSTR pstrHandlerDllName; - bool preventUnload; + HRESULT hr = S_OK; + PCWSTR pstrHandlerDllName = nullptr; + bool preventUnload = false; if (pConfiguration.QueryHostingModel() == APP_HOSTING_MODEL::HOSTING_IN_PROCESS) { preventUnload = false; @@ -76,7 +77,7 @@ HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, Shim hr = FindNativeAssemblyFromHostfxr(*options.get(), pstrHandlerDllName, handlerDllPath); outputManager->Stop(); - if (FAILED(hr) && m_hHostFxrDll != NULL) + if (FAILED(hr) && m_hHostFxrDll != nullptr) { STRA content; STRU struStdMsg; @@ -119,7 +120,7 @@ HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, Shim RETURN_LAST_ERROR_IF_NULL(hRequestHandlerDll); } - auto pfnAspNetCoreCreateApplication = reinterpret_cast(GetProcAddress(hRequestHandlerDll, "CreateApplication")); + auto pfnAspNetCoreCreateApplication = ModuleHelpers::GetKnownProcAddress(hRequestHandlerDll, "CreateApplication"); RETURN_LAST_ERROR_IF_NULL(pfnAspNetCoreCreateApplication); pApplicationFactory = std::make_unique(hRequestHandlerDll.release(), location, pfnAspNetCoreCreateApplication); @@ -127,7 +128,7 @@ HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, Shim } HRESULT -HandlerResolver::GetApplicationFactory(IHttpApplication &pApplication, std::unique_ptr& pApplicationFactory) +HandlerResolver::GetApplicationFactory(const IHttpApplication &pApplication, std::unique_ptr& pApplicationFactory) { try { @@ -189,7 +190,7 @@ void HandlerResolver::ResetHostingModel() HRESULT HandlerResolver::FindNativeAssemblyFromGlobalLocation( - ShimOptions& pConfiguration, + const ShimOptions& pConfiguration, PCWSTR pstrHandlerDllName, std::wstring& handlerDllPath ) @@ -225,20 +226,20 @@ HandlerResolver::FindNativeAssemblyFromGlobalLocation( // HRESULT HandlerResolver::FindNativeAssemblyFromHostfxr( - HOSTFXR_OPTIONS& hostfxrOptions, + const HOSTFXR_OPTIONS& hostfxrOptions, PCWSTR libraryName, std::wstring& handlerDllPath ) { std::wstring struNativeSearchPaths; - size_t intIndex; + size_t intIndex = 0; size_t intPrevIndex = 0; DWORD dwBufferSize = s_initialGetNativeSearchDirectoriesBufferSize; DWORD dwRequiredBufferSize = 0; RETURN_LAST_ERROR_IF_NULL(m_hHostFxrDll = LoadLibraryW(hostfxrOptions.GetHostFxrLocation().c_str())); - auto pFnHostFxrSearchDirectories = reinterpret_cast(GetProcAddress(m_hHostFxrDll, "hostfxr_get_native_search_directories")); + const auto pFnHostFxrSearchDirectories = ModuleHelpers::GetKnownProcAddress(m_hHostFxrDll, "hostfxr_get_native_search_directories"); if (pFnHostFxrSearchDirectories == nullptr) { EventLog::Error( diff --git a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h index be927693ab..7b230a928f 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h +++ b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h @@ -13,17 +13,17 @@ class HandlerResolver { public: - HandlerResolver(HMODULE hModule, IHttpServer &pServer); - HRESULT GetApplicationFactory(IHttpApplication &pApplication, std::unique_ptr& pApplicationFactory); + HandlerResolver(HMODULE hModule, const IHttpServer &pServer); + HRESULT GetApplicationFactory(const IHttpApplication &pApplication, std::unique_ptr& pApplicationFactory); void ResetHostingModel(); private: - HRESULT LoadRequestHandlerAssembly(IHttpApplication &pApplication, ShimOptions& pConfiguration, std::unique_ptr& pApplicationFactory); - HRESULT FindNativeAssemblyFromGlobalLocation(ShimOptions& pConfiguration, PCWSTR libraryName, std::wstring& handlerDllPath); - HRESULT FindNativeAssemblyFromHostfxr(HOSTFXR_OPTIONS& hostfxrOptions, PCWSTR libraryName, std::wstring& handlerDllPath); + HRESULT LoadRequestHandlerAssembly(const IHttpApplication &pApplication, const ShimOptions& pConfiguration, std::unique_ptr& pApplicationFactory); + HRESULT FindNativeAssemblyFromGlobalLocation(const ShimOptions& pConfiguration, PCWSTR libraryName, std::wstring& handlerDllPath); + HRESULT FindNativeAssemblyFromHostfxr(const HOSTFXR_OPTIONS& hostfxrOptions, PCWSTR libraryName, std::wstring& handlerDllPath); HMODULE m_hModule; - IHttpServer &m_pServer; + const IHttpServer &m_pServer; SRWLOCK m_requestHandlerLoadLock {}; std::wstring m_loadedApplicationId; diff --git a/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.cpp b/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.cpp index 1270319e8b..382950fde4 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.cpp @@ -44,12 +44,12 @@ PollingAppOfflineApplication::CheckAppOffline() } -std::filesystem::path PollingAppOfflineApplication::GetAppOfflineLocation(IHttpApplication& pApplication) +std::filesystem::path PollingAppOfflineApplication::GetAppOfflineLocation(const IHttpApplication& pApplication) { return std::filesystem::path(pApplication.GetApplicationPhysicalPath()) / "app_offline.htm"; } -bool PollingAppOfflineApplication::FileExists(const std::filesystem::path& path) +bool PollingAppOfflineApplication::FileExists(const std::filesystem::path& path) noexcept { std::error_code ec; return is_regular_file(path, ec) || ec.value() == ERROR_SHARING_VIOLATION; diff --git a/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.h b/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.h index 193a63e639..eadd0da3b8 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.h +++ b/src/AspNetCoreModuleV2/AspNetCore/PollingAppOfflineApplication.h @@ -31,8 +31,8 @@ public: protected: std::filesystem::path m_appOfflineLocation; - static std::filesystem::path GetAppOfflineLocation(IHttpApplication& pApplication); - static bool FileExists(const std::filesystem::path& path); + static std::filesystem::path GetAppOfflineLocation(const IHttpApplication& pApplication); + static bool FileExists(const std::filesystem::path& path) noexcept; private: static const int c_appOfflineRefreshIntervalMS = 200; std::string m_strAppOfflineContent; diff --git a/src/AspNetCoreModuleV2/AspNetCore/ServerErrorApplication.h b/src/AspNetCoreModuleV2/AspNetCore/ServerErrorApplication.h index 2162075950..86a9a1a0e3 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/ServerErrorApplication.h +++ b/src/AspNetCoreModuleV2/AspNetCore/ServerErrorApplication.h @@ -17,13 +17,14 @@ public: ~ServerErrorApplication() = default; - HRESULT CreateHandler(IHttpContext * pHttpContext, IREQUEST_HANDLER ** pRequestHandler) override + HRESULT CreateHandler(IHttpContext *pHttpContext, IREQUEST_HANDLER ** pRequestHandler) override { - *pRequestHandler = new ServerErrorHandler(pHttpContext, m_HR); + auto handler = std::make_unique(*pHttpContext, m_HR); + *pRequestHandler = handler.release(); return S_OK; } - HRESULT OnAppOfflineFound() override { return S_OK; } + HRESULT OnAppOfflineFound() noexcept override { return S_OK; } private: HRESULT m_HR; }; diff --git a/src/AspNetCoreModuleV2/AspNetCore/ServerErrorHandler.h b/src/AspNetCoreModuleV2/AspNetCore/ServerErrorHandler.h index b3d917039d..10cf3ee049 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/ServerErrorHandler.h +++ b/src/AspNetCoreModuleV2/AspNetCore/ServerErrorHandler.h @@ -7,17 +7,17 @@ class ServerErrorHandler : public REQUEST_HANDLER { public: - ServerErrorHandler(IHttpContext* pContext, HRESULT hr) : m_pContext(pContext), m_HR(hr) + ServerErrorHandler(IHttpContext &pContext, HRESULT hr) : m_pContext(pContext), m_HR(hr) { } REQUEST_NOTIFICATION_STATUS OnExecuteRequestHandler() override { - m_pContext->GetResponse()->SetStatus(500, "Internal Server Error", 0, m_HR); + m_pContext.GetResponse()->SetStatus(500, "Internal Server Error", 0, m_HR); return RQ_NOTIFICATION_FINISH_REQUEST; } private: - IHttpContext * m_pContext; + IHttpContext &m_pContext; HRESULT m_HR; }; diff --git a/src/AspNetCoreModuleV2/AspNetCore/ShimOptions.h b/src/AspNetCoreModuleV2/AspNetCore/ShimOptions.h index f987e864ba..4b934ad9b4 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/ShimOptions.h +++ b/src/AspNetCoreModuleV2/AspNetCore/ShimOptions.h @@ -18,37 +18,37 @@ class ShimOptions: NonCopyable { public: const std::wstring& - QueryProcessPath() const + QueryProcessPath() const noexcept { return m_strProcessPath; } const std::wstring& - QueryArguments() const + QueryArguments() const noexcept { return m_strArguments; } APP_HOSTING_MODEL - QueryHostingModel() const + QueryHostingModel() const noexcept { return m_hostingModel; } const std::wstring& - QueryHandlerVersion() const + QueryHandlerVersion() const noexcept { return m_strHandlerVersion; } BOOL - QueryStdoutLogEnabled() const + QueryStdoutLogEnabled() const noexcept { return m_fStdoutLogEnabled; } const std::wstring& - QueryStdoutLogFile() const + QueryStdoutLogFile() const noexcept { return m_struStdoutLogFile; } diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp index 9f2b20cf8e..fb558e52d0 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp @@ -13,14 +13,9 @@ #include "ServerErrorApplication.h" #include "AppOfflineApplication.h" -APPLICATION_INFO::~APPLICATION_INFO() -{ - ShutDownApplication(/* fServerInitiated */ false); -} - HRESULT APPLICATION_INFO::GetOrCreateApplication( - IHttpContext *pHttpContext, + IHttpContext& pHttpContext, std::unique_ptr& pApplication ) { @@ -28,7 +23,7 @@ APPLICATION_INFO::GetOrCreateApplication( SRWExclusiveLock lock(m_applicationLock); - auto& httpApplication = *pHttpContext->GetApplication(); + auto& httpApplication = *pHttpContext.GetApplication(); if (m_pApplication != nullptr) { @@ -51,7 +46,10 @@ APPLICATION_INFO::GetOrCreateApplication( if (AppOfflineApplication::ShouldBeStarted(httpApplication)) { LOG_INFO(L"Detected app_offline file, creating polling application"); + #pragma warning( push ) + #pragma warning ( disable : 26409 ) // Disable "Avoid using new", using custom deleter here m_pApplication.reset(new AppOfflineApplication(httpApplication)); + #pragma warning( pop ) } else { @@ -77,8 +75,11 @@ Finished: ASPNETCORE_EVENT_ADD_APPLICATION_ERROR_MSG, httpApplication.GetApplicationId(), hr); - + + #pragma warning( push ) + #pragma warning ( disable : 26409 ) // Disable "Avoid using new", using custom deleter here m_pApplication.reset(new ServerErrorApplication(httpApplication, hr)); + #pragma warning( pop ) } if (m_pApplication) diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h index 94f2718e08..998cdc5ae1 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h @@ -12,7 +12,7 @@ extern BOOL g_fRecycleProcessCalled; -class APPLICATION_INFO +class APPLICATION_INFO: NonCopyable { public: @@ -29,16 +29,16 @@ public: InitializeSRWLock(&m_applicationLock); } - ~APPLICATION_INFO(); + ~APPLICATION_INFO() = default; - std::wstring& - QueryApplicationInfoKey() + const std::wstring& + QueryApplicationInfoKey() const noexcept { return m_strInfoKey; } - std::wstring& - QueryConfigPath() + const std::wstring& + QueryConfigPath() const noexcept { return m_strConfigPath; } @@ -48,7 +48,7 @@ public: HRESULT GetOrCreateApplication( - IHttpContext *pHttpContext, + IHttpContext& pHttpContext, std::unique_ptr& pApplication ); diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp index 7b9df85a43..52d9730ec1 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp @@ -11,9 +11,6 @@ extern BOOL g_fInShutdown; -// The application manager is a singleton across ANCM. -APPLICATION_MANAGER* APPLICATION_MANAGER::sm_pApplicationManager = NULL; - // // Retrieves the application info from the application manager // Will create the application info if it isn't initalized diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index ab3fbe9746..a9fdde6a5f 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -19,25 +19,6 @@ class APPLICATION_MANAGER { public: - static - APPLICATION_MANAGER* - GetInstance() - { - assert(sm_pApplicationManager); - return sm_pApplicationManager; - } - - static - VOID - Cleanup() - { - if(sm_pApplicationManager != NULL) - { - delete sm_pApplicationManager; - sm_pApplicationManager = NULL; - } - } - HRESULT GetOrCreateApplicationInfo( _In_ IHttpContext& pHttpContext, @@ -51,16 +32,7 @@ public: VOID ShutDown(); - - static HRESULT StaticInitialize(HMODULE hModule, IHttpServer& pHttpServer) - { - assert(!sm_pApplicationManager); - sm_pApplicationManager = new APPLICATION_MANAGER(hModule, pHttpServer); - return S_OK; - } - - -private: + APPLICATION_MANAGER(HMODULE hModule, IHttpServer& pHttpServer) : m_pApplicationInfoHash(NULL), m_fDebugInitialize(FALSE), @@ -70,8 +42,9 @@ private: InitializeSRWLock(&m_srwLock); } +private: + std::unordered_map> m_pApplicationInfoHash; - static APPLICATION_MANAGER *sm_pApplicationManager; SRWLOCK m_srwLock {}; BOOL m_fDebugInitialize; IHttpServer &m_pHttpServer; diff --git a/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp b/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp index b20ca03b97..1f34270aee 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp @@ -13,7 +13,7 @@ DECLARE_DEBUG_PRINT_OBJECT("aspnetcorev2.dll"); -HANDLE g_hEventLog = NULL; +HANDLE g_hEventLog = nullptr; BOOL g_fRecycleProcessCalled = FALSE; BOOL g_fInShutdown = FALSE; HINSTANCE g_hServerModule; @@ -21,14 +21,14 @@ HINSTANCE g_hServerModule; VOID StaticCleanup() { - APPLICATION_MANAGER::Cleanup(); - if (g_hEventLog != NULL) + if (g_hEventLog != nullptr) { DeregisterEventSource(g_hEventLog); - g_hEventLog = NULL; + g_hEventLog = nullptr; } DebugStop(); + ALLOC_CACHE_HANDLER::StaticTerminate(); } BOOL WINAPI DllMain(HMODULE hModule, @@ -41,11 +41,14 @@ BOOL WINAPI DllMain(HMODULE hModule, switch (ul_reason_for_call) { case DLL_PROCESS_ATTACH: + + ALLOC_CACHE_HANDLER::StaticInitialize(); g_hServerModule = hModule; DisableThreadLibraryCalls(hModule); DebugInitialize(hModule); break; case DLL_PROCESS_DETACH: + // IIS can cause dll detach to occur before we receive global notifications // For example, when we switch the bitness of the worker process, // this is a bug in IIS. To try to avoid AVs, we will set a global flag @@ -85,21 +88,18 @@ HRESULT --*/ { - HRESULT hr = S_OK; - HKEY hKey; + HKEY hKey {}; BOOL fDisableANCM = FALSE; - ASPNET_CORE_PROXY_MODULE_FACTORY * pFactory = NULL; - ASPNET_CORE_GLOBAL_MODULE * pGlobalModule = NULL; UNREFERENCED_PARAMETER(dwServerVersion); if (pHttpServer->IsCommandLineLaunch()) { - g_hEventLog = RegisterEventSource(NULL, ASPNETCORE_IISEXPRESS_EVENT_PROVIDER); + g_hEventLog = RegisterEventSource(nullptr, ASPNETCORE_IISEXPRESS_EVENT_PROVIDER); } else { - g_hEventLog = RegisterEventSource(NULL, ASPNETCORE_EVENT_PROVIDER); + g_hEventLog = RegisterEventSource(nullptr, ASPNETCORE_EVENT_PROVIDER); } // check whether the feature is disabled due to security reason @@ -109,14 +109,14 @@ HRESULT KEY_READ, &hKey) == NO_ERROR) { - DWORD dwType; - DWORD dwData; + DWORD dwType = 0; + DWORD dwData = 0; DWORD cbData; cbData = sizeof(dwData); if ((RegQueryValueEx(hKey, L"DisableANCM", - NULL, + nullptr, &dwType, (LPBYTE)&dwData, &cbData) == NO_ERROR) && @@ -136,7 +136,7 @@ HRESULT ASPNETCORE_EVENT_MODULE_DISABLED_MSG); // this will return 500 error to client // as we did not register the module - goto Finished; + return S_OK; } // @@ -144,43 +144,22 @@ HRESULT // The ASPNET_CORE_PROXY_MODULE_FACTORY::Terminate method will clean any // static object initialized. // - pFactory = new ASPNET_CORE_PROXY_MODULE_FACTORY; + + auto applicationManager = std::make_shared(g_hServerModule, *pHttpServer); + auto moduleFactory = std::make_unique(applicationManager); - FINISHED_IF_FAILED(pModuleInfo->SetRequestNotifications( - pFactory, + RETURN_IF_FAILED(pModuleInfo->SetRequestNotifications( + moduleFactory.release(), RQ_EXECUTE_REQUEST_HANDLER, 0)); +; + auto pGlobalModule = std::make_unique(std::move(applicationManager)); - pFactory = NULL; - - FINISHED_IF_FAILED(APPLICATION_MANAGER::StaticInitialize(g_hServerModule, *pHttpServer)); - - pGlobalModule = NULL; - - pGlobalModule = new ASPNET_CORE_GLOBAL_MODULE(APPLICATION_MANAGER::GetInstance()); - - FINISHED_IF_FAILED(pModuleInfo->SetGlobalNotifications( - pGlobalModule, + RETURN_IF_FAILED(pModuleInfo->SetGlobalNotifications( + pGlobalModule.release(), GL_CONFIGURATION_CHANGE | // Configuration change trigers IIS application stop GL_STOP_LISTENING)); // worker process stop or recycle - - pGlobalModule = NULL; - - FINISHED_IF_FAILED(ALLOC_CACHE_HANDLER::StaticInitialize()); - -Finished: - if (pGlobalModule != NULL) - { - delete pGlobalModule; - pGlobalModule = NULL; - } - - if (pFactory != NULL) - { - pFactory->Terminate(); - pFactory = NULL; - } - - return hr; + + return S_OK; } CATCH_RETURN() diff --git a/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index 755b1ffc79..abe08964cd 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -5,10 +5,9 @@ extern BOOL g_fInShutdown; -ASPNET_CORE_GLOBAL_MODULE::ASPNET_CORE_GLOBAL_MODULE( - APPLICATION_MANAGER* pApplicationManager) +ASPNET_CORE_GLOBAL_MODULE::ASPNET_CORE_GLOBAL_MODULE(std::shared_ptr pApplicationManager) noexcept + :m_pApplicationManager(std::move(pApplicationManager)) { - m_pApplicationManager = pApplicationManager; } // @@ -30,11 +29,8 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalStopListening( return GL_NOTIFICATION_CONTINUE; } - DBG_ASSERT(m_pApplicationManager); - // we should let application manager to shutdown all allication - // and dereference it as some requests may still reference to application manager m_pApplicationManager->ShutDown(); - m_pApplicationManager = NULL; + m_pApplicationManager = nullptr; // Return processing to the pipeline. return GL_NOTIFICATION_CONTINUE; @@ -59,13 +55,13 @@ ASPNET_CORE_GLOBAL_MODULE::OnGlobalConfigurationChange( LOG_INFOF(L"ASPNET_CORE_GLOBAL_MODULE::OnGlobalConfigurationChange '%ls'", pwszChangePath); // Test for an error. - if (NULL != pwszChangePath && + if (nullptr != pwszChangePath && _wcsicmp(pwszChangePath, L"MACHINE") != 0 && _wcsicmp(pwszChangePath, L"MACHINE/WEBROOT") != 0) { - if (m_pApplicationManager != NULL) + if (m_pApplicationManager) { - m_pApplicationManager->RecycleApplicationFromManager(pwszChangePath); + m_pApplicationManager->RecycleApplicationFromManager(pwszChangePath); } } diff --git a/src/AspNetCoreModuleV2/AspNetCore/globalmodule.h b/src/AspNetCoreModuleV2/AspNetCore/globalmodule.h index 13b6f86bea..80f047e08d 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/globalmodule.h +++ b/src/AspNetCoreModuleV2/AspNetCore/globalmodule.h @@ -5,20 +5,18 @@ #include "applicationmanager.h" -class ASPNET_CORE_GLOBAL_MODULE : public CGlobalModule +class ASPNET_CORE_GLOBAL_MODULE : NonCopyable, public CGlobalModule { public: ASPNET_CORE_GLOBAL_MODULE( - APPLICATION_MANAGER* pApplicationManager - ); + std::shared_ptr pApplicationManager + ) noexcept; - ~ASPNET_CORE_GLOBAL_MODULE() - { - } + virtual ~ASPNET_CORE_GLOBAL_MODULE() = default; - VOID Terminate() + VOID Terminate() override { LOG_INFO(L"ASPNET_CORE_GLOBAL_MODULE::Terminate"); // Remove the class from memory. @@ -28,13 +26,13 @@ public: GLOBAL_NOTIFICATION_STATUS OnGlobalStopListening( _In_ IGlobalStopListeningProvider * pProvider - ); + ) override; GLOBAL_NOTIFICATION_STATUS OnGlobalConfigurationChange( _In_ IGlobalConfigurationChangeProvider * pProvider - ); + ) override; private: - APPLICATION_MANAGER * m_pApplicationManager; + std::shared_ptr m_pApplicationManager; }; diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp index b4ed0fbaad..c93c7c7602 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp @@ -5,31 +5,40 @@ #include "applicationmanager.h" #include "applicationinfo.h" -#include "acache.h" #include "exceptions.h" extern BOOL g_fInShutdown; __override + +ASPNET_CORE_PROXY_MODULE_FACTORY::ASPNET_CORE_PROXY_MODULE_FACTORY(std::shared_ptr applicationManager) noexcept + :m_pApplicationManager(std::move(applicationManager)) +{ +} + HRESULT ASPNET_CORE_PROXY_MODULE_FACTORY::GetHttpModule( CHttpModule ** ppModule, IModuleAllocator * pAllocator ) { - try + + #pragma warning( push ) + #pragma warning ( disable : 26409 ) // Disable "Avoid using new" + *ppModule = new (pAllocator) ASPNET_CORE_PROXY_MODULE(m_pApplicationManager); + #pragma warning( push ) + if (*ppModule == nullptr) { - *ppModule = THROW_IF_NULL_ALLOC(new (pAllocator) ASPNET_CORE_PROXY_MODULE());; - return S_OK; + return E_OUTOFMEMORY; } - CATCH_RETURN(); + return S_OK; } __override VOID ASPNET_CORE_PROXY_MODULE_FACTORY::Terminate( VOID -) +) noexcept /*++ Routine description: @@ -46,12 +55,13 @@ Return value: --*/ { - ALLOC_CACHE_HANDLER::StaticTerminate(); delete this; } -ASPNET_CORE_PROXY_MODULE::ASPNET_CORE_PROXY_MODULE( -) : m_pApplicationInfo(nullptr), m_pHandler(nullptr) +ASPNET_CORE_PROXY_MODULE::ASPNET_CORE_PROXY_MODULE(std::shared_ptr applicationManager) noexcept + : m_pApplicationManager(std::move(applicationManager)), + m_pApplicationInfo(nullptr), + m_pHandler(nullptr) { } @@ -64,6 +74,7 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( { HRESULT hr = S_OK; REQUEST_NOTIFICATION_STATUS retVal = RQ_NOTIFICATION_CONTINUE; + try { @@ -72,14 +83,12 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS)); } - auto pApplicationManager = APPLICATION_MANAGER::GetInstance(); - - FINISHED_IF_FAILED(pApplicationManager->GetOrCreateApplicationInfo( + FINISHED_IF_FAILED(m_pApplicationManager->GetOrCreateApplicationInfo( *pHttpContext, m_pApplicationInfo)); std::unique_ptr pApplication; - FINISHED_IF_FAILED(m_pApplicationInfo->GetOrCreateApplication(pHttpContext, pApplication)); + FINISHED_IF_FAILED(m_pApplicationInfo->GetOrCreateApplication(*pHttpContext, pApplication)); IREQUEST_HANDLER* pHandler; // Create RequestHandler and process the request @@ -94,7 +103,7 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( } Finished: - if (LOG_IF_FAILED(hr)) + if (FAILED(LOG_IF_FAILED(hr))) { retVal = RQ_NOTIFICATION_FINISH_REQUEST; if (hr == HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS)) diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h index 06e333d595..7c67712bab 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h @@ -6,14 +6,15 @@ #include #include "applicationinfo.h" #include "irequesthandler.h" +#include "applicationmanager.h" extern HTTP_MODULE_ID g_pModuleId; -class ASPNET_CORE_PROXY_MODULE : public CHttpModule +class ASPNET_CORE_PROXY_MODULE : NonCopyable, public CHttpModule { public: - ASPNET_CORE_PROXY_MODULE(); + ASPNET_CORE_PROXY_MODULE(std::shared_ptr applicationManager) noexcept; ~ASPNET_CORE_PROXY_MODULE() = default; @@ -33,7 +34,7 @@ class ASPNET_CORE_PROXY_MODULE : public CHttpModule OnExecuteRequestHandler( IHttpContext * pHttpContext, IHttpEventProvider * pProvider - ); + ) override; __override REQUEST_NOTIFICATION_STATUS @@ -43,22 +44,29 @@ class ASPNET_CORE_PROXY_MODULE : public CHttpModule BOOL fPostNotification, IHttpEventProvider * pProvider, IHttpCompletionInfo * pCompletionInfo - ); + ) override; private: + std::shared_ptr m_pApplicationManager; std::shared_ptr m_pApplicationInfo; - std::unique_ptr m_pHandler; + std::unique_ptr m_pHandler; }; -class ASPNET_CORE_PROXY_MODULE_FACTORY : public IHttpModuleFactory +class ASPNET_CORE_PROXY_MODULE_FACTORY : NonCopyable, public IHttpModuleFactory { public: + ASPNET_CORE_PROXY_MODULE_FACTORY(std::shared_ptr applicationManager) noexcept; + virtual ~ASPNET_CORE_PROXY_MODULE_FACTORY() = default; + HRESULT GetHttpModule( CHttpModule ** ppModule, IModuleAllocator * pAllocator - ); + ) override; VOID - Terminate(); + Terminate() noexcept override; + + private: + std::shared_ptr m_pApplicationManager; }; diff --git a/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp b/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp index a8b32849ff..c3a3608820 100644 --- a/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp @@ -50,7 +50,7 @@ EventLog::LogEventF( _In_ WORD dwEventInfoType, _In_ DWORD dwEventId, _In_ LPCWSTR pstrMsg, - va_list argsList + _In_ va_list argsList ) { STACK_STRU ( strEventMsg, 256 ); diff --git a/src/AspNetCoreModuleV2/CommonLib/EventLog.h b/src/AspNetCoreModuleV2/CommonLib/EventLog.h index cdc330300f..fa2cde1947 100644 --- a/src/AspNetCoreModuleV2/CommonLib/EventLog.h +++ b/src/AspNetCoreModuleV2/CommonLib/EventLog.h @@ -5,6 +5,18 @@ #include "resources.h" +#define _va_start(ap, x) \ + __pragma(warning(push)) \ + __pragma(warning(disable:26481 26492)) /*Don't use pointer arithmetic. Don't use const_cast to cast away const.*/ \ + va_start(ap, x) \ + __pragma(warning(pop)) + +#define _va_end(args) \ + __pragma(warning(push)) \ + __pragma(warning(disable:26477)) /*Use 'nullptr' rather than 0 or NULL*/ \ + va_end(args) \ + __pragma(warning(pop)) + class EventLog { public: @@ -16,9 +28,9 @@ public: ...) { va_list args; - va_start(args, pstrMsg); + _va_start(args, pstrMsg); LogEventF(EVENTLOG_ERROR_TYPE, dwEventId, pstrMsg, args); - va_end(args); + _va_end(args); } static @@ -29,9 +41,9 @@ public: ...) { va_list args; - va_start(args, pstrMsg); + _va_start(args, pstrMsg); LogEventF(EVENTLOG_INFORMATION_TYPE, dwEventId, pstrMsg, args); - va_end(args); + _va_end(args); } static @@ -42,9 +54,9 @@ public: ...) { va_list args; - va_start(args, pstrMsg); + _va_start(args, pstrMsg); LogEventF(EVENTLOG_WARNING_TYPE, dwEventId, pstrMsg, args); - va_end(args); + _va_end(args); } private: diff --git a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp index 69cc0fe76a..fb95017af8 100644 --- a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp @@ -174,7 +174,7 @@ FileOutputManager::Stop() if (li.LowPart == 0 || li.HighPart > 0) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } dwFilePointer = SetFilePointer(m_hLogFileHandle, 0, NULL, FILE_BEGIN); diff --git a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h index d3afa64551..56da4ae035 100644 --- a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h +++ b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h @@ -10,37 +10,41 @@ struct InvalidHandleTraits { using HandleType = HANDLE; static const HANDLE DefaultHandle; - static void Close(HANDLE handle) { CloseHandle(handle); } + static void Close(HANDLE handle) noexcept { CloseHandle(handle); } }; struct NullHandleTraits { using HandleType = HANDLE; - static constexpr HANDLE DefaultHandle = NULL; - static void Close(HANDLE handle) { CloseHandle(handle); } + static constexpr HANDLE DefaultHandle = nullptr; + static void Close(HANDLE handle) noexcept { CloseHandle(handle); } }; struct FindFileHandleTraits { using HandleType = HANDLE; static const HANDLE DefaultHandle; - static void Close(HANDLE handle) { FindClose(handle); } + static void Close(HANDLE handle) noexcept { FindClose(handle); } }; struct ModuleHandleTraits { using HandleType = HMODULE; - static constexpr HMODULE DefaultHandle = NULL; - static void Close(HMODULE handle) { FreeModule(handle); } + static constexpr HMODULE DefaultHandle = nullptr; + static void Close(HMODULE handle) noexcept { FreeModule(handle); } }; +// Code analysis doesn't like nullptr usages via traits +#pragma warning( push ) +#pragma warning ( disable : 26477 ) // disable Use 'nullptr' rather than 0 or NULL (es.47). + template class HandleWrapper { public: using HandleType = typename traits::HandleType; - HandleWrapper(HandleType handle = traits::DefaultHandle) : m_handle(handle) { } + HandleWrapper(HandleType handle = traits::DefaultHandle) noexcept : m_handle(handle) { } ~HandleWrapper() { if (m_handle != traits::DefaultHandle) @@ -49,15 +53,15 @@ public: } } - operator HandleType() { return m_handle; } - HandleWrapper& operator =(HandleType value) + operator HandleType() noexcept { return m_handle; } + HandleWrapper& operator =(HandleType value) noexcept { DBG_ASSERT(m_handle == traits::DefaultHandle); m_handle = value; return *this; } - HandleType* operator&() { return &m_handle; } + HandleType* operator&() noexcept { return &m_handle; } HandleType release() noexcept { @@ -69,3 +73,5 @@ public: private: HandleType m_handle; }; + +#pragma warning( pop ) diff --git a/src/AspNetCoreModuleV2/CommonLib/IOutputManager.h b/src/AspNetCoreModuleV2/CommonLib/IOutputManager.h index 6b733dd0e6..fec85cea96 100644 --- a/src/AspNetCoreModuleV2/CommonLib/IOutputManager.h +++ b/src/AspNetCoreModuleV2/CommonLib/IOutputManager.h @@ -14,7 +14,7 @@ public: Start() = 0; virtual - ~IOutputManager() {}; + ~IOutputManager() = default; virtual bool diff --git a/src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h b/src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h index 0ef9a7662d..cab86d76ea 100644 --- a/src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h +++ b/src/AspNetCoreModuleV2/CommonLib/ModuleHelpers.h @@ -14,7 +14,26 @@ public: void IncrementCurrentModuleRefCount(HandleWrapper &handle) { WCHAR path[MAX_PATH]; - THROW_LAST_ERROR_IF(!GetModuleFileName(g_hModule, path, sizeof(path))); + +#pragma warning( push ) +#pragma warning ( disable : 26485 ) // Calling WinAPI causes expected array to pointer decay + + THROW_LAST_ERROR_IF(!GetModuleFileName(g_hModule, path, MAX_PATH)); THROW_LAST_ERROR_IF(!GetModuleHandleEx(0, path, &handle)); + +#pragma warning( pop ) + } + + template + static + Func GetKnownProcAddress(HMODULE hModule, LPCSTR lpProcName) { + +#pragma warning( push ) +#pragma warning ( disable : 26490 ) // Disable Don't use reinterpret_cast + auto proc = reinterpret_cast(GetProcAddress(hModule, lpProcName)); +#pragma warning( pop ) + + THROW_LAST_ERROR_IF (!proc); + return proc; } }; diff --git a/src/AspNetCoreModuleV2/CommonLib/NonCopyable.h b/src/AspNetCoreModuleV2/CommonLib/NonCopyable.h index 8dcb282411..11f73903fa 100644 --- a/src/AspNetCoreModuleV2/CommonLib/NonCopyable.h +++ b/src/AspNetCoreModuleV2/CommonLib/NonCopyable.h @@ -6,6 +6,6 @@ class NonCopyable { public: NonCopyable() = default; - NonCopyable(const NonCopyable&) = default; - NonCopyable& operator=(const NonCopyable&) = default; + NonCopyable(const NonCopyable&) = delete; + NonCopyable& operator=(const NonCopyable&) = delete; }; diff --git a/src/AspNetCoreModuleV2/CommonLib/ResultException.h b/src/AspNetCoreModuleV2/CommonLib/ResultException.h new file mode 100644 index 0000000000..e6cb8b4b81 --- /dev/null +++ b/src/AspNetCoreModuleV2/CommonLib/ResultException.h @@ -0,0 +1,19 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#pragma once + +class ResultException: public std::runtime_error +{ +public: + ResultException(HRESULT hr, LOCATION_ARGUMENTS_ONLY) : + runtime_error(format("HRESULT 0x%x returned at " LOCATION_FORMAT, hr, LOCATION_CALL_ONLY)), + m_hr(hr) + { + } + + HRESULT GetResult() const noexcept { return m_hr; } + +private: + HRESULT m_hr; +}; diff --git a/src/AspNetCoreModuleV2/CommonLib/StdWrapper.cpp b/src/AspNetCoreModuleV2/CommonLib/StdWrapper.cpp index dffaa491c5..432e48a163 100644 --- a/src/AspNetCoreModuleV2/CommonLib/StdWrapper.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/StdWrapper.cpp @@ -78,14 +78,14 @@ StdWrapper::StartRedirection() if (fileDescriptor == -1) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } m_redirectedFile = _fdopen(fileDescriptor, "w"); if (m_redirectedFile == nullptr) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } // Set stdout/stderr to the newly created file. @@ -93,13 +93,13 @@ StdWrapper::StartRedirection() if (dup2Result != 0) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } // Removes buffering from the output if (setvbuf(m_stdStream, nullptr, _IONBF, 0) != 0) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } return S_OK; @@ -117,7 +117,7 @@ StdWrapper::StopRedirection() const FILE * file = _fdopen(m_previousFileDescriptor, "w"); if (file == nullptr) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } RETURN_LAST_ERROR_IF(!SetStdHandle(m_stdHandleNumber, reinterpret_cast(_get_osfhandle(m_previousFileDescriptor)))); @@ -131,17 +131,17 @@ StdWrapper::StopRedirection() const const auto dup2Result = _dup2(_fileno(file), _fileno(m_stdStream)); if (dup2Result != 0) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } if (setvbuf(m_stdStream, nullptr, _IONBF, 0) != 0) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } if (fclose(m_redirectedFile) != 0) { - RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); + RETURN_HR(HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); } return S_OK; diff --git a/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h b/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h index 957f5bc3d6..e6cda03ff7 100644 --- a/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h +++ b/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h @@ -15,19 +15,33 @@ template [[nodiscard]] std::wstring format(const std::wstring& format, Args ... args) { - const size_t size = swprintf(nullptr, 0, format.c_str(), args ...) + 1; // Extra char for '\0' - std::unique_ptr formattedBuffer(new wchar_t[size]); - swprintf(formattedBuffer.get(), size, format.c_str(), args ... ); - return std::wstring(formattedBuffer.get(), formattedBuffer.get() + size - 1); + std::wstring result; + if (!format.empty()) + { + const size_t size = swprintf(nullptr, 0, format.c_str(), args ...); // Extra char for '\0' + result.resize(size + 1); + if (swprintf(result.data(), result.size(), format.c_str(), args ... ) == -1) + { + throw std::system_error(std::error_code(errno, std::system_category())); + } + } + return result; } template [[nodiscard]] std::string format(const std::string& format, Args ... args) { - const size_t size = snprintf(nullptr, 0, format.c_str(), args ...) + 1; // Extra char for '\0' - std::unique_ptr formattedBuffer(new char[size]); - snprintf(formattedBuffer.get(), size, format.c_str(), args ... ); - return std::string(formattedBuffer.get(), formattedBuffer.get() + size - 1); + std::string result; + if (!format.empty()) + { + const size_t size = snprintf(nullptr, 0, format.c_str(), args ...); // Extra char for '\0' + result.resize(size + 1); + if (snprintf(result.data(), result.size(), format.c_str(), args ... ) == -1) + { + throw std::system_error(std::error_code(errno, std::system_category())); + } + } + return result; } diff --git a/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSource.h b/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSource.h index ce8dbcdc50..d0c866096b 100644 --- a/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSource.h +++ b/src/AspNetCoreModuleV2/CommonLib/WebConfigConfigurationSource.h @@ -9,7 +9,7 @@ class WebConfigConfigurationSource: public ConfigurationSource { public: - WebConfigConfigurationSource(IAppHostAdminManager *pAdminManager, IHttpApplication &pHttpApplication) + WebConfigConfigurationSource(IAppHostAdminManager *pAdminManager, const IHttpApplication &pHttpApplication) noexcept : m_manager(pAdminManager), m_application(pHttpApplication) { @@ -19,5 +19,5 @@ public: private: CComPtr m_manager; - IHttpApplication &m_application; + const IHttpApplication &m_application; }; diff --git a/src/AspNetCoreModuleV2/CommonLib/application.h b/src/AspNetCoreModuleV2/CommonLib/application.h index e99159d324..c3d8eeb898 100644 --- a/src/AspNetCoreModuleV2/CommonLib/application.h +++ b/src/AspNetCoreModuleV2/CommonLib/application.h @@ -58,7 +58,7 @@ public: } VOID - ReferenceApplication() override + ReferenceApplication() noexcept override { DBG_ASSERT(m_cRefs > 0); @@ -66,7 +66,7 @@ public: } VOID - DereferenceApplication() override + DereferenceApplication() noexcept override { DBG_ASSERT(m_cRefs > 0); @@ -77,25 +77,25 @@ public: } const std::wstring& - QueryApplicationId() const + QueryApplicationId() const noexcept { return m_applicationId; } const std::wstring& - QueryApplicationPhysicalPath() const + QueryApplicationPhysicalPath() const noexcept { return m_applicationPhysicalPath; } const std::wstring& - QueryApplicationVirtualPath() const + QueryApplicationVirtualPath() const noexcept { return m_applicationVirtualPath; } const std::wstring& - QueryConfigPath() const + QueryConfigPath() const noexcept { return m_applicationConfigPath; } diff --git a/src/AspNetCoreModuleV2/CommonLib/exceptions.h b/src/AspNetCoreModuleV2/CommonLib/exceptions.h index 0bf30d1bcc..109f76ce87 100644 --- a/src/AspNetCoreModuleV2/CommonLib/exceptions.h +++ b/src/AspNetCoreModuleV2/CommonLib/exceptions.h @@ -31,20 +31,36 @@ #define OBSERVE_CAUGHT_EXCEPTION() CaughtExceptionHResult(LOCATION_INFO); #define RETURN_CAUGHT_EXCEPTION() return CaughtExceptionHResult(LOCATION_INFO); -#define RETURN_HR(hr) do { HRESULT __hrRet = hr; if (FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); } return __hrRet; } while (0, 0) +#define _CHECK_FAILED(expr) __pragma(warning(push)) \ + __pragma(warning(disable:4127)) /*disable condition is const warning*/ \ + FAILED(expr) \ + __pragma(warning(pop)) + +#define _HR_RET(hr) __pragma(warning(push)) \ + __pragma(warning(disable:26498)) /*disable constexpr warning */ \ + const HRESULT __hrRet = hr; \ + __pragma(warning(pop)) + +#define RETURN_HR(hr) do { _HR_RET(hr); if (_CHECK_FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); } return __hrRet; } while (0, 0) #define RETURN_LAST_ERROR() do { return LogLastError(LOCATION_INFO); } while (0, 0) -#define RETURN_IF_FAILED(hr) do { HRESULT __hrRet = hr; if (FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); return __hrRet; }} while (0, 0) +#define RETURN_IF_FAILED(hr) do { _HR_RET(hr); if (FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); return __hrRet; }} while (0, 0) #define RETURN_LAST_ERROR_IF(condition) do { if (condition) { return LogLastError(LOCATION_INFO); }} while (0, 0) #define RETURN_LAST_ERROR_IF_NULL(ptr) do { if ((ptr) == nullptr) { return LogLastError(LOCATION_INFO); }} while (0, 0) -#define FINISHED(hrr) do { HRESULT __hrRet = hrr; LogHResultFailed(LOCATION_INFO, __hrRet); hr = __hrRet; goto Finished; } while (0, 0) -#define FINISHED_IF_FAILED(hrr) do { HRESULT __hrRet = hrr; if (FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); hr = __hrRet; goto Finished; }} while (0, 0) -#define FINISHED_IF_NULL_ALLOC(ptr) do { if ((ptr) == nullptr) { hr = LogHResultFailed(LOCATION_INFO, E_OUTOFMEMORY); goto Finished; }} while (0, 0) -#define FINISHED_LAST_ERROR_IF(condition) do { if (condition) { hr = LogLastError(LOCATION_INFO); goto Finished; }} while (0, 0) -#define FINISHED_LAST_ERROR_IF_NULL(ptr) do { if ((ptr) == nullptr) { hr = LogLastError(LOCATION_INFO); goto Finished; }} while (0, 0) +#define _GOTO_FINISHED() __pragma(warning(push)) \ + __pragma(warning(disable:26438)) /*disable avoid goto warning*/ \ + goto Finished \ + __pragma(warning(pop)) + + +#define FINISHED(hrr) do { _HR_RET(hrr); if (_CHECK_FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); } hr = __hrRet; _GOTO_FINISHED(); } while (0, 0) +#define FINISHED_IF_FAILED(hrr) do { _HR_RET(hrr); if (FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); hr = __hrRet; _GOTO_FINISHED(); }} while (0, 0) +#define FINISHED_IF_NULL_ALLOC(ptr) do { if ((ptr) == nullptr) { hr = LogHResultFailed(LOCATION_INFO, E_OUTOFMEMORY); _GOTO_FINISHED(); }} while (0, 0) +#define FINISHED_LAST_ERROR_IF(condition) do { if (condition) { hr = LogLastError(LOCATION_INFO); _GOTO_FINISHED(); }} while (0, 0) +#define FINISHED_LAST_ERROR_IF_NULL(ptr) do { if ((ptr) == nullptr) { hr = LogLastError(LOCATION_INFO); _GOTO_FINISHED(); }} while (0, 0) #define THROW_LAST_ERROR() do { ThrowResultException(LOCATION_INFO, LogLastError(LOCATION_INFO)); } while (0, 0) -#define THROW_IF_FAILED(hr) do { HRESULT __hrRet = hr; if (FAILED(__hrRet)) { ThrowResultException(LOCATION_INFO, __hrRet); }} while (0, 0) +#define THROW_IF_FAILED(hr) do { _HR_RET(hr); if (FAILED(__hrRet)) { ThrowResultException(LOCATION_INFO, __hrRet); }} while (0, 0) #define THROW_LAST_ERROR_IF(condition) do { if (condition) { ThrowResultException(LOCATION_INFO, LogLastError(LOCATION_INFO)); }} while (0, 0) #define THROW_LAST_ERROR_IF_NULL(ptr) do { if ((ptr) == nullptr) { ThrowResultException(LOCATION_INFO, LogLastError(LOCATION_INFO)); }} while (0, 0) @@ -57,21 +73,24 @@ #define SUCCEEDED_LOG(hr) SUCCEEDED(LOG_IF_FAILED(hr)) #define FAILED_LOG(hr) FAILED(LOG_IF_FAILED(hr)) - class ResultException: public std::runtime_error { public: - explicit ResultException(HRESULT hr, LOCATION_ARGUMENTS_ONLY) : + ResultException(HRESULT hr, LOCATION_ARGUMENTS_ONLY) : runtime_error(format("HRESULT 0x%x returned at " LOCATION_FORMAT, hr, LOCATION_CALL_ONLY)), m_hr(hr) { } - HRESULT GetResult() const { return m_hr; } + HRESULT GetResult() const noexcept { return m_hr; } private: - HRESULT m_hr; + +#pragma warning( push ) +#pragma warning ( disable : 26495 ) // bug in CA: m_hr is reported as uninitialized + const HRESULT m_hr = S_OK; }; +#pragma warning( pop ) __declspec(noinline) inline VOID ReportUntypedException(LOCATION_ARGUMENTS_ONLY) { @@ -98,7 +117,7 @@ private: return condition; } - __declspec(noinline) inline VOID ReportException(LOCATION_ARGUMENTS std::exception& exception) + __declspec(noinline) inline VOID ReportException(LOCATION_ARGUMENTS const std::exception& exception) { DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, "Exception '%s' caught at " LOCATION_FORMAT, exception.what(), LOCATION_CALL_ONLY); } @@ -122,12 +141,12 @@ __declspec(noinline) inline HRESULT CaughtExceptionHResult(LOCATION_ARGUMENTS_ON { return E_OUTOFMEMORY; } - catch (ResultException& exception) + catch (const ResultException& exception) { ReportException(LOCATION_CALL exception); return exception.GetResult(); } - catch (std::exception& exception) + catch (const std::exception& exception) { ReportException(LOCATION_CALL exception); return HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION); @@ -154,7 +173,7 @@ template auto Throw_IfNullAlloc(PointerT pointer) } return pointer; } -__declspec(noinline) inline std::wstring GetUnexpectedExceptionMessage(std::runtime_error& ex) +__declspec(noinline) inline std::wstring GetUnexpectedExceptionMessage(const std::runtime_error& ex) { return format(L"Unexpected exception: %S", ex.what()); } diff --git a/src/AspNetCoreModuleV2/CommonLib/fx_ver.h b/src/AspNetCoreModuleV2/CommonLib/fx_ver.h index 2740fa37ee..723a0da360 100644 --- a/src/AspNetCoreModuleV2/CommonLib/fx_ver.h +++ b/src/AspNetCoreModuleV2/CommonLib/fx_ver.h @@ -13,15 +13,15 @@ struct fx_ver_t fx_ver_t(int major, int minor, int patch, const std::wstring& pre); fx_ver_t(int major, int minor, int patch, const std::wstring& pre, const std::wstring& build); - int get_major() const { return m_major; } - int get_minor() const { return m_minor; } - int get_patch() const { return m_patch; } + int get_major() const noexcept { return m_major; } + int get_minor() const noexcept { return m_minor; } + int get_patch() const noexcept { return m_patch; } - void set_major(int m) { m_major = m; } - void set_minor(int m) { m_minor = m; } - void set_patch(int p) { m_patch = p; } + void set_major(int m) noexcept { m_major = m; } + void set_minor(int m) noexcept { m_minor = m; } + void set_patch(int p) noexcept { m_patch = p; } - bool is_prerelease() const { return !m_pre.empty(); } + bool is_prerelease() const noexcept { return !m_pre.empty(); } std::wstring as_str() const; diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h index 8c42d08c37..243cc53ae7 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h @@ -17,7 +17,7 @@ public: std::filesystem::path dotnetExeLocation, std::filesystem::path hostFxrLocation, std::vector arguments - ) + ) noexcept : m_dotnetExeLocation(std::move(dotnetExeLocation)), m_hostFxrLocation(std::move(hostFxrLocation)), m_arguments(std::move(arguments)) @@ -27,7 +27,7 @@ public: GetArguments(DWORD &hostfxrArgc, std::unique_ptr &hostfxrArgv) const { hostfxrArgc = static_cast(m_arguments.size()); - hostfxrArgv = std::unique_ptr(new PCWSTR[hostfxrArgc]); + hostfxrArgv = std::make_unique(hostfxrArgc); for (DWORD i = 0; i < hostfxrArgc; ++i) { hostfxrArgv[i] = m_arguments[i].c_str(); @@ -35,13 +35,13 @@ public: } const std::filesystem::path& - GetHostFxrLocation() const + GetHostFxrLocation() const noexcept { return m_hostFxrLocation; } const std::filesystem::path& - GetDotnetExeLocation() const + GetDotnetExeLocation() const noexcept { return m_dotnetExeLocation; } diff --git a/src/AspNetCoreModuleV2/CommonLib/iapplication.h b/src/AspNetCoreModuleV2/CommonLib/iapplication.h index 222230c672..7c205ed68c 100644 --- a/src/AspNetCoreModuleV2/CommonLib/iapplication.h +++ b/src/AspNetCoreModuleV2/CommonLib/iapplication.h @@ -44,7 +44,7 @@ public: HRESULT CreateHandler( _In_ IHttpContext *pHttpContext, - _Out_ IREQUEST_HANDLER **pRequestHandler) = 0; + _Outptr_ IREQUEST_HANDLER **pRequestHandler) = 0; }; struct IAPPLICATION_DELETER diff --git a/src/AspNetCoreModuleV2/CommonLib/requesthandler.h b/src/AspNetCoreModuleV2/CommonLib/requesthandler.h index ea4692f506..7de54e4c77 100644 --- a/src/AspNetCoreModuleV2/CommonLib/requesthandler.h +++ b/src/AspNetCoreModuleV2/CommonLib/requesthandler.h @@ -14,13 +14,13 @@ class REQUEST_HANDLER: public virtual IREQUEST_HANDLER public: VOID - ReferenceRequestHandler() override + ReferenceRequestHandler() noexcept override { InterlockedIncrement(&m_cRefs); } VOID - DereferenceRequestHandler() override + DereferenceRequestHandler() noexcept override { DBG_ASSERT(m_cRefs != 0); diff --git a/src/AspNetCoreModuleV2/DefaultRules.ruleset b/src/AspNetCoreModuleV2/DefaultRules.ruleset new file mode 100644 index 0000000000..1c2bbdaeaf --- /dev/null +++ b/src/AspNetCoreModuleV2/DefaultRules.ruleset @@ -0,0 +1,432 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/AspNetCoreModuleV2/IISLib/acache.cpp b/src/AspNetCoreModuleV2/IISLib/acache.cpp index d68813edbc..5f2125134e 100644 --- a/src/AspNetCoreModuleV2/IISLib/acache.cpp +++ b/src/AspNetCoreModuleV2/IISLib/acache.cpp @@ -3,6 +3,9 @@ #include "precomp.h" +#pragma warning( push ) +#pragma warning ( disable : ALL_CODE_ANALYSIS_WARNINGS ) + LONG ALLOC_CACHE_HANDLER::sm_nFillPattern = 0xACA50000; HANDLE ALLOC_CACHE_HANDLER::sm_hHeap; @@ -440,4 +443,6 @@ Finished: } return fRet; -} \ No newline at end of file +} + +#pragma warning( pop ) diff --git a/src/AspNetCoreModuleV2/IISLib/buffer.h b/src/AspNetCoreModuleV2/IISLib/buffer.h index 1d68155387..385b73d717 100644 --- a/src/AspNetCoreModuleV2/IISLib/buffer.h +++ b/src/AspNetCoreModuleV2/IISLib/buffer.h @@ -4,7 +4,10 @@ #pragma once #include +#include +#pragma warning( push ) +#pragma warning ( disable : ALL_CODE_ANALYSIS_WARNINGS ) // // BUFFER_T class shouldn't be used directly. Use BUFFER specialization class instead. @@ -269,3 +272,5 @@ C_ASSERT( sizeof(VOID*) <= sizeof(ULONGLONG) ); #define INLINE_BUFFER_INIT( _name ) \ _name( (BYTE*)__aqw##_name, sizeof( __aqw##_name ) ) + +#pragma warning( pop ) diff --git a/src/AspNetCoreModuleV2/IISLib/multisz.h b/src/AspNetCoreModuleV2/IISLib/multisz.h index f65c151d4f..9473f52033 100644 --- a/src/AspNetCoreModuleV2/IISLib/multisz.h +++ b/src/AspNetCoreModuleV2/IISLib/multisz.h @@ -6,6 +6,10 @@ #include "stringu.h" #include "ntassert.h" +#include + +#pragma warning( push ) +#pragma warning ( disable : ALL_CODE_ANALYSIS_WARNINGS ) /*++ class MULTISZ: @@ -221,5 +225,6 @@ SplitCommaDelimitedString( MULTISZ * pmszList ); -#endif // !_MULTISZ_HXX_ +#pragma warning( pop ) +#endif // !_MULTISZ_HXX_ diff --git a/src/AspNetCoreModuleV2/IISLib/percpu.h b/src/AspNetCoreModuleV2/IISLib/percpu.h index 5d3c563935..07828830d7 100644 --- a/src/AspNetCoreModuleV2/IISLib/percpu.h +++ b/src/AspNetCoreModuleV2/IISLib/percpu.h @@ -3,6 +3,9 @@ #pragma once +#pragma warning( push ) +#pragma warning ( disable : 26451 ) + template class PER_CPU { @@ -302,4 +305,6 @@ Return: *pCacheLineSize = SYSTEM_CACHE_ALIGNMENT_SIZE; return S_OK; -} \ No newline at end of file +} + +#pragma warning( pop ) diff --git a/src/AspNetCoreModuleV2/IISLib/stringa.h b/src/AspNetCoreModuleV2/IISLib/stringa.h index 39737f4a69..94ace540f6 100644 --- a/src/AspNetCoreModuleV2/IISLib/stringa.h +++ b/src/AspNetCoreModuleV2/IISLib/stringa.h @@ -7,6 +7,10 @@ #include "macros.h" #include + +#pragma warning( push ) +#pragma warning ( disable : ALL_CODE_ANALYSIS_WARNINGS ) + class STRA { @@ -513,3 +517,5 @@ CHAR* InitHelper(__out CHAR (&psz)[size]) STRA name; #define INLINE_STRA_INIT(name) name(InitHelper(__ach##name), sizeof(__ach##name)) + +#pragma warning( pop ) diff --git a/src/AspNetCoreModuleV2/IISLib/stringu.cpp b/src/AspNetCoreModuleV2/IISLib/stringu.cpp index 15da79a7fe..74f8595482 100644 --- a/src/AspNetCoreModuleV2/IISLib/stringu.cpp +++ b/src/AspNetCoreModuleV2/IISLib/stringu.cpp @@ -1,10 +1,12 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for license information. -#pragma warning (disable : 4267) #include "precomp.h" +#pragma warning( push ) +#pragma warning ( disable : 4267 ALL_CODE_ANALYSIS_WARNINGS ) + STRU::STRU( VOID ) : m_cchLen( 0 ) @@ -1199,25 +1201,17 @@ STRU::ExpandEnvironmentVariables( __out STRU * pstrExpandedString ) /*++ - Routine Description: - Expand the environment variables in a string - Arguments: - pszString - String with environment variables to expand pstrExpandedString - Receives expanded string on success - Return Value: - HRESULT - --*/ { HRESULT hr = S_OK; DWORD cchNewSize = 0; - if ( pszString == NULL || pstrExpandedString == NULL ) { @@ -1225,7 +1219,6 @@ Return Value: hr = HRESULT_FROM_WIN32( ERROR_INVALID_PARAMETER ); goto Exit; } - cchNewSize = ExpandEnvironmentStrings( pszString, pstrExpandedString->QueryStr(), pstrExpandedString->QuerySizeCCH() ); @@ -1234,7 +1227,6 @@ Return Value: hr = HRESULT_FROM_WIN32( GetLastError() ); goto Exit; } - if ( cchNewSize > pstrExpandedString->QuerySizeCCH() ) { hr = pstrExpandedString->Resize( @@ -1244,13 +1236,11 @@ Return Value: { goto Exit; } - cchNewSize = ExpandEnvironmentStrings( pszString, pstrExpandedString->QueryStr(), pstrExpandedString->QuerySizeCCH() ); - if ( cchNewSize == 0 || cchNewSize > pstrExpandedString->QuerySizeCCH() ) { @@ -1258,14 +1248,10 @@ Return Value: goto Exit; } } - pstrExpandedString->SyncWithBuffer(); - hr = S_OK; - Exit: - return hr; } -#pragma warning(default:4267) +#pragma warning( pop ) diff --git a/src/AspNetCoreModuleV2/IISLib/stringu.h b/src/AspNetCoreModuleV2/IISLib/stringu.h index 6f27c5421d..f60f04cfbb 100644 --- a/src/AspNetCoreModuleV2/IISLib/stringu.h +++ b/src/AspNetCoreModuleV2/IISLib/stringu.h @@ -4,8 +4,12 @@ #pragma once #include "buffer.h" +#include #include +#pragma warning( push ) +#pragma warning ( disable : ALL_CODE_ANALYSIS_WARNINGS ) + class STRU { @@ -349,13 +353,12 @@ public: __in PCWSTR pwszFormatString, va_list argsList ); - + static HRESULT ExpandEnvironmentVariables( __in PCWSTR pszString, __out STRU * pstrExpandedString ); - private: // @@ -425,3 +428,4 @@ MakePathCanonicalizationProof( IN PCWSTR pszName, OUT STRU * pstrPath ); +#pragma warning( pop ) diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp index d8d28e5a28..c1444a8571 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp @@ -184,7 +184,7 @@ http_get_application_properties( return E_FAIL; } - auto pConfiguration = pInProcessApplication->QueryConfig(); + const auto& pConfiguration = pInProcessApplication->QueryConfig(); pIISCofigurationData->pInProcessApplication = pInProcessApplication; pIISCofigurationData->pwzFullApplicationPath = SysAllocString(pInProcessApplication->QueryApplicationPhysicalPath().c_str()); diff --git a/src/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp b/src/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp index 0b987bf8cd..a946e7c2dc 100644 --- a/src/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp +++ b/src/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp @@ -38,7 +38,7 @@ HRESULT AppOfflineTrackingApplication::StartMonitoringAppOflineImpl() { if (m_fileWatcher) { - RETURN_IF_FAILED(E_UNEXPECTED); + RETURN_HR(E_UNEXPECTED); } m_fileWatcher = std::make_unique(); From 18a718d1f09ccd527f82e52fee1546a4fd2a4a08 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 5 Sep 2018 16:48:10 -0700 Subject: [PATCH 13/18] Catch all exception in native callbacks (#1369) --- .../Core/IISHttpServer.cs | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs index 1e6a7c86f4..ddf16f269d 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs @@ -20,9 +20,9 @@ namespace Microsoft.AspNetCore.Server.IIS.Core { private const string WebSocketVersionString = "WEBSOCKET_VERSION"; - private static NativeMethods.PFN_REQUEST_HANDLER _requestHandler = HandleRequest; - private static NativeMethods.PFN_SHUTDOWN_HANDLER _shutdownHandler = HandleShutdown; - private static NativeMethods.PFN_ASYNC_COMPLETION _onAsyncCompletion = OnAsyncCompletion; + private static readonly NativeMethods.PFN_REQUEST_HANDLER _requestHandler = HandleRequest; + private static readonly NativeMethods.PFN_SHUTDOWN_HANDLER _shutdownHandler = HandleShutdown; + private static readonly NativeMethods.PFN_ASYNC_COMPLETION _onAsyncCompletion = OnAsyncCompletion; private IISContextFactory _iisContextFactory; private readonly MemoryPool _memoryPool = new SlabMemoryPool(); @@ -147,15 +147,24 @@ namespace Microsoft.AspNetCore.Server.IIS.Core private static NativeMethods.REQUEST_NOTIFICATION_STATUS HandleRequest(IntPtr pInProcessHandler, IntPtr pvRequestContext) { - // Unwrap the server so we can create an http context and process the request - var server = (IISHttpServer)GCHandle.FromIntPtr(pvRequestContext).Target; - Interlocked.Increment(ref server._outstandingRequests); + IISHttpServer server = null; + try + { + // Unwrap the server so we can create an http context and process the request + server = (IISHttpServer)GCHandle.FromIntPtr(pvRequestContext).Target; + Interlocked.Increment(ref server._outstandingRequests); - var context = server._iisContextFactory.CreateHttpContext(pInProcessHandler); + var context = server._iisContextFactory.CreateHttpContext(pInProcessHandler); - ThreadPool.QueueUserWorkItem(state => _ = HandleRequest((IISHttpContext)state), context); + ThreadPool.QueueUserWorkItem(state => _ = HandleRequest((IISHttpContext)state), context); + return NativeMethods.REQUEST_NOTIFICATION_STATUS.RQ_NOTIFICATION_PENDING; + } + catch (Exception ex) + { + server?._logger.LogError(0, ex, $"Unexpected exception in static {nameof(IISHttpServer)}.{nameof(HandleRequest)}."); - return NativeMethods.REQUEST_NOTIFICATION_STATUS.RQ_NOTIFICATION_PENDING; + return NativeMethods.REQUEST_NOTIFICATION_STATUS.RQ_NOTIFICATION_FINISH_REQUEST; + } } private static async Task HandleRequest(IISHttpContext context) @@ -167,7 +176,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core } catch (Exception ex) { - context.Server._logger.LogError("Exception in ProcessRequestAsync", ex); + context.Server._logger.LogError(0, ex, $"Unexpected exception in {nameof(IISHttpServer)}.{nameof(HandleRequest)}."); } finally { @@ -177,16 +186,34 @@ namespace Microsoft.AspNetCore.Server.IIS.Core private static bool HandleShutdown(IntPtr pvRequestContext) { - var server = (IISHttpServer)GCHandle.FromIntPtr(pvRequestContext).Target; - server._applicationLifetime.StopApplication(); + IISHttpServer server = null; + try + { + server = (IISHttpServer)GCHandle.FromIntPtr(pvRequestContext).Target; + server._applicationLifetime.StopApplication(); + } + catch (Exception ex) + { + server?._logger.LogError(0, ex, $"Unexpected exception in {nameof(IISHttpServer)}.{nameof(HandleShutdown)}."); + } return true; } private static NativeMethods.REQUEST_NOTIFICATION_STATUS OnAsyncCompletion(IntPtr pvManagedHttpContext, int hr, int bytes) { - var context = (IISHttpContext)GCHandle.FromIntPtr(pvManagedHttpContext).Target; - context.OnAsyncCompletion(hr, bytes); - return NativeMethods.REQUEST_NOTIFICATION_STATUS.RQ_NOTIFICATION_PENDING; + IISHttpContext context = null; + try + { + context = (IISHttpContext)GCHandle.FromIntPtr(pvManagedHttpContext).Target; + context.OnAsyncCompletion(hr, bytes); + return NativeMethods.REQUEST_NOTIFICATION_STATUS.RQ_NOTIFICATION_PENDING; + } + catch (Exception ex) + { + context?.Server._logger.LogError(0, ex, $"Unexpected exception in {nameof(IISHttpServer)}.{nameof(OnAsyncCompletion)}."); + + return NativeMethods.REQUEST_NOTIFICATION_STATUS.RQ_NOTIFICATION_FINISH_REQUEST; + } } private static void CompleteRequest(IISHttpContext context, bool result) From 5956300df7fdc55ed77cdba0a55bda9e9d2549b9 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 5 Sep 2018 16:48:23 -0700 Subject: [PATCH 14/18] Retry TestServer port selection (#1367) --- test/IIS.Tests/Utilities/TestServer.cs | 83 +++++++++++++++++++------- 1 file changed, 63 insertions(+), 20 deletions(-) diff --git a/test/IIS.Tests/Utilities/TestServer.cs b/test/IIS.Tests/Utilities/TestServer.cs index 4adaaff776..cd3188bf83 100644 --- a/test/IIS.Tests/Utilities/TestServer.cs +++ b/test/IIS.Tests/Utilities/TestServer.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.Collections.Generic; using System.IO; using System.Net.Http; using System.Reflection; @@ -14,7 +15,9 @@ using System.Xml.XPath; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Server.IntegrationTesting; +using Microsoft.AspNetCore.Server.IntegrationTesting.Common; using Microsoft.AspNetCore.Server.IntegrationTesting.IIS; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -29,39 +32,33 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests internal static string HostableWebCoreLocation => Environment.ExpandEnvironmentVariables($@"%windir%\system32\inetsrv\{HWebCoreDll}"); internal static string BasePath => Path.GetDirectoryName(new Uri(typeof(TestServer).Assembly.CodeBase).AbsolutePath); - internal static string InProcessHandlerLocation => Path.Combine(BasePath, InProcessHandlerDll); internal static string AspNetCoreModuleLocation => Path.Combine(BasePath, AspNetCoreModuleDll); private static readonly SemaphoreSlim WebCoreLock = new SemaphoreSlim(1, 1); - // Currently this is hardcoded in HostableWebCore.config - private static readonly int BasePort = 50691; - private static readonly Uri BaseUri = new Uri("http://localhost:" + BasePort); + private static readonly int PortRetryCount = 10; private readonly TaskCompletionSource _startedTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); private readonly Action _appBuilder; private readonly ILoggerFactory _loggerFactory; private readonly hostfxr_main_fn _hostfxrMainFn; - - public HttpClient HttpClient { get; } - public TestConnection CreateConnection() => new TestConnection(BasePort); + + private Uri BaseUri => new Uri("http://localhost:" + _currentPort); + public HttpClient HttpClient { get; private set; } + public TestConnection CreateConnection() => new TestConnection(_currentPort); private IWebHost _host; private string _appHostConfigPath; + private int _currentPort; private TestServer(Action appBuilder, ILoggerFactory loggerFactory) { _hostfxrMainFn = Main; _appBuilder = appBuilder; _loggerFactory = loggerFactory; - - HttpClient = new HttpClient(new LoggingHandler(new SocketsHttpHandler(), _loggerFactory.CreateLogger())) - { - BaseAddress = BaseUri - }; } public static async Task Create(Action appBuilder, ILoggerFactory loggerFactory) @@ -83,19 +80,45 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { LoadLibrary(HostableWebCoreLocation); _appHostConfigPath = Path.GetTempFileName(); + + set_main_handler(_hostfxrMainFn); + Retry(() => + { + _currentPort = TestPortHelper.GetNextPort(); + + InitializeConfig(_currentPort); + + var startResult = WebCoreActivate(_appHostConfigPath, null, "Instance"); + if (startResult != 0) + { + throw new InvalidOperationException($"Error while running WebCoreActivate: {startResult} on port {_currentPort}"); + } + }, PortRetryCount); + + HttpClient = new HttpClient(new LoggingHandler(new SocketsHttpHandler(), _loggerFactory.CreateLogger())) + { + BaseAddress = BaseUri + }; + } + + private void InitializeConfig(int port) + { var webHostConfig = XDocument.Load(Path.GetFullPath("HostableWebCore.config")); webHostConfig.XPathSelectElement("/configuration/system.webServer/globalModules/add[@name='AspNetCoreModuleV2']") .SetAttributeValue("image", AspNetCoreModuleLocation); + + var siteElement = webHostConfig.Root + .RequiredElement("system.applicationHost") + .RequiredElement("sites") + .RequiredElement("site"); + + siteElement + .RequiredElement("bindings") + .RequiredElement("binding") + .SetAttributeValue("bindingInformation", $":{port}:localhost"); + webHostConfig.Save(_appHostConfigPath); - - set_main_handler(_hostfxrMainFn); - - var startResult = WebCoreActivate(_appHostConfigPath, null, "Instance"); - if (startResult != 0) - { - throw new InvalidOperationException($"Error while running WebCoreActivate: {startResult}"); - } } private int Main(IntPtr argc, IntPtr argv) @@ -157,5 +180,25 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests [DllImport("kernel32", SetLastError=true, CharSet = CharSet.Ansi)] private static extern IntPtr LoadLibrary([MarshalAs(UnmanagedType.LPStr)] string lpFileName); + + private void Retry(Action func, int attempts) + { + var exceptions = new List(); + + for (var attempt = 0; attempt < attempts; attempt++) + { + try + { + func(); + return; + } + catch (Exception e) + { + exceptions.Add(e); + } + } + + throw new AggregateException(exceptions); + } } } From 88d857147454cf739d43b2676cb8ff1dce23b1f2 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 5 Sep 2018 17:15:04 -0700 Subject: [PATCH 15/18] Add Trace level for request events, allow logging to eventlog (#1337) --- .../AspNetCore/applicationinfo.cpp | 2 +- src/AspNetCoreModuleV2/CommonLib/EventLog.cpp | 65 +++++++++------- src/AspNetCoreModuleV2/CommonLib/EventLog.h | 7 ++ .../CommonLib/aspnetcore_msg.mc | 6 ++ .../CommonLib/debugutil.cpp | 52 +++++++------ src/AspNetCoreModuleV2/CommonLib/debugutil.h | 9 ++- src/AspNetCoreModuleV2/IISLib/dbgutil.h | 10 ++- .../OutOfProcessRequestHandler/dllmain.cpp | 6 +- .../forwardinghandler.cpp | 40 +++------- .../websockethandler.cpp | 78 +++++++------------ .../requesthandler_config.cpp | 2 - .../IISDeployer.cs | 2 +- .../IISDeploymentParameterExtensions.cs | 2 +- .../Inprocess/LoggingTests.cs | 39 ++++++++-- 14 files changed, 170 insertions(+), 150 deletions(-) diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp index fb558e52d0..225a82bdec 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp @@ -98,7 +98,7 @@ APPLICATION_INFO::ShutDownApplication(bool fServerInitiated) if (m_pApplication) { - LOG_ERRORF(L"Stopping application '%ls'", QueryApplicationInfoKey().c_str()); + LOG_INFOF(L"Stopping application '%ls'", QueryApplicationInfoKey().c_str()); m_pApplication ->Stop(fServerInitiated); m_pApplication = nullptr; m_pApplicationFactory = nullptr; diff --git a/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp b/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp index c3a3608820..e5ee8c1708 100644 --- a/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/EventLog.cpp @@ -4,10 +4,46 @@ #include #include "EventLog.h" #include "debugutil.h" -#include "StringHelpers.h" +#include "exceptions.h" extern HANDLE g_hEventLog; +bool +EventLog::LogEventNoTrace( + _In_ WORD dwEventInfoType, + _In_ DWORD dwEventId, + _In_ LPCWSTR pstrMsg +) +{ + if (g_hEventLog == nullptr) + { + return true; + } + // Static locals to avoid getting the process ID and string multiple times. + // Effectively have the same semantics as global variables, except initialized + // on first occurence. + static const auto processIdString = GetProcessIdString(); + static const auto versionInfoString = GetVersionInfoString(); + + std::array eventLogDataStrings + { + pstrMsg, + processIdString.c_str(), + versionInfoString.c_str() + }; + + return ReportEventW(g_hEventLog, + dwEventInfoType, + 0, // wCategory + dwEventId, + NULL, // lpUserSid + static_cast(eventLogDataStrings.size()), // wNumStrings + 0, // dwDataSize, + eventLogDataStrings.data(), + NULL // lpRawData + ); +} + VOID EventLog::LogEvent( _In_ WORD dwEventInfoType, @@ -15,32 +51,7 @@ EventLog::LogEvent( _In_ LPCWSTR pstrMsg ) { - // Static locals to avoid getting the process ID and string multiple times. - // Effectively have the same semantics as global variables, except initialized - // on first occurence. - static const auto processIdString = GetProcessIdString(); - static const auto versionInfoString = GetVersionInfoString(); - - std::array eventLogDataStrings - { - pstrMsg, - processIdString.c_str(), - versionInfoString.c_str() - }; - - if (g_hEventLog != NULL) - { - ReportEventW(g_hEventLog, - dwEventInfoType, - 0, // wCategory - dwEventId, - NULL, // lpUserSid - 3, // wNumStrings - 0, // dwDataSize, - eventLogDataStrings.data(), - NULL // lpRawData - ); - } + LOG_LAST_ERROR_IF(!LogEventNoTrace(dwEventInfoType, dwEventId, pstrMsg)); DebugPrintfW(dwEventInfoType == EVENTLOG_ERROR_TYPE ? ASPNETCORE_DEBUG_FLAG_ERROR : ASPNETCORE_DEBUG_FLAG_INFO, L"Event Log: '%ls' \r\nEnd Event Log Message.", pstrMsg); } diff --git a/src/AspNetCoreModuleV2/CommonLib/EventLog.h b/src/AspNetCoreModuleV2/CommonLib/EventLog.h index fa2cde1947..4bb72b623a 100644 --- a/src/AspNetCoreModuleV2/CommonLib/EventLog.h +++ b/src/AspNetCoreModuleV2/CommonLib/EventLog.h @@ -59,6 +59,13 @@ public: _va_end(args); } + static + bool + LogEventNoTrace( + _In_ WORD dwEventInfoType, + _In_ DWORD dwEventId, + _In_ LPCWSTR pstrMsg); + private: static VOID diff --git a/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc b/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc index 7289b85ce6..547192d0df 100644 --- a/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc +++ b/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc @@ -201,6 +201,12 @@ Language=English %1 . +Messageid=1036 +SymbolicName=ASPNETCORE_EVENT_DEBUG_LOG +Language=English +%1 +. + ; ;#endif // _ASPNETCORE_MODULE_MSG_H_ diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp index 964c411bf9..cbf48a3b20 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp @@ -3,17 +3,19 @@ #include "debugutil.h" +#include #include #include "dbgutil.h" #include "stringu.h" #include "stringa.h" -#include "dbgutil.h" #include "Environment.h" #include "SRWExclusiveLock.h" #include "exceptions.h" #include "atlbase.h" #include "config_utility.h" #include "StringHelpers.h" +#include "aspnetcore_msg.h" +#include "EventLog.h" inline HANDLE g_logFile = INVALID_HANDLE_VALUE; inline HMODULE g_hModule; @@ -23,7 +25,7 @@ HRESULT PrintDebugHeader() { // Major, minor are stored in dwFileVersionMS field and patch, build in dwFileVersionLS field as pair of 32 bit numbers - DebugPrintfW(ASPNETCORE_DEBUG_FLAG_INFO, L"Initializing logs for '%ls'. %ls. %ls.", + LOG_INFOF(L"Initializing logs for '%ls'. %ls. %ls.", GetModuleName().c_str(), GetProcessIdString().c_str(), GetVersionInfoString().c_str() @@ -92,26 +94,6 @@ GetModuleName() void SetDebugFlags(const std::wstring &debugValue) { - try - { - if (!debugValue.empty() && debugValue.find_first_not_of(L"0123456789") == std::wstring::npos) - { - const auto value = std::stoi(debugValue); - - if (value >= 1) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_ERROR; - if (value >= 2) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_WARNING; - if (value >= 3) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_INFO; - if (value >= 4) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_CONSOLE; - if (value >= 5) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_FILE; - - return; - } - } - catch (...) - { - // ignore - } - try { std::wstringstream stringStream(debugValue); @@ -122,14 +104,16 @@ void SetDebugFlags(const std::wstring &debugValue) if (_wcsnicmp(flag.c_str(), L"error", wcslen(L"error")) == 0) DEBUG_FLAGS_VAR |= DEBUG_FLAGS_ERROR; if (_wcsnicmp(flag.c_str(), L"warning", wcslen(L"warning")) == 0) DEBUG_FLAGS_VAR |= DEBUG_FLAGS_WARN; if (_wcsnicmp(flag.c_str(), L"info", wcslen(L"info")) == 0) DEBUG_FLAGS_VAR |= DEBUG_FLAGS_INFO; + if (_wcsnicmp(flag.c_str(), L"trace", wcslen(L"trace")) == 0) DEBUG_FLAGS_VAR |= DEBUG_FLAGS_TRACE; if (_wcsnicmp(flag.c_str(), L"console", wcslen(L"console")) == 0) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_CONSOLE; if (_wcsnicmp(flag.c_str(), L"file", wcslen(L"file")) == 0) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_FILE; + if (_wcsnicmp(flag.c_str(), L"eventlog", wcslen(L"eventlog")) == 0) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_EVENTLOG; } - // If file or console is enabled but level is not set, enable all levels + // If file or console is enabled but level is not set, enable levels up to info if (DEBUG_FLAGS_VAR != 0 && (DEBUG_FLAGS_VAR & DEBUG_FLAGS_ANY) == 0) { - DEBUG_FLAGS_VAR |= DEBUG_FLAGS_ANY; + DEBUG_FLAGS_VAR |= DEBUG_FLAGS_INFO; } } catch (...) @@ -209,7 +193,7 @@ DebugInitialize(HMODULE hModule) try { - SetDebugFlags(Environment::GetEnvironmentVariableValue(L"ASPNETCORE_MODULE_DEBUG").value_or(L"0")); + SetDebugFlags(Environment::GetEnvironmentVariableValue(L"ASPNETCORE_MODULE_DEBUG").value_or(L"")); } catch (...) { @@ -344,6 +328,24 @@ DebugPrintW( FlushFileBuffers(g_logFile); } } + + if (IsEnabled(ASPNETCORE_DEBUG_FLAG_EVENTLOG)) + { + WORD eventType; + switch (dwFlag) + { + case ASPNETCORE_DEBUG_FLAG_ERROR: + eventType = EVENTLOG_ERROR_TYPE; + break; + case ASPNETCORE_DEBUG_FLAG_WARNING: + eventType = EVENTLOG_WARNING_TYPE; + break; + default: + eventType = EVENTLOG_INFORMATION_TYPE; + break; + } + EventLog::LogEventNoTrace(eventType, ASPNETCORE_EVENT_DEBUG_LOG, strOutput.QueryStr()); + } } } diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.h b/src/AspNetCoreModuleV2/CommonLib/debugutil.h index f8c7a9b7c9..24446dbd80 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.h +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.h @@ -8,11 +8,16 @@ #include #include "dbgutil.h" +#define ASPNETCORE_DEBUG_FLAG_TRACE DEBUG_FLAG_TRACE #define ASPNETCORE_DEBUG_FLAG_INFO DEBUG_FLAG_INFO #define ASPNETCORE_DEBUG_FLAG_WARNING DEBUG_FLAG_WARN #define ASPNETCORE_DEBUG_FLAG_ERROR DEBUG_FLAG_ERROR -#define ASPNETCORE_DEBUG_FLAG_CONSOLE 0x00000008 -#define ASPNETCORE_DEBUG_FLAG_FILE 0x00000010 +#define ASPNETCORE_DEBUG_FLAG_CONSOLE 0x00000010 +#define ASPNETCORE_DEBUG_FLAG_FILE 0x00000020 +#define ASPNETCORE_DEBUG_FLAG_EVENTLOG 0x00000040 + +#define LOG_TRACE(...) DebugPrintW(ASPNETCORE_DEBUG_FLAG_TRACE, __VA_ARGS__) +#define LOG_TRACEF(...) DebugPrintfW(ASPNETCORE_DEBUG_FLAG_TRACE, __VA_ARGS__) #define LOG_INFO(...) DebugPrintW(ASPNETCORE_DEBUG_FLAG_INFO, __VA_ARGS__) #define LOG_INFOF(...) DebugPrintfW(ASPNETCORE_DEBUG_FLAG_INFO, __VA_ARGS__) diff --git a/src/AspNetCoreModuleV2/IISLib/dbgutil.h b/src/AspNetCoreModuleV2/IISLib/dbgutil.h index 60365faad2..9b714e9bbf 100644 --- a/src/AspNetCoreModuleV2/IISLib/dbgutil.h +++ b/src/AspNetCoreModuleV2/IISLib/dbgutil.h @@ -21,19 +21,21 @@ // Debug error levels for DEBUG_FLAGS_VAR. // -#define DEBUG_FLAG_INFO 0x00000001 -#define DEBUG_FLAG_WARN 0x00000002 -#define DEBUG_FLAG_ERROR 0x00000004 +#define DEBUG_FLAG_TRACE 0x00000001 +#define DEBUG_FLAG_INFO 0x00000002 +#define DEBUG_FLAG_WARN 0x00000004 +#define DEBUG_FLAG_ERROR 0x00000008 // // Predefined error level values. These are backwards from the // windows definitions. // +#define DEBUG_FLAGS_TRACE (DEBUG_FLAG_ERROR | DEBUG_FLAG_WARN | DEBUG_FLAG_INFO | DEBUG_FLAG_TRACE) #define DEBUG_FLAGS_INFO (DEBUG_FLAG_ERROR | DEBUG_FLAG_WARN | DEBUG_FLAG_INFO) #define DEBUG_FLAGS_WARN (DEBUG_FLAG_ERROR | DEBUG_FLAG_WARN) #define DEBUG_FLAGS_ERROR (DEBUG_FLAG_ERROR) -#define DEBUG_FLAGS_ANY (DEBUG_FLAG_INFO | DEBUG_FLAG_WARN | DEBUG_FLAG_ERROR) +#define DEBUG_FLAGS_ANY (DEBUG_FLAG_INFO | DEBUG_FLAG_WARN | DEBUG_FLAG_ERROR | DEBUG_FLAG_TRACE) // // Global variables to control tracing. Generally per module diff --git a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp index 451221255b..b5f2014bf2 100644 --- a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp +++ b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp @@ -115,7 +115,7 @@ Finished: // Global initialization routine for OutOfProcess // HRESULT -EnsureOutOfProcessInitializtion() +EnsureOutOfProcessInitializtion(IHttpApplication *pHttpApplication) { DBG_ASSERT(g_pHttpServer); @@ -202,6 +202,8 @@ EnsureOutOfProcessInitializtion() FINISHED_IF_FAILED(ALLOC_CACHE_HANDLER::StaticInitialize()); FINISHED_IF_FAILED(FORWARDING_HANDLER::StaticInitialize(g_fEnableReferenceCountTracing)); FINISHED_IF_FAILED(WEBSOCKET_HANDLER::StaticInitialize(g_fEnableReferenceCountTracing)); + + DebugInitializeFromConfig(*g_pHttpServer, *pHttpApplication); } Finished: if (FAILED(hr)) @@ -255,7 +257,7 @@ CreateApplication( RETURN_IF_FAILED(REQUESTHANDLER_CONFIG::CreateRequestHandlerConfig(pServer, pHttpApplication, &pConfig)); std::unique_ptr pRequestHandlerConfig(pConfig); - RETURN_IF_FAILED(EnsureOutOfProcessInitializtion()); + RETURN_IF_FAILED(EnsureOutOfProcessInitializtion(pHttpApplication)); std::unique_ptr pApplication = std::make_unique(*pHttpApplication, std::move(pRequestHandlerConfig)); diff --git a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp index 881c6393ad..b0fa1aa03f 100644 --- a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp +++ b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp @@ -48,10 +48,7 @@ FORWARDING_HANDLER::FORWARDING_HANDLER( m_pW3Context(pW3Context), m_pApplication(pApplication) { -#ifdef DEBUG - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "FORWARDING_HANDLER::FORWARDING_HANDLER"); -#endif + LOG_TRACE(L"FORWARDING_HANDLER::FORWARDING_HANDLER"); m_fWebSocketSupported = m_pApplication->QueryWebsocketStatus(); InitializeSRWLock(&m_RequestLock); @@ -65,10 +62,8 @@ FORWARDING_HANDLER::~FORWARDING_HANDLER( // m_Signature = FORWARDING_HANDLER_SIGNATURE_FREE; -#ifdef DEBUG - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "FORWARDING_HANDLER::~FORWARDING_HANDLER"); -#endif + LOG_TRACE(L"FORWARDING_HANDLER::~FORWARDING_HANDLER"); + // // RemoveRequest() should already have been called and m_pDisconnect // has been freed or m_pDisconnect was never initialized. @@ -309,10 +304,9 @@ FORWARDING_HANDLER::OnExecuteRequestHandler() reinterpret_cast(static_cast(this)))) { hr = HRESULT_FROM_WIN32(GetLastError()); -#ifdef DEBUG - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "FORWARDING_HANDLER::OnExecuteRequestHandler, Send request failed"); -#endif + + LOG_TRACE(L"FORWARDING_HANDLER::OnExecuteRequestHandler, Send request failed"); + // FREB log if (ANCMEvents::ANCM_REQUEST_FORWARD_FAIL::IsEnabled(m_pW3Context->GetTraceContext())) { @@ -460,10 +454,8 @@ REQUEST_NOTIFICATION_STATUS if (m_RequestStatus == FORWARDER_RECEIVED_WEBSOCKET_RESPONSE) { -#ifdef DEBUG - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "FORWARDING_HANDLER::OnAsyncCompletion, Send completed for 101 response"); -#endif + LOG_TRACE(L"FORWARDING_HANDLER::OnAsyncCompletion, Send completed for 101 response"); + // // This should be the write completion of the 101 response. // @@ -696,10 +688,7 @@ Finished: // // Do not use this object after dereferencing it, it may be gone. // -#ifdef DEBUG - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "FORWARDING_HANDLER::OnAsyncCompletion Done %d", retVal); -#endif + LOG_TRACEF(L"FORWARDING_HANDLER::OnAsyncCompletion Done %d", retVal); return retVal; } @@ -1336,10 +1325,8 @@ None dwInternetStatus); } -#ifdef DEBUG - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "FORWARDING_HANDLER::OnWinHttpCompletionInternal %x -- %d --%p\n", dwInternetStatus, GetCurrentThreadId(), m_pW3Context); -#endif + LOG_TRACEF(L"FORWARDING_HANDLER::OnWinHttpCompletionInternal %x -- %d --%p\n", dwInternetStatus, GetCurrentThreadId(), m_pW3Context); + // // Exclusive lock on the winhttp handle to protect from a client disconnect/ // server stop closing the handle while we are using it. @@ -2751,10 +2738,7 @@ FORWARDING_HANDLER::TerminateRequest( // a winhttp callback on the same thread and we donot want to // acquire the lock again -#ifdef DEBUG - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "FORWARDING_HANDLER::TerminateRequest %d --%p\n", GetCurrentThreadId(), m_pW3Context); -#endif // DEBUG + LOG_TRACEF(L"FORWARDING_HANDLER::TerminateRequest %d --%p\n", GetCurrentThreadId(), m_pW3Context); if (!m_fHttpHandleInClose) { diff --git a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/websockethandler.cpp b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/websockethandler.cpp index 5eac6001e9..ecd97737fe 100644 --- a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/websockethandler.cpp +++ b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/websockethandler.cpp @@ -47,7 +47,7 @@ WEBSOCKET_HANDLER::WEBSOCKET_HANDLER() : _fHandleClosed(FALSE), _fReceivedCloseMsg(FALSE) { - DebugPrintf (ASPNETCORE_DEBUG_FLAG_INFO, "WEBSOCKET_HANDLER::WEBSOCKET_HANDLER"); + LOG_TRACE(L"WEBSOCKET_HANDLER::WEBSOCKET_HANDLER"); InitializeCriticalSectionAndSpinCount(&_RequestLock, 1000); InsertRequest(); @@ -58,7 +58,7 @@ WEBSOCKET_HANDLER::Terminate( VOID ) { - DebugPrintf (ASPNETCORE_DEBUG_FLAG_INFO, "WEBSOCKET_HANDLER::Terminate"); + LOG_TRACE(L"WEBSOCKET_HANDLER::Terminate"); if (!_fHandleClosed) { RemoveRequest(); @@ -212,8 +212,7 @@ WEBSOCKET_HANDLER::IndicateCompletionToIIS( --*/ { - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::IndicateCompletionToIIS called %d", _dwOutstandingIo); + LOG_TRACEF(L"WEBSOCKET_HANDLER::IndicateCompletionToIIS called %d", _dwOutstandingIo); // // close Websocket handle. This will triger a WinHttp callback @@ -223,8 +222,7 @@ WEBSOCKET_HANDLER::IndicateCompletionToIIS( // if (_hWebSocketRequest != NULL && _dwOutstandingIo == 0) { - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::IndicateCompletionToIIS"); + LOG_TRACE(L"WEBSOCKET_HANDLER::IndicateCompletionToIIS"); _pHandler->SetStatus(FORWARDER_DONE); _fHandleClosed = TRUE; @@ -262,8 +260,7 @@ Routine Description: _pHandler = pHandler; EnterCriticalSection(&_RequestLock); - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::ProcessRequest"); + LOG_TRACEF(L"WEBSOCKET_HANDLER::ProcessRequest"); // // Cache the points to IHttpContext3 @@ -359,8 +356,7 @@ Finished: if (FAILED_LOG(hr)) { - DebugPrintf (ASPNETCORE_DEBUG_FLAG_ERROR, - "Process Request Failed with HR=%08x", hr); + LOG_ERRORF(L"Process Request Failed with HR=%08x", hr); } return hr; @@ -385,8 +381,7 @@ Routine Description: BOOL fFinalFragment; BOOL fClose; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::DoIisWebSocketReceive"); + LOG_TRACE(L"WEBSOCKET_HANDLER::DoIisWebSocketReceive"); IncrementOutstandingIo(); @@ -403,8 +398,7 @@ Routine Description: if (FAILED_LOG(hr)) { DecrementOutstandingIo(); - DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::DoIisWebSocketSend failed with %08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::DoIisWebSocketSend failed with %08x", hr); } return hr; @@ -426,8 +420,7 @@ Routine Description: HRESULT hr = S_OK; DWORD dwError = NO_ERROR; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::DoWinHttpWebSocketReceive"); + LOG_TRACE(L"WEBSOCKET_HANDLER::DoWinHttpWebSocketReceive"); IncrementOutstandingIo(); @@ -442,8 +435,7 @@ Routine Description: { DecrementOutstandingIo(); hr = HRESULT_FROM_WIN32(dwError); - DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::DoWinHttpWebSocketReceive failed with %08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::DoWinHttpWebSocketReceive failed with %08x", hr); } return hr; @@ -467,8 +459,7 @@ Routine Description: BOOL fFinalFragment = FALSE; BOOL fClose = FALSE; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::DoIisWebSocketSend %d", eBufferType); + LOG_TRACEF(L"WEBSOCKET_HANDLER::DoIisWebSocketSend %d", eBufferType); if (eBufferType == WINHTTP_WEB_SOCKET_CLOSE_BUFFER_TYPE) { @@ -558,8 +549,7 @@ Routine Description: Finished: if (FAILED_LOG(hr)) { - DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::DoIisWebSocketSend failed with %08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::DoIisWebSocketSend failed with %08x", hr); } return hr; @@ -581,8 +571,7 @@ Routine Description: DWORD dwError = NO_ERROR; HRESULT hr = S_OK; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::DoWinHttpWebSocketSend, %d", eBufferType); + LOG_TRACEF(L"WEBSOCKET_HANDLER::DoWinHttpWebSocketSend, %d", eBufferType); if (eBufferType == WINHTTP_WEB_SOCKET_CLOSE_BUFFER_TYPE) { @@ -627,8 +616,7 @@ Routine Description: // Call will complete asynchronously, return. // ignore error. // - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::DoWinhttpWebSocketSend IO_PENDING"); + LOG_TRACE(L"WEBSOCKET_HANDLER::DoWinhttpWebSocketSend IO_PENDING"); dwError = NO_ERROR; } @@ -639,8 +627,7 @@ Routine Description: // // Call completed synchronously. // - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::DoWinhttpWebSocketSend Shutdown successful."); + LOG_TRACE(L"WEBSOCKET_HANDLER::DoWinhttpWebSocketSend Shutdown successful."); } } } @@ -666,8 +653,7 @@ Routine Description: Finished: if (FAILED_LOG(hr)) { - DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::DoWinHttpWebSocketSend failed with %08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::DoWinHttpWebSocketSend failed with %08x", hr); } return hr; @@ -755,8 +741,7 @@ Routine Description: BOOL fLocked = FALSE; CleanupReason cleanupReason = CleanupReasonUnknown; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::OnWinHttpSendComplete"); + LOG_TRACE(L"WEBSOCKET_HANDLER::OnWinHttpSendComplete"); if (_fCleanupInProgress) { @@ -792,8 +777,7 @@ Finished: { Cleanup (cleanupReason); - DebugPrintf (ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::OnWinsockSendComplete failed with HR=%08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::OnWinsockSendComplete failed with HR=%08x", hr); } // @@ -810,8 +794,7 @@ WEBSOCKET_HANDLER::OnWinHttpShutdownComplete( VOID ) { - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::OnWinHttpShutdownComplete --%p", _pHandler); + LOG_TRACEF(L"WEBSOCKET_HANDLER::OnWinHttpShutdownComplete --%p", _pHandler); DecrementOutstandingIo(); @@ -825,8 +808,7 @@ WEBSOCKET_HANDLER::OnWinHttpIoError( { HRESULT hr = HRESULT_FROM_WIN32(pCompletionStatus->AsyncResult.dwError); - DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::OnWinHttpIoError HR = %08x, Operation = %d", + LOG_ERRORF(L"WEBSOCKET_HANDLER::OnWinHttpIoError HR = %08x, Operation = %d", hr, pCompletionStatus->AsyncResult.dwResult); Cleanup(ServerDisconnect); @@ -858,8 +840,7 @@ Routine Description: BOOL fLocked = FALSE; CleanupReason cleanupReason = CleanupReasonUnknown; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::OnWinHttpReceiveComplete --%p", _pHandler); + LOG_TRACEF(L"WEBSOCKET_HANDLER::OnWinHttpReceiveComplete --%p", _pHandler); if (_fCleanupInProgress) { @@ -893,8 +874,7 @@ Finished: { Cleanup (cleanupReason); - DebugPrintf (ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::OnWinsockReceiveComplete failed with HR=%08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::OnWinsockReceiveComplete failed with HR=%08x", hr); } // @@ -929,7 +909,7 @@ Routine Description: UNREFERENCED_PARAMETER(cbIo); - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, "WEBSOCKET_HANDLER::OnIisSendComplete"); + LOG_TRACE(L"WEBSOCKET_HANDLER::OnIisSendComplete"); if (FAILED_LOG(hrCompletion)) { @@ -974,8 +954,7 @@ Finished: { Cleanup (cleanupReason); - DebugPrintf (ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::OnIisSendComplete failed with HR=%08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::OnIisSendComplete failed with HR=%08x", hr); } // @@ -1014,8 +993,7 @@ Routine Description: CleanupReason cleanupReason = CleanupReasonUnknown; WINHTTP_WEB_SOCKET_BUFFER_TYPE BufferType; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::OnIisReceiveComplete"); + LOG_TRACE(L"WEBSOCKET_HANDLER::OnIisReceiveComplete"); if (FAILED_LOG(hrCompletion)) { @@ -1065,8 +1043,7 @@ Finished: { Cleanup (cleanupReason); - DebugPrintf (ASPNETCORE_DEBUG_FLAG_ERROR, - "WEBSOCKET_HANDLER::OnIisReceiveComplete failed with HR=%08x", hr); + LOG_ERRORF(L"WEBSOCKET_HANDLER::OnIisReceiveComplete failed with HR=%08x", hr); } // @@ -1097,8 +1074,7 @@ Arguments: --*/ { BOOL fLocked = FALSE; - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "WEBSOCKET_HANDLER::Cleanup Initiated with reason %d", reason); + LOG_TRACEF(L"WEBSOCKET_HANDLER::Cleanup Initiated with reason %d", reason); if (_fCleanupInProgress) { diff --git a/src/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp b/src/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp index 0af41d0f1c..1a087d5c95 100644 --- a/src/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp +++ b/src/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp @@ -55,8 +55,6 @@ REQUESTHANDLER_CONFIG::CreateRequestHandlerConfig( goto Finished; } - DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, - "REQUESTHANDLER_CONFIG::GetConfig, set config to ModuleContext"); // set appliction info here instead of inside Populate() // as the destructor will delete the backend process hr = pRequestHandlerConfig->QueryApplicationPath()->Copy(pHttpApplication->GetApplicationId()); diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs index 814ca5e14b..0e6b1cc932 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs @@ -88,7 +88,7 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS !IISDeploymentParameters.HandlerSettings.ContainsKey("debugFile")) { _debugLogFile = Path.GetTempFileName(); - IISDeploymentParameters.HandlerSettings["debugLevel"] = "4"; + IISDeploymentParameters.HandlerSettings["debugLevel"] = "file"; IISDeploymentParameters.HandlerSettings["debugFile"] = _debugLogFile; } diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs index 3cb2d40c3a..2d45f036a7 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeploymentParameterExtensions.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS { public static void AddDebugLogToWebConfig(this IISDeploymentParameters parameters, string filename) { - parameters.HandlerSettings["debugLevel"] = "4"; + parameters.HandlerSettings["debugLevel"] = "file"; parameters.HandlerSettings["debugFile"] = filename; } diff --git a/test/Common.FunctionalTests/Inprocess/LoggingTests.cs b/test/Common.FunctionalTests/Inprocess/LoggingTests.cs index dd0ac8cf6c..e574d4f96f 100644 --- a/test/Common.FunctionalTests/Inprocess/LoggingTests.cs +++ b/test/Common.FunctionalTests/Inprocess/LoggingTests.cs @@ -200,17 +200,44 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } } - private static void AssertLogs(string logPath) + [ConditionalFact] + public async Task DebugLogsAreWrittenToEventLog() + { + var deploymentParameters = _fixture.GetBaseDeploymentParameters(publish: true); + deploymentParameters.HandlerSettings["debugLevel"] = "file,eventlog"; + var deploymentResult = await StartAsync(deploymentParameters); + StopServer(); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, @"\[aspnetcorev2.dll\] Initializing logs for .*?Description: IIS ASP.NET Core Module V2"); + } + + [ConditionalFact] + public async Task OutOfProcessReadsLogConfiguration() + { + var deploymentParameters = _fixture.GetBaseDeploymentParameters(hostingModel:HostingModel.OutOfProcess, publish: true); + deploymentParameters.HandlerSettings["debugLevel"] = "file,trace"; + deploymentParameters.HandlerSettings["debugFile"] = ""; + var deploymentResult = await StartAsync(deploymentParameters); + + var logContents = ReadLogs(Path.Combine(deploymentResult.ContentRoot, "aspnetcore-debug.log")); + Assert.Contains("FORWARDING_HANDLER::", logContents); + } + + private static string ReadLogs(string logPath) { using (var stream = File.Open(logPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite)) using (var streamReader = new StreamReader(stream)) { - var logContents = streamReader.ReadToEnd(); - Assert.Contains("[aspnetcorev2.dll]", logContents); - Assert.Contains("[aspnetcorev2_inprocess.dll]", logContents); - Assert.Contains("Description: IIS ASP.NET Core Module V2. Commit:", logContents); - Assert.Contains("Description: IIS ASP.NET Core Module V2 Request Handler. Commit:", logContents); + return streamReader.ReadToEnd(); } } + + private static void AssertLogs(string logPath) + { + var logContents = ReadLogs(logPath); + Assert.Contains("[aspnetcorev2.dll]", logContents); + Assert.Contains("[aspnetcorev2_inprocess.dll]", logContents); + Assert.Contains("Description: IIS ASP.NET Core Module V2. Commit:", logContents); + Assert.Contains("Description: IIS ASP.NET Core Module V2 Request Handler. Commit:", logContents); + } } } From 71b7bb50b28409614d730c7965ff7035534b1974 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 5 Sep 2018 17:21:59 -0700 Subject: [PATCH 16/18] Ensure external processes are killed when test process exits (#1371) - Prevents locked files when stop debugging unit test - Addresses #1279 --- .../IISDeployer.cs | 5 + .../IISExpressDeployer.cs | 6 + .../ProcessTracker.cs | 128 ++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/ProcessTracker.cs diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs index 0e6b1cc932..b1394bccb5 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISDeployer.cs @@ -254,6 +254,11 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS } HostProcess = Process.GetProcessById(workerProcess.ProcessId); + + // Ensure w3wp.exe is killed if test process termination is non-graceful. + // Prevents locked files when stop debugging unit test. + ProcessTracker.Add(HostProcess); + // cache the process start time for verifying log file name. var _ = HostProcess.StartTime; diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs index 4604fb5652..56d4afb46b 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/IISExpressDeployer.cs @@ -249,8 +249,14 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS else { _hostProcess = process; + + // Ensure iisexpress.exe is killed if test process termination is non-graceful. + // Prevents locked files when stop debugging unit test. + ProcessTracker.Add(_hostProcess); + // cache the process start time for verifying log file name. var _ = _hostProcess.StartTime; + Logger.LogInformation("Started iisexpress successfully. Process Id : {processId}, Port: {port}", _hostProcess.Id, port); return (url: url, hostExitToken: hostExitTokenSource.Token); } diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/ProcessTracker.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/ProcessTracker.cs new file mode 100644 index 0000000000..be2e6cc62b --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting.IIS/ProcessTracker.cs @@ -0,0 +1,128 @@ +// 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.ComponentModel; +using System.Diagnostics; +using System.Runtime.InteropServices; + +namespace Microsoft.AspNetCore.Server.IntegrationTesting.IIS +{ + // Uses Windows Job Objects to ensure external processes are killed if the current process is terminated non-gracefully. + internal static class ProcessTracker + { + private static readonly IntPtr _jobHandle; + + static ProcessTracker() + { + // Requires Win8 or later + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || Environment.OSVersion.Version < new Version(6, 2)) + { + return; + } + + // Job name is optional but helps with diagnostics. Job name must be unique if non-null. + _jobHandle = CreateJobObject(IntPtr.Zero, name: $"ProcessTracker{Process.GetCurrentProcess().Id}"); + + var extendedInfo = new JOBOBJECT_EXTENDED_LIMIT_INFORMATION + { + BasicLimitInformation = new JOBOBJECT_BASIC_LIMIT_INFORMATION + { + LimitFlags = JOBOBJECTLIMIT.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + } + }; + + var length = Marshal.SizeOf(typeof(JOBOBJECT_EXTENDED_LIMIT_INFORMATION)); + var extendedInfoPtr = Marshal.AllocHGlobal(length); + try + { + Marshal.StructureToPtr(extendedInfo, extendedInfoPtr, false); + + if (!SetInformationJobObject(_jobHandle, JobObjectInfoType.ExtendedLimitInformation, + extendedInfoPtr, (uint)length)) + { + throw new Win32Exception(); + } + } + finally + { + Marshal.FreeHGlobal(extendedInfoPtr); + } + } + + public static void Add(Process process) + { + if (_jobHandle != IntPtr.Zero) + { + var success = AssignProcessToJobObject(_jobHandle, process.Handle); + if (!success && !process.HasExited) + { + throw new Win32Exception(); + } + } + } + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode)] + static extern IntPtr CreateJobObject(IntPtr lpJobAttributes, string name); + + [DllImport("kernel32.dll")] + static extern bool SetInformationJobObject(IntPtr job, JobObjectInfoType infoType, + IntPtr lpJobObjectInfo, uint cbJobObjectInfoLength); + + [DllImport("kernel32.dll", SetLastError = true)] + static extern bool AssignProcessToJobObject(IntPtr job, IntPtr process); + + private enum JobObjectInfoType + { + AssociateCompletionPortInformation = 7, + BasicLimitInformation = 2, + BasicUIRestrictions = 4, + EndOfJobTimeInformation = 6, + ExtendedLimitInformation = 9, + SecurityLimitInformation = 5, + GroupInformation = 11 + } + + [StructLayout(LayoutKind.Sequential)] + private struct JOBOBJECT_BASIC_LIMIT_INFORMATION + { + public Int64 PerProcessUserTimeLimit; + public Int64 PerJobUserTimeLimit; + public JOBOBJECTLIMIT LimitFlags; + public UIntPtr MinimumWorkingSetSize; + public UIntPtr MaximumWorkingSetSize; + public UInt32 ActiveProcessLimit; + public Int64 Affinity; + public UInt32 PriorityClass; + public UInt32 SchedulingClass; + } + + [Flags] + private enum JOBOBJECTLIMIT : uint + { + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x2000 + } + + [StructLayout(LayoutKind.Sequential)] + private struct IO_COUNTERS + { + public UInt64 ReadOperationCount; + public UInt64 WriteOperationCount; + public UInt64 OtherOperationCount; + public UInt64 ReadTransferCount; + public UInt64 WriteTransferCount; + public UInt64 OtherTransferCount; + } + + [StructLayout(LayoutKind.Sequential)] + private struct JOBOBJECT_EXTENDED_LIMIT_INFORMATION + { + public JOBOBJECT_BASIC_LIMIT_INFORMATION BasicLimitInformation; + public IO_COUNTERS IoInfo; + public UIntPtr ProcessMemoryLimit; + public UIntPtr JobMemoryLimit; + public UIntPtr PeakProcessMemoryUsed; + public UIntPtr PeakJobMemoryUsed; + } + } +} From b1f0f173aa937bed8910bd42ad92436c0de1b830 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Thu, 6 Sep 2018 10:10:07 -0700 Subject: [PATCH 17/18] Use ConnectionResetException with 'The client disconnected' message (#1364) --- build/dependencies.props | 1 + .../Core/IO/AsyncIOOperation.cs | 4 +- .../Microsoft.AspNetCore.Server.IIS.csproj | 3 +- test/IIS.Tests/ClientDisconnectTests.cs | 50 +++++++++++++++++-- test/IIS.Tests/TestServerTest.cs | 22 ++++---- 5 files changed, 62 insertions(+), 18 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 3ecafbac3f..5418510639 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -8,6 +8,7 @@ 2.2.0-preview2-35143 2.2.0-preview2-35143 2.2.0-preview2-35143 + 2.2.0-preview2-35143 2.2.0-preview2-35143 2.2.0-preview2-35143 2.2.0-preview2-35143 diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IO/AsyncIOOperation.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IO/AsyncIOOperation.cs index 9d3752ef08..80e9234ea2 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IO/AsyncIOOperation.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IO/AsyncIOOperation.cs @@ -8,6 +8,7 @@ using System.IO; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks.Sources; +using Microsoft.AspNetCore.Connections; namespace Microsoft.AspNetCore.Server.IIS.Core.IO { @@ -103,7 +104,8 @@ namespace Microsoft.AspNetCore.Server.IIS.Core.IO _result = bytes; if (hr != NativeMethods.HR_OK) { - _exception = new IOException("Native IO operation failed", Marshal.GetExceptionForHR(hr)); + // Treat all errors as the client disconnect + _exception = new ConnectionResetException("The client has disconnected", Marshal.GetExceptionForHR(hr)); } } else diff --git a/src/Microsoft.AspNetCore.Server.IIS/Microsoft.AspNetCore.Server.IIS.csproj b/src/Microsoft.AspNetCore.Server.IIS/Microsoft.AspNetCore.Server.IIS.csproj index f9c6fbbd6f..d82d80f9a6 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Microsoft.AspNetCore.Server.IIS.csproj +++ b/src/Microsoft.AspNetCore.Server.IIS/Microsoft.AspNetCore.Server.IIS.csproj @@ -1,4 +1,4 @@ - + netstandard2.0 @@ -24,6 +24,7 @@ + diff --git a/test/IIS.Tests/ClientDisconnectTests.cs b/test/IIS.Tests/ClientDisconnectTests.cs index 72e7adc5bb..c1b2c25062 100644 --- a/test/IIS.Tests/ClientDisconnectTests.cs +++ b/test/IIS.Tests/ClientDisconnectTests.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.Logging.Testing; @@ -17,7 +18,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests public class ClientDisconnectTests : LoggedTest { - [ConditionalFact(Skip = "See: https://github.com/aspnet/IISIntegration/issues/1075")] + [ConditionalFact] public async Task WritesSucceedAfterClientDisconnect() { var requestStartedCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -49,7 +50,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } } - [ConditionalFact(Skip = "See: https://github.com/aspnet/IISIntegration/issues/1075")] + [ConditionalFact] public async Task ReadThrowsAfterClientDisconnect() { var requestStartedCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -82,8 +83,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests await requestCompletedCompletionSource.Task.TimeoutAfterDefault(); } - Assert.IsType(exception); - Assert.Equal("Native IO operation failed", exception.Message); + Assert.IsType(exception); + Assert.Equal("The client has disconnected", exception.Message); } [ConditionalFact(Skip = "See: https://github.com/aspnet/IISIntegration/issues/1075")] @@ -126,7 +127,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.IsType(exception); } } - [ConditionalFact(Skip = "See: https://github.com/aspnet/IISIntegration/issues/1075")] public async Task ReaderThrowsCancelledException() { @@ -163,6 +163,46 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } } + [ConditionalFact] + public async Task ReaderThrowsResetExceptionOnInvalidBody() + { + var requestCompletedCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + Exception exception = null; + + var data = new byte[1024]; + using (var testServer = await TestServer.Create(async ctx => + { + try + { + await ctx.Request.Body.ReadAsync(data); + } + catch (Exception e) + { + exception = e; + } + + requestCompletedCompletionSource.SetResult(true); + }, LoggerFactory)) + { + using (var connection = testServer.CreateConnection()) + { + await connection.Send( + "POST / HTTP/1.1", + "Transfer-Encoding: chunked", + "Host: localhost", + "Connection: close", + "", + "ZZZ", + ""); + await requestCompletedCompletionSource.Task.TimeoutAfterDefault(); + } + } + + Assert.IsType(exception); + Assert.Equal("The client has disconnected", exception.Message); + } + private static async Task SendContentLength1Post(TestConnection connection) { await connection.Send( diff --git a/test/IIS.Tests/TestServerTest.cs b/test/IIS.Tests/TestServerTest.cs index 79b1c42113..c168af99d8 100644 --- a/test/IIS.Tests/TestServerTest.cs +++ b/test/IIS.Tests/TestServerTest.cs @@ -19,17 +19,17 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests var helloWorld = "Hello World"; var expectedPath = "/Path"; - string path = null; - using (var testServer = await TestServer.Create(ctx => - { - path = ctx.Request.Path.ToString(); - return ctx.Response.WriteAsync(helloWorld); - }, LoggerFactory)) - { - var result = await testServer.HttpClient.GetAsync(expectedPath); - Assert.Equal(helloWorld, await result.Content.ReadAsStringAsync()); - Assert.Equal(expectedPath, path); - } + string path = null; + using (var testServer = await TestServer.Create(ctx => + { + path = ctx.Request.Path.ToString(); + return ctx.Response.WriteAsync(helloWorld); + }, LoggerFactory)) + { + var result = await testServer.HttpClient.GetAsync(expectedPath); + Assert.Equal(helloWorld, await result.Content.ReadAsStringAsync()); + Assert.Equal(expectedPath, path); } } + } } From 1fca011674f920e6bbb9d9f19c83afd1cf8c6c7e Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 6 Sep 2018 10:32:49 -0700 Subject: [PATCH 18/18] Fix output location for lib files (#1373) --- src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj | 4 ++++ .../RequestHandlerLib/RequestHandlerLib.vcxproj | 4 ++++ test/gtest/gtest.vcxproj | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj index 20e37c30f3..5e430905e1 100644 --- a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj +++ b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj @@ -72,16 +72,20 @@ true + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ true + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ false + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ false C:\AspNetCoreModule\src\IISLib;$(IncludePath) + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ diff --git a/src/AspNetCoreModuleV2/RequestHandlerLib/RequestHandlerLib.vcxproj b/src/AspNetCoreModuleV2/RequestHandlerLib/RequestHandlerLib.vcxproj index a2c89e2b40..2519c101b1 100644 --- a/src/AspNetCoreModuleV2/RequestHandlerLib/RequestHandlerLib.vcxproj +++ b/src/AspNetCoreModuleV2/RequestHandlerLib/RequestHandlerLib.vcxproj @@ -72,16 +72,20 @@ true + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ true + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ false + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ false C:\AspNetCoreModule\src\IISLib;$(IncludePath) + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ diff --git a/test/gtest/gtest.vcxproj b/test/gtest/gtest.vcxproj index 9f745f3a84..924f26337d 100644 --- a/test/gtest/gtest.vcxproj +++ b/test/gtest/gtest.vcxproj @@ -82,21 +82,25 @@ true $(VC_IncludePath);$(WindowsSDK_IncludePath); $(VC_SourcePath); + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ true $(VC_IncludePath);$(WindowsSDK_IncludePath); $(VC_SourcePath); + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ false $(VC_IncludePath);$(WindowsSDK_IncludePath); $(VC_SourcePath); + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\ false $(VC_IncludePath);$(WindowsSDK_IncludePath); $(VC_SourcePath); + $(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\