From e26231b61314b35571c99f032ae7353e72bb4056 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 22 Aug 2018 08:23:40 -0700 Subject: [PATCH 1/4] Cleanup application manager (#1250) --- .../AspNetCore/ApplicationFactory.h | 46 +++ .../AspNetCore/AspNetCore.vcxproj | 1 + .../AspNetCore/HandlerResolver.cpp | 194 ++++++------ .../AspNetCore/HandlerResolver.h | 34 +- .../AspNetCore/applicationinfo.cpp | 72 +---- .../AspNetCore/applicationinfo.h | 158 +++------- .../AspNetCore/applicationmanager.cpp | 290 ++++++------------ .../AspNetCore/applicationmanager.h | 57 +--- .../AspNetCore/aspnetcore_shim_config.cpp | 14 +- .../AspNetCore/aspnetcore_shim_config.h | 23 +- .../AspNetCore/proxymodule.cpp | 21 +- .../AspNetCore/proxymodule.h | 6 +- .../CommonLib/HandleWrapper.h | 7 + .../CommonLib/debugutil.cpp | 2 +- .../CommonLib/file_utility.cpp | 31 -- .../CommonLib/file_utility.h | 6 - .../CommonLib/iapplication.h | 4 +- .../OutOfProcessRequestHandler/dllmain.cpp | 2 + .../ConfigurationChangeTests.cs | 52 +++- test/CommonLibTests/hostfxr_utility_tests.cpp | 4 +- 20 files changed, 394 insertions(+), 630 deletions(-) create mode 100644 src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h diff --git a/src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h b/src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h new file mode 100644 index 0000000000..380b0de6cc --- /dev/null +++ b/src/AspNetCoreModuleV2/AspNetCore/ApplicationFactory.h @@ -0,0 +1,46 @@ +// 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 +#include +#include "iapplication.h" +#include "HandleWrapper.h" + +typedef +HRESULT +(WINAPI * PFN_ASPNETCORE_CREATE_APPLICATION)( + _In_ IHttpServer *pServer, + _In_ IHttpApplication *pHttpApplication, + _In_ APPLICATION_PARAMETER *pParameters, + _In_ DWORD nParameters, + _Out_ IAPPLICATION **pApplication + ); + +class ApplicationFactory +{ +public: + ApplicationFactory(HMODULE hRequestHandlerDll, std::wstring location, PFN_ASPNETCORE_CREATE_APPLICATION pfnAspNetCoreCreateApplication): + m_pfnAspNetCoreCreateApplication(pfnAspNetCoreCreateApplication), + m_location(location), + m_hRequestHandlerDll(hRequestHandlerDll) + { + } + + HRESULT Execute( + _In_ IHttpServer *pServer, + _In_ IHttpApplication *pHttpApplication, + _Out_ IAPPLICATION **pApplication) const + { + std::array parameters { + {"InProcessExeLocation", reinterpret_cast(m_location.data())} + }; + return m_pfnAspNetCoreCreateApplication(pServer, pHttpApplication, parameters.data(), static_cast(parameters.size()), pApplication); + } + +private: + PFN_ASPNETCORE_CREATE_APPLICATION m_pfnAspNetCoreCreateApplication; + std::wstring m_location; + HandleWrapper m_hRequestHandlerDll; +}; diff --git a/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj b/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj index 9663eb06ce..b809cdeadc 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj +++ b/src/AspNetCoreModuleV2/AspNetCore/AspNetCore.vcxproj @@ -227,6 +227,7 @@ + diff --git a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp index 49d1a29093..83c34a30c0 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp @@ -19,56 +19,59 @@ const PCWSTR HandlerResolver::s_pwzAspnetcoreOutOfProcessRequestHandlerName = L" HandlerResolver::HandlerResolver(HMODULE hModule, IHttpServer &pServer) : m_hModule(hModule), m_pServer(pServer), - m_fAspnetcoreRHLoadResult(S_FALSE), - m_loadedApplicationHostingModel(HOSTING_UNKNOWN), - m_hRequestHandlerDll(nullptr), - m_pfnAspNetCoreCreateApplication(nullptr) + m_loadedApplicationHostingModel(HOSTING_UNKNOWN) { InitializeSRWLock(&m_requestHandlerLoadLock); } HRESULT -HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, STRU& location, ASPNETCORE_SHIM_CONFIG * pConfiguration) +HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, ASPNETCORE_SHIM_CONFIG& pConfiguration, std::unique_ptr& pApplicationFactory) { HRESULT hr; - STACK_STRU(struFileName, MAX_PATH); PCWSTR pstrHandlerDllName; - if (pConfiguration->QueryHostingModel() == APP_HOSTING_MODEL::HOSTING_IN_PROCESS) + bool preventUnload; + if (pConfiguration.QueryHostingModel() == APP_HOSTING_MODEL::HOSTING_IN_PROCESS) { + preventUnload = false; pstrHandlerDllName = s_pwzAspnetcoreInProcessRequestHandlerName; } else { + // OutOfProcess handler is not able to handle unload correctly + // It has code running after application.Stop exits + preventUnload = true; pstrHandlerDllName = s_pwzAspnetcoreOutOfProcessRequestHandlerName; } - + HandleWrapper hRequestHandlerDll; + std::wstring location; + std::wstring handlerDllPath; // Try to see if RH is already loaded, use GetModuleHandleEx to increment ref count - if (!GetModuleHandleEx(0, pstrHandlerDllName, &m_hRequestHandlerDll)) + if (!GetModuleHandleEx(0, pstrHandlerDllName, &hRequestHandlerDll)) { - if (pConfiguration->QueryHostingModel() == APP_HOSTING_MODEL::HOSTING_IN_PROCESS) + if (pConfiguration.QueryHostingModel() == APP_HOSTING_MODEL::HOSTING_IN_PROCESS) { std::unique_ptr options; std::unique_ptr outputManager; RETURN_IF_FAILED(HOSTFXR_OPTIONS::Create( NULL, - pConfiguration->QueryProcessPath()->QueryStr(), + pConfiguration.QueryProcessPath().c_str(), pApplication.GetApplicationPhysicalPath(), - pConfiguration->QueryArguments()->QueryStr(), + pConfiguration.QueryArguments().c_str(), options)); - RETURN_IF_FAILED(location.Copy(options->GetExeLocation())); + location = options->GetExeLocation(); RETURN_IF_FAILED(LoggingHelpers::CreateLoggingProvider( - pConfiguration->QueryStdoutLogEnabled(), + pConfiguration.QueryStdoutLogEnabled(), !m_pServer.IsCommandLineLaunch(), - pConfiguration->QueryStdoutLogFile()->QueryStr(), + pConfiguration.QueryStdoutLogFile()->QueryStr(), pApplication.GetApplicationPhysicalPath(), outputManager)); outputManager->Start(); - hr = FindNativeAssemblyFromHostfxr(options.get(), pstrHandlerDllName, &struFileName); + hr = FindNativeAssemblyFromHostfxr(*options.get(), pstrHandlerDllName, handlerDllPath); outputManager->Stop(); if (FAILED(hr) && m_hHostFxrDll != NULL) @@ -85,85 +88,99 @@ HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, STRU EventLog::Error( ASPNETCORE_EVENT_GENERAL_ERROR, ASPNETCORE_EVENT_INPROCESS_RH_ERROR_MSG, - struFileName.IsEmpty() ? s_pwzAspnetcoreInProcessRequestHandlerName : struFileName.QueryStr(), + handlerDllPath.empty()? s_pwzAspnetcoreInProcessRequestHandlerName : handlerDllPath.c_str(), struStdMsg.QueryStr()); } } else { - if (FAILED_LOG(hr = FindNativeAssemblyFromGlobalLocation(pConfiguration, pstrHandlerDllName, &struFileName))) + if (FAILED_LOG(hr = FindNativeAssemblyFromGlobalLocation(pConfiguration, pstrHandlerDllName, handlerDllPath))) { EventLog::Error( ASPNETCORE_EVENT_OUT_OF_PROCESS_RH_MISSING, ASPNETCORE_EVENT_OUT_OF_PROCESS_RH_MISSING_MSG, - struFileName.IsEmpty() ? s_pwzAspnetcoreOutOfProcessRequestHandlerName : struFileName.QueryStr()); + handlerDllPath.empty() ? s_pwzAspnetcoreOutOfProcessRequestHandlerName : handlerDllPath.c_str()); return hr; } } - LOG_INFOF("Loading request handler: %S", struFileName.QueryStr()); + LOG_INFOF("Loading request handler: %S", handlerDllPath.c_str()); - m_hRequestHandlerDll = LoadLibraryW(struFileName.QueryStr()); - RETURN_LAST_ERROR_IF_NULL(m_hRequestHandlerDll); + hRequestHandlerDll = LoadLibrary(handlerDllPath.c_str()); + if (preventUnload) + { + // Pin module in memory + GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_PIN, pstrHandlerDllName, &hRequestHandlerDll); + } + RETURN_LAST_ERROR_IF_NULL(hRequestHandlerDll); } - m_pfnAspNetCoreCreateApplication = reinterpret_cast(GetProcAddress(m_hRequestHandlerDll, "CreateApplication")); - - RETURN_LAST_ERROR_IF_NULL(m_pfnAspNetCoreCreateApplication); + auto pfnAspNetCoreCreateApplication = reinterpret_cast(GetProcAddress(hRequestHandlerDll, "CreateApplication")); + RETURN_LAST_ERROR_IF_NULL(pfnAspNetCoreCreateApplication); + pApplicationFactory = std::make_unique(hRequestHandlerDll.release(), location, pfnAspNetCoreCreateApplication); return S_OK; } HRESULT -HandlerResolver::GetApplicationFactory(IHttpApplication &pApplication, STRU& location, PFN_ASPNETCORE_CREATE_APPLICATION * pfnCreateApplication) +HandlerResolver::GetApplicationFactory(IHttpApplication &pApplication, std::unique_ptr& pApplicationFactory) { - ASPNETCORE_SHIM_CONFIG pConfiguration; - RETURN_IF_FAILED(pConfiguration.Populate(&m_pServer, &pApplication)); - - if (m_fAspnetcoreRHLoadResult == S_FALSE) + try { + ASPNETCORE_SHIM_CONFIG pConfiguration; + RETURN_IF_FAILED(pConfiguration.Populate(&m_pServer, &pApplication)); + SRWExclusiveLock lock(m_requestHandlerLoadLock); - if (m_fAspnetcoreRHLoadResult == S_FALSE) + if (m_loadedApplicationHostingModel != HOSTING_UNKNOWN) { - m_loadedApplicationHostingModel = pConfiguration.QueryHostingModel(); - m_loadedApplicationId = pApplication.GetApplicationId(); - LOG_IF_FAILED(m_fAspnetcoreRHLoadResult = LoadRequestHandlerAssembly(pApplication, location, &pConfiguration)); + // Mixed hosting models + if (m_loadedApplicationHostingModel != pConfiguration.QueryHostingModel()) + { + EventLog::Error( + ASPNETCORE_EVENT_MIXED_HOSTING_MODEL_ERROR, + ASPNETCORE_EVENT_MIXED_HOSTING_MODEL_ERROR_MSG, + pApplication.GetApplicationId(), + pConfiguration.QueryHostingModel()); + + return E_FAIL; + } + // Multiple in-process apps + if (m_loadedApplicationHostingModel == HOSTING_IN_PROCESS && m_loadedApplicationId != pApplication.GetApplicationId()) + { + EventLog::Error( + ASPNETCORE_EVENT_DUPLICATED_INPROCESS_APP, + ASPNETCORE_EVENT_DUPLICATED_INPROCESS_APP_MSG, + pApplication.GetApplicationId()); + + return E_FAIL; + } } + + m_loadedApplicationHostingModel = pConfiguration.QueryHostingModel(); + m_loadedApplicationId = pApplication.GetApplicationId(); + RETURN_IF_FAILED(LoadRequestHandlerAssembly(pApplication, pConfiguration, pApplicationFactory)); + } + CATCH_RETURN(); - // Mixed hosting models - if (m_loadedApplicationHostingModel != pConfiguration.QueryHostingModel()) - { - EventLog::Error( - ASPNETCORE_EVENT_MIXED_HOSTING_MODEL_ERROR, - ASPNETCORE_EVENT_MIXED_HOSTING_MODEL_ERROR_MSG, - pApplication.GetApplicationId(), - pConfiguration.QueryHostingModel()); + return S_OK; +} - return E_FAIL; - } - // Multiple in-process apps - else if (m_loadedApplicationHostingModel == HOSTING_IN_PROCESS && m_loadedApplicationId != pApplication.GetApplicationId()) - { - EventLog::Error( - ASPNETCORE_EVENT_DUPLICATED_INPROCESS_APP, - ASPNETCORE_EVENT_DUPLICATED_INPROCESS_APP_MSG, - pApplication.GetApplicationId()); +void HandlerResolver::ResetHostingModel() +{ + SRWExclusiveLock lock(m_requestHandlerLoadLock); - return E_FAIL; - } - - *pfnCreateApplication = m_pfnAspNetCoreCreateApplication; - return m_fAspnetcoreRHLoadResult; + m_loadedApplicationHostingModel = APP_HOSTING_MODEL::HOSTING_UNKNOWN; + m_loadedApplicationId.resize(0); } HRESULT HandlerResolver::FindNativeAssemblyFromGlobalLocation( - ASPNETCORE_SHIM_CONFIG * pConfiguration, + ASPNETCORE_SHIM_CONFIG& pConfiguration, PCWSTR pstrHandlerDllName, - STRU* struFilename + std::wstring& handlerDllPath ) { try @@ -172,12 +189,10 @@ HandlerResolver::FindNativeAssemblyFromGlobalLocation( modulePath = GlobalVersionUtility::RemoveFileNameFromFolderPath(modulePath); - std::wstring retval = GlobalVersionUtility::GetGlobalRequestHandlerPath(modulePath.c_str(), - pConfiguration->QueryHandlerVersion()->QueryStr(), + handlerDllPath = GlobalVersionUtility::GetGlobalRequestHandlerPath(modulePath.c_str(), + pConfiguration.QueryHandlerVersion().c_str(), pstrHandlerDllName ); - - RETURN_IF_FAILED(struFilename->Copy(retval.c_str())); } catch (...) { @@ -199,25 +214,18 @@ HandlerResolver::FindNativeAssemblyFromGlobalLocation( // HRESULT HandlerResolver::FindNativeAssemblyFromHostfxr( - HOSTFXR_OPTIONS* hostfxrOptions, + HOSTFXR_OPTIONS& hostfxrOptions, PCWSTR libraryName, - STRU* struFilename + std::wstring& handlerDllPath ) { - STRU struApplicationFullPath; - STRU struNativeSearchPaths; - STRU struNativeDllLocation; - INT intIndex = -1; - INT intPrevIndex = 0; - BOOL fFound = FALSE; - DWORD dwBufferSize = 1024 * 10; - DWORD dwRequiredBufferSize = 0; - STRA output; + std::wstring struNativeSearchPaths; + size_t intIndex; + size_t intPrevIndex = 0; + DWORD dwBufferSize = s_initialGetNativeSearchDirectoriesBufferSize; + DWORD dwRequiredBufferSize = 0; - - DBG_ASSERT(struFilename != NULL); - - RETURN_LAST_ERROR_IF_NULL(m_hHostFxrDll = LoadLibraryW(hostfxrOptions->GetHostFxrLocation())); + RETURN_LAST_ERROR_IF_NULL(m_hHostFxrDll = LoadLibraryW(hostfxrOptions.GetHostFxrLocation())); auto pFnHostFxrSearchDirectories = reinterpret_cast(GetProcAddress(m_hHostFxrDll, "hostfxr_get_native_search_directories")); if (pFnHostFxrSearchDirectories == nullptr) @@ -225,20 +233,20 @@ HandlerResolver::FindNativeAssemblyFromHostfxr( EventLog::Error( ASPNETCORE_EVENT_GENERAL_ERROR, ASPNETCORE_EVENT_HOSTFXR_DLL_INVALID_VERSION_MSG, - hostfxrOptions->GetHostFxrLocation() + hostfxrOptions.GetHostFxrLocation() ); RETURN_IF_FAILED(E_FAIL); } RETURN_LAST_ERROR_IF_NULL(pFnHostFxrSearchDirectories); - RETURN_IF_FAILED(struNativeSearchPaths.Resize(dwBufferSize)); + struNativeSearchPaths.resize(dwBufferSize); while (TRUE) { - auto intHostFxrExitCode = pFnHostFxrSearchDirectories( - hostfxrOptions->GetArgc(), - hostfxrOptions->GetArgv(), - struNativeSearchPaths.QueryStr(), + const auto intHostFxrExitCode = pFnHostFxrSearchDirectories( + hostfxrOptions.GetArgc(), + hostfxrOptions.GetArgv(), + struNativeSearchPaths.data(), dwBufferSize, &dwRequiredBufferSize ); @@ -251,7 +259,7 @@ HandlerResolver::FindNativeAssemblyFromHostfxr( { dwBufferSize = dwRequiredBufferSize + 1; // for null terminator - RETURN_IF_FAILED(struNativeSearchPaths.Resize(dwBufferSize)); + struNativeSearchPaths.resize(dwBufferSize); } else { @@ -260,26 +268,26 @@ HandlerResolver::FindNativeAssemblyFromHostfxr( } } - RETURN_IF_FAILED(struNativeSearchPaths.SyncWithBuffer()); + struNativeSearchPaths.resize(struNativeSearchPaths.find(L'\0')); - fFound = FALSE; + auto fFound = FALSE; // The native search directories are semicolon delimited. // Split on semicolons, append aspnetcorerh.dll, and check if the file exists. - while ((intIndex = struNativeSearchPaths.IndexOf(L";", intPrevIndex)) != -1) + while ((intIndex = struNativeSearchPaths.find(L';', intPrevIndex)) != std::wstring::npos) { - RETURN_IF_FAILED(struNativeDllLocation.Copy(&struNativeSearchPaths.QueryStr()[intPrevIndex], intIndex - intPrevIndex)); + auto path = struNativeSearchPaths.substr(intPrevIndex, intIndex - intPrevIndex); - if (!struNativeDllLocation.EndsWith(L"\\")) + if (!path.empty() && !(path[path.length() - 1] == L'\\')) { - RETURN_IF_FAILED(struNativeDllLocation.Append(L"\\")); + path.append(L"\\"); } - RETURN_IF_FAILED(struNativeDllLocation.Append(libraryName)); + path.append(libraryName); - if (FILE_UTILITY::CheckIfFileExists(struNativeDllLocation.QueryStr())) + if (std::filesystem::is_regular_file(path)) { - RETURN_IF_FAILED(struFilename->Copy(struNativeDllLocation)); + handlerDllPath = path; fFound = TRUE; break; } diff --git a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h index 847d664956..101b1ad2a0 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h +++ b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.h @@ -3,47 +3,35 @@ #pragma once #include "aspnetcore_shim_config.h" -#include "hostfxroptions.h" -#include -#include "iapplication.h" -#include -#include "HandleWrapper.h" -typedef -HRESULT -(WINAPI * PFN_ASPNETCORE_CREATE_APPLICATION)( - _In_ IHttpServer *pServer, - _In_ IHttpApplication *pHttpApplication, - _In_ APPLICATION_PARAMETER *pParameters, - _In_ DWORD nParameters, - _Out_ IAPPLICATION **pApplication - ); +#include +#include +#include "hostfxroptions.h" +#include "HandleWrapper.h" +#include "ApplicationFactory.h" class HandlerResolver { public: HandlerResolver(HMODULE hModule, IHttpServer &pServer); - HRESULT GetApplicationFactory(IHttpApplication &pApplication, STRU& location, PFN_ASPNETCORE_CREATE_APPLICATION *pfnCreateApplication); + HRESULT GetApplicationFactory(IHttpApplication &pApplication, std::unique_ptr& pApplicationFactory); + void ResetHostingModel(); private: - HRESULT LoadRequestHandlerAssembly(IHttpApplication &pApplication,STRU& location, ASPNETCORE_SHIM_CONFIG * pConfiguration); - HRESULT FindNativeAssemblyFromGlobalLocation(ASPNETCORE_SHIM_CONFIG * pConfiguration, PCWSTR libraryName, STRU* location); - HRESULT FindNativeAssemblyFromHostfxr(HOSTFXR_OPTIONS* hostfxrOptions, PCWSTR libraryName, STRU* location); + HRESULT LoadRequestHandlerAssembly(IHttpApplication &pApplication, ASPNETCORE_SHIM_CONFIG& pConfiguration, std::unique_ptr& pApplicationFactory); + HRESULT FindNativeAssemblyFromGlobalLocation(ASPNETCORE_SHIM_CONFIG& pConfiguration, PCWSTR libraryName, std::wstring& handlerDllPath); + HRESULT FindNativeAssemblyFromHostfxr(HOSTFXR_OPTIONS& hostfxrOptions, PCWSTR libraryName, std::wstring& handlerDllPath); HMODULE m_hModule; IHttpServer &m_pServer; SRWLOCK m_requestHandlerLoadLock {}; - // S_FALSE - not loaded, S_OK - loaded, everything else - error - HRESULT m_fAspnetcoreRHLoadResult; std::wstring m_loadedApplicationId; APP_HOSTING_MODEL m_loadedApplicationHostingModel; - HandleWrapper m_hRequestHandlerDll; HandleWrapper m_hHostFxrDll; - PFN_ASPNETCORE_CREATE_APPLICATION m_pfnAspNetCoreCreateApplication; - static const PCWSTR s_pwzAspnetcoreInProcessRequestHandlerName; static const PCWSTR s_pwzAspnetcoreOutOfProcessRequestHandlerName; + static const DWORD s_initialGetNativeSearchDirectoriesBufferSize = MAX_PATH * 4; }; diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp index 2a00d3c82c..e250045677 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp @@ -3,7 +3,6 @@ #include "applicationinfo.h" -#include #include "proxymodule.h" #include "hostfxr_utility.h" #include "debugutil.h" @@ -11,28 +10,14 @@ #include "SRWExclusiveLock.h" #include "exceptions.h" #include "EventLog.h" -#include "HandleWrapper.h" #include "ServerErrorApplication.h" #include "AppOfflineApplication.h" APPLICATION_INFO::~APPLICATION_INFO() { - ShutDownApplication(); + ShutDownApplication(/* fServerInitiated */ false); } -HRESULT -APPLICATION_INFO::Initialize( - _In_ IHttpApplication &pApplication, - HandlerResolver * pHandlerResolver -) -{ - m_handlerResolver = pHandlerResolver; - RETURN_IF_FAILED(m_struConfigPath.Copy(pApplication.GetAppConfigPath())); - RETURN_IF_FAILED(m_struInfoKey.Copy(pApplication.GetApplicationId())); - return S_OK; -} - - HRESULT APPLICATION_INFO::GetOrCreateApplication( IHttpContext *pHttpContext, @@ -54,6 +39,7 @@ APPLICATION_INFO::GetOrCreateApplication( // Call to wait for application to complete stopping m_pApplication->Stop(/* fServerInitiated */ false); m_pApplication = nullptr; + m_pApplicationFactory = nullptr; } else { @@ -69,20 +55,13 @@ APPLICATION_INFO::GetOrCreateApplication( } else { - STRU struExeLocation; - PFN_ASPNETCORE_CREATE_APPLICATION pfnAspNetCoreCreateApplication; - FINISHED_IF_FAILED(m_handlerResolver->GetApplicationFactory(httpApplication, struExeLocation, &pfnAspNetCoreCreateApplication)); - std::array parameters { - {"InProcessExeLocation", struExeLocation.QueryStr()} - }; + FINISHED_IF_FAILED(m_handlerResolver.GetApplicationFactory(httpApplication, m_pApplicationFactory)); LOG_INFO("Creating handler application"); IAPPLICATION * newApplication; - FINISHED_IF_FAILED(pfnAspNetCoreCreateApplication( + FINISHED_IF_FAILED(m_pApplicationFactory->Execute( &m_pServer, &httpApplication, - parameters.data(), - static_cast(parameters.size()), &newApplication)); m_pApplication.reset(newApplication); @@ -110,50 +89,17 @@ Finished: return hr; } + VOID -APPLICATION_INFO::RecycleApplication() +APPLICATION_INFO::ShutDownApplication(bool fServerInitiated) { SRWExclusiveLock lock(m_applicationLock); if (m_pApplication) { - const auto pApplication = m_pApplication.release(); - - HandleWrapper hThread = CreateThread( - NULL, // default security attributes - 0, // default stack size - (LPTHREAD_START_ROUTINE)DoRecycleApplication, - pApplication, // thread function arguments - 0, // default creation flags - NULL); // receive thread identifier - } -} - - -DWORD WINAPI -APPLICATION_INFO::DoRecycleApplication( - LPVOID lpParam) -{ - auto pApplication = std::unique_ptr(static_cast(lpParam)); - - if (pApplication) - { - // Recycle will call shutdown for out of process - pApplication->Stop(/*fServerInitiated*/ false); - } - - return 0; -} - - -VOID -APPLICATION_INFO::ShutDownApplication() -{ - SRWExclusiveLock lock(m_applicationLock); - - if (m_pApplication) - { - m_pApplication ->Stop(/* fServerInitiated */ true); + LOG_ERRORF("Stopping application %S", QueryApplicationInfoKey().c_str()); + m_pApplication ->Stop(fServerInitiated); m_pApplication = nullptr; + m_pApplicationFactory = nullptr; } } diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h index ddf3e23aaa..90f9fe8df9 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h @@ -4,8 +4,6 @@ #pragma once #include "hostfxroptions.h" -#include "hashtable.h" -#include "hashfn.h" #include "aspnetcore_shim_config.h" #include "iapplication.h" #include "SRWSharedLock.h" @@ -19,61 +17,35 @@ class APPLICATION_INFO { public: - APPLICATION_INFO(IHttpServer &pServer) : + APPLICATION_INFO( + IHttpServer &pServer, + IHttpApplication &pApplication, + HandlerResolver &pHandlerResolver + ) : m_pServer(pServer), - m_cRefs(1), - m_handlerResolver(nullptr) + m_handlerResolver(pHandlerResolver), + m_strConfigPath(pApplication.GetAppConfigPath()), + m_strInfoKey(pApplication.GetApplicationId()) { InitializeSRWLock(&m_applicationLock); } - PCWSTR - QueryApplicationInfoKey() - { - return m_struInfoKey.QueryStr(); - } - - STRU* - QueryConfigPath() - { - return &m_struConfigPath; - } - - virtual ~APPLICATION_INFO(); - static - void - StaticInitialize() + std::wstring& + QueryApplicationInfoKey() { + return m_strInfoKey; } - HRESULT - Initialize( - IHttpApplication &pApplication, - HandlerResolver *pHandlerResolver - ); - - VOID - ReferenceApplicationInfo() const + std::wstring& + QueryConfigPath() { - InterlockedIncrement(&m_cRefs); + return m_strConfigPath; } VOID - DereferenceApplicationInfo() const - { - if (InterlockedDecrement(&m_cRefs) == 0) - { - delete this; - } - } - - VOID - RecycleApplication(); - - VOID - ShutDownApplication(); + ShutDownApplication(bool fServerInitiated); HRESULT GetOrCreateApplication( @@ -81,88 +53,30 @@ public: std::unique_ptr& pApplication ); + bool ConfigurationPathApplies(const std::wstring& path) + { + // We need to check that the last character of the config path + // is either a null terminator or a slash. + // This checks the case where the config path was + // MACHINE/WEBROOT/site and your site path is MACHINE/WEBROOT/siteTest + auto const changed = m_strConfigPath._Starts_with(path); + if (changed) + { + const auto lastChar = m_strConfigPath[m_strConfigPath.length()]; + return lastChar == L'\0' || lastChar == L'/'; + } + return false; + } + private: - - static DWORD WINAPI DoRecycleApplication(LPVOID lpParam); - - mutable LONG m_cRefs; - STRU m_struConfigPath; - STRU m_struInfoKey; - SRWLOCK m_applicationLock; IHttpServer &m_pServer; - HandlerResolver *m_handlerResolver; + HandlerResolver &m_handlerResolver; + std::wstring m_strConfigPath; + std::wstring m_strInfoKey; + SRWLOCK m_applicationLock {}; + + std::unique_ptr m_pApplicationFactory; std::unique_ptr m_pApplication; }; -class APPLICATION_INFO_HASH : - public HASH_TABLE -{ - -public: - - APPLICATION_INFO_HASH() - {} - - PCWSTR - ExtractKey( - APPLICATION_INFO *pApplicationInfo - ) - { - return pApplicationInfo->QueryApplicationInfoKey(); - } - - DWORD - CalcKeyHash( - PCWSTR pszApplicationId - ) - { - return HashStringNoCase(pszApplicationId); - } - - BOOL - EqualKeys( - PCWSTR pszKey1, - PCWSTR pszKey2 - ) - { - return CompareStringOrdinal(pszKey1, - -1, - pszKey2, - -1, - TRUE) == CSTR_EQUAL; - } - - VOID - ReferenceRecord( - APPLICATION_INFO *pApplicationInfo - ) - { - pApplicationInfo->ReferenceApplicationInfo(); - } - - VOID - DereferenceRecord( - APPLICATION_INFO *pApplicationInfo - ) - { - pApplicationInfo->DereferenceApplicationInfo(); - } - - static - VOID - ReferenceCopyToTable( - APPLICATION_INFO * pEntry, - PVOID pvData - ) - { - APPLICATION_INFO_HASH *pHash = static_cast(pvData); - DBG_ASSERT(pHash); - pHash->InsertRecord(pEntry); - } - -private: - - APPLICATION_INFO_HASH(const APPLICATION_INFO_HASH &); - void operator=(const APPLICATION_INFO_HASH &); -}; diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp index 25d5966a3d..36fc9a96ba 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp @@ -20,25 +20,16 @@ APPLICATION_MANAGER* APPLICATION_MANAGER::sm_pApplicationManager = NULL; // HRESULT APPLICATION_MANAGER::GetOrCreateApplicationInfo( - _In_ IHttpContext* pHttpContext, - _Out_ APPLICATION_INFO ** ppApplicationInfo + _In_ IHttpContext& pHttpContext, + _Out_ std::shared_ptr& ppApplicationInfo ) { - HRESULT hr = S_OK; - APPLICATION_INFO *pApplicationInfo = NULL; - PCWSTR pszApplicationId = NULL; - STACK_STRU ( strEventMsg, 256 ); - - DBG_ASSERT(pHttpContext); - DBG_ASSERT(ppApplicationInfo); - - *ppApplicationInfo = NULL; - IHttpApplication &pApplication = *pHttpContext->GetApplication(); + auto &pApplication = *pHttpContext.GetApplication(); // The configuration path is unique for each application and is used for the // key in the applicationInfoHash. - pszApplicationId = pApplication.GetApplicationId(); + std::wstring pszApplicationId = pApplication.GetApplicationId(); { // When accessing the m_pApplicationInfoHash, we need to acquire the application manager @@ -47,93 +38,41 @@ APPLICATION_MANAGER::GetOrCreateApplicationInfo( if (g_fInShutdown) { - FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS)); + return HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS); } - m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo); + const auto pair = m_pApplicationInfoHash.find(pszApplicationId); + if (pair != m_pApplicationInfoHash.end()) + { + ppApplicationInfo = pair->second; + return S_OK; + } // It's important to release read lock here so exclusive lock // can be reacquired later as SRW lock doesn't allow upgrades } - if (*ppApplicationInfo == NULL) + // Take exclusive lock before creating the application + SRWExclusiveLock writeLock(m_srwLock); + + if (!m_fDebugInitialize) { - // Take exclusive lock before creating the application - SRWExclusiveLock writeLock(m_srwLock); - - if (!m_fDebugInitialize) - { - DebugInitializeFromConfig(m_pHttpServer, pApplication); - m_fDebugInitialize = TRUE; - } - - // Check if other thread created the application - m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo); - if (*ppApplicationInfo != NULL) - { - FINISHED(S_OK); - } - - pApplicationInfo = new APPLICATION_INFO(m_pHttpServer); - - FINISHED_IF_FAILED(pApplicationInfo->Initialize(pApplication, &m_handlerResolver)); - FINISHED_IF_FAILED(m_pApplicationInfoHash->InsertRecord(pApplicationInfo)); - - *ppApplicationInfo = pApplicationInfo; - pApplicationInfo = NULL; - } -Finished: - - if (pApplicationInfo != NULL) - { - pApplicationInfo->DereferenceApplicationInfo(); - pApplicationInfo = NULL; + DebugInitializeFromConfig(m_pHttpServer, pApplication); + m_fDebugInitialize = TRUE; } - return hr; -} - - -// -// If the application's configuration was changed, -// append the configuration path to the config change context. -// -BOOL -APPLICATION_MANAGER::FindConfigChangedApplication( - _In_ APPLICATION_INFO * pEntry, - _In_ PVOID pvContext) -{ - DBG_ASSERT(pEntry); - DBG_ASSERT(pvContext); - - // Config Change context contains the original config path that changed - // and a multiStr containing - CONFIG_CHANGE_CONTEXT* pContext = static_cast(pvContext); - STRU* pstruConfigPath = pEntry->QueryConfigPath(); - - // check if the application path contains our app/subapp by seeing if the config path - // starts with the notification path. - BOOL fChanged = pstruConfigPath->StartsWith(pContext->pstrPath, true); - if (fChanged) + // Check if other thread created the application + const auto pair = m_pApplicationInfoHash.find(pszApplicationId); + if (pair != m_pApplicationInfoHash.end()) { - auto dwLen = wcslen(pContext->pstrPath); - WCHAR wChar = pstruConfigPath->QueryStr()[dwLen]; - - // We need to check that the last character of the config path - // is either a null terminator or a slash. - // This checks the case where the config path was - // MACHINE/WEBROOT/site and your site path is MACHINE/WEBROOT/siteTest - if (wChar != L'\0' && wChar != L'/') - { - // not current app or sub app - fChanged = FALSE; - } - else - { - pContext->MultiSz.Append(pEntry->QueryApplicationInfoKey()); - } + ppApplicationInfo = pair->second; + return S_OK; } - return fChanged; + + ppApplicationInfo = std::make_shared(m_pHttpServer, pApplication, m_handlerResolver); + m_pApplicationInfoHash.emplace(pszApplicationId, ppApplicationInfo); + + return S_OK; } // @@ -148,101 +87,82 @@ APPLICATION_MANAGER::RecycleApplicationFromManager( _In_ LPCWSTR pszApplicationId ) { - HRESULT hr = S_OK; - DWORD dwPreviousCounter = 0; - APPLICATION_INFO_HASH* table = NULL; - CONFIG_CHANGE_CONTEXT context; - - if (g_fInShutdown) + try { - // We are already shutting down, ignore this event as a global configuration change event - // can occur after global stop listening for some reason. - return hr; - } + std::vector> applicationsToRecycle; - { - SRWExclusiveLock lock(m_srwLock); if (g_fInShutdown) { - return hr; + // We are already shutting down, ignore this event as a global configuration change event + // can occur after global stop listening for some reason. + return S_OK; } - // Make a shallow copy of existing hashtable as we may need to remove nodes - // This will be used for finding differences in which applications are affected by a config change. - table = new APPLICATION_INFO_HASH(); - - // few application expected, small bucket size for hash table - if (FAILED(hr = table->Initialize(17 /*prime*/))) { - goto Finished; + SRWExclusiveLock lock(m_srwLock); + if (g_fInShutdown) + { + return S_OK; + } + const std::wstring configurationPath = pszApplicationId; + + auto itr = m_pApplicationInfoHash.begin(); + while (itr != m_pApplicationInfoHash.end()) + { + if (itr->second->ConfigurationPathApplies(configurationPath)) + { + applicationsToRecycle.emplace_back(itr->second); + itr = m_pApplicationInfoHash.erase(itr); + } + else + { + ++itr; + } + } + + // All applications were unloaded reset handler resolver validation logic + if (m_pApplicationInfoHash.empty()) + { + m_handlerResolver.ResetHostingModel(); + } } - context.pstrPath = pszApplicationId; + // If we receive a request at this point. + // OutOfProcess: we will create a new application with new configuration + // InProcess: the request would have to be rejected, as we are about to call g_HttpServer->RecycleProcess + // on the worker proocess - // Keep track of the preview count of applications to know whether there are applications to delete - dwPreviousCounter = m_pApplicationInfoHash->Count(); - - // We don't want to hold the application manager lock for long time as it will block all incoming requests - // Don't call application shutdown inside the lock - m_pApplicationInfoHash->Apply(APPLICATION_INFO_HASH::ReferenceCopyToTable, static_cast(table)); - DBG_ASSERT(dwPreviousCounter == table->Count()); - - // Removed the applications which are impacted by the configurtion change - m_pApplicationInfoHash->DeleteIf(FindConfigChangedApplication, (PVOID)&context); - } - - // If we receive a request at this point. - // OutOfProcess: we will create a new application with new configuration - // InProcess: the request would have to be rejected, as we are about to call g_HttpServer->RecycleProcess - // on the worker proocess - if (!context.MultiSz.IsEmpty()) - { - PCWSTR path = context.MultiSz.First(); - // Iterate through each of the paths that were shut down, - // calling RecycleApplication on each of them. - while (path != NULL) + if (!applicationsToRecycle.empty()) { - APPLICATION_INFO* pRecord; + for (auto& application : applicationsToRecycle) + { + try + { + application->ShutDownApplication(/* fServerInitiated */ false); + } + catch (...) + { + LOG_ERRORF("Failed to stop application %S", application->QueryApplicationInfoKey().c_str()); + OBSERVE_CAUGHT_EXCEPTION(); - // Application got recycled. Log an event - EventLog::Info( - ASPNETCORE_EVENT_RECYCLE_CONFIGURATION, - ASPNETCORE_EVENT_RECYCLE_CONFIGURATION_MSG, - path); - - table->FindKey(path, &pRecord); - DBG_ASSERT(pRecord != NULL); - - // RecycleApplication is called on a separate thread. - pRecord->RecycleApplication(); - pRecord->DereferenceApplicationInfo(); - path = context.MultiSz.Next(path); + // Failed to recycle an application. Log an event + EventLog::Error( + ASPNETCORE_EVENT_RECYCLE_APP_FAILURE, + ASPNETCORE_EVENT_RECYCLE_FAILURE_CONFIGURATION_MSG, + pszApplicationId); + // Need to recycle the process as we cannot recycle the application + if (!g_fRecycleProcessCalled) + { + g_fRecycleProcessCalled = TRUE; + m_pHttpServer.RecycleProcess(L"AspNetCore Recycle Process on Demand Due Application Recycle Error"); + } + } + } } } + CATCH_RETURN(); -Finished: - if (table != NULL) - { - table->Clear(); - delete table; - } - - if (FAILED(hr)) - { - // Failed to recycle an application. Log an event - EventLog::Error( - ASPNETCORE_EVENT_RECYCLE_APP_FAILURE, - ASPNETCORE_EVENT_RECYCLE_FAILURE_CONFIGURATION_MSG, - pszApplicationId); - // Need to recycle the process as we cannot recycle the application - if (!g_fRecycleProcessCalled) - { - g_fRecycleProcessCalled = TRUE; - m_pHttpServer.RecycleProcess(L"AspNetCore Recycle Process on Demand Due Application Recycle Error"); - } - } - - return hr; + return S_OK; } // @@ -256,34 +176,12 @@ APPLICATION_MANAGER::ShutDown() // However, it is possible to receive multiple OnGlobalStopListening events // Protect against this by checking if we already shut down. g_fInShutdown = TRUE; - if (m_pApplicationInfoHash != NULL) + + // During shutdown we lock until we delete the application + SRWExclusiveLock lock(m_srwLock); + for (auto &pair : m_pApplicationInfoHash) { - DBG_ASSERT(m_pApplicationInfoHash); - - // During shutdown we lock until we delete the application - SRWExclusiveLock lock(m_srwLock); - - // Call shutdown on each application in the application manager - m_pApplicationInfoHash->Apply(ShutdownApplication, NULL); - m_pApplicationInfoHash->Clear(); - delete m_pApplicationInfoHash; - m_pApplicationInfoHash = NULL; + pair.second->ShutDownApplication(/* fServerInitiated */ true); + pair.second = nullptr; } } - -// -// Calls shutdown on each application. ApplicationManager's lock is held for duration of -// each shutdown call, guaranteeing another application cannot be created. -// -// static -VOID -APPLICATION_MANAGER::ShutdownApplication( - _In_ APPLICATION_INFO * pEntry, - _In_ PVOID pvContext -) -{ - UNREFERENCED_PARAMETER(pvContext); - DBG_ASSERT(pEntry != NULL); - - pEntry->ShutDownApplication(); -} diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index e0f2025f04..5cb341087d 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -6,6 +6,7 @@ #include "applicationinfo.h" #include "multisz.h" #include "exceptions.h" +#include #define DEFAULT_HASH_BUCKETS 17 @@ -15,11 +16,6 @@ // Should always call GetInstance to get the object instance // -struct CONFIG_CHANGE_CONTEXT -{ - PCWSTR pstrPath; - MULTISZ MultiSz; -}; class APPLICATION_MANAGER { @@ -35,9 +31,7 @@ public: static VOID - Cleanup( - VOID - ) + Cleanup() { if(sm_pApplicationManager != NULL) { @@ -46,24 +40,10 @@ public: } } - static - BOOL - FindConfigChangedApplication( - _In_ APPLICATION_INFO * pEntry, - _In_ PVOID pvContext - ); - - static - VOID - ShutdownApplication( - _In_ APPLICATION_INFO * pEntry, - _In_ PVOID pvContext - ); - HRESULT GetOrCreateApplicationInfo( - _In_ IHttpContext* pHttpContext, - _Out_ APPLICATION_INFO ** ppApplicationInfo + _In_ IHttpContext& pHttpContext, + _Out_ std::shared_ptr& ppApplicationInfo ); HRESULT @@ -74,40 +54,13 @@ public: VOID ShutDown(); - ~APPLICATION_MANAGER() - { - - if(m_pApplicationInfoHash != NULL) - { - m_pApplicationInfoHash->Clear(); - delete m_pApplicationInfoHash; - m_pApplicationInfoHash = NULL; - } - } - static HRESULT StaticInitialize(HMODULE hModule, IHttpServer& pHttpServer) { assert(!sm_pApplicationManager); sm_pApplicationManager = new APPLICATION_MANAGER(hModule, pHttpServer); - RETURN_IF_FAILED(sm_pApplicationManager->Initialize()); - - APPLICATION_INFO::StaticInitialize(); return S_OK; } - HRESULT Initialize() - { - if(m_pApplicationInfoHash == NULL) - { - try - { - m_pApplicationInfoHash = new APPLICATION_INFO_HASH(); - } - CATCH_RETURN(); - RETURN_IF_FAILED(m_pApplicationInfoHash->Initialize(DEFAULT_HASH_BUCKETS)); - } - return S_OK; - } private: APPLICATION_MANAGER(HMODULE hModule, IHttpServer& pHttpServer) : @@ -119,7 +72,7 @@ private: InitializeSRWLock(&m_srwLock); } - APPLICATION_INFO_HASH *m_pApplicationInfoHash; + std::unordered_map> m_pApplicationInfoHash; static APPLICATION_MANAGER *sm_pApplicationManager; SRWLOCK m_srwLock {}; BOOL m_fDebugInitialize; diff --git a/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.cpp b/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.cpp index bf62cd9e87..f994fcc06d 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.cpp @@ -14,7 +14,6 @@ ASPNETCORE_SHIM_CONFIG::Populate( ) { STACK_STRU(strHostingModel, 12); - STRU strApplicationFullPath; CComPtr pAspNetCoreElement; IAppHostAdminManager *pAdminManager = pHttpServer->GetAdminManager(); @@ -25,9 +24,11 @@ ASPNETCORE_SHIM_CONFIG::Populate( applicationConfigPath, &pAspNetCoreElement)); + CComBSTR struProcessPath; RETURN_IF_FAILED(GetElementStringProperty(pAspNetCoreElement, CS_ASPNETCORE_PROCESS_EXE_PATH, - &m_struProcessPath)); + &struProcessPath)); + m_strProcessPath = struProcessPath; // Swallow this error for backward compatibility // Use default behavior for empty string @@ -53,13 +54,18 @@ ASPNETCORE_SHIM_CONFIG::Populate( RETURN_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED)); } + CComBSTR struArguments; RETURN_IF_FAILED(GetElementStringProperty(pAspNetCoreElement, CS_ASPNETCORE_PROCESS_ARGUMENTS, - &m_struArguments)); + &struArguments)); + + m_strArguments = struArguments; if (m_hostingModel == HOSTING_OUT_PROCESS) { - RETURN_IF_FAILED(ConfigUtility::FindHandlerVersion(pAspNetCoreElement, m_struHandlerVersion)); + STRU struHandlerVersion; + RETURN_IF_FAILED(ConfigUtility::FindHandlerVersion(pAspNetCoreElement, struHandlerVersion)); + m_strHandlerVersion = struHandlerVersion.QueryStr(); } diff --git a/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.h b/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.h index e80071b1c9..c380433e72 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.h +++ b/src/AspNetCoreModuleV2/AspNetCore/aspnetcore_shim_config.h @@ -3,11 +3,10 @@ #pragma once +#include #include #include -#include "stringu.h" - #define CS_ASPNETCORE_SECTION L"system.webServer/aspNetCore" #define CS_ASPNETCORE_PROCESS_EXE_PATH L"processPath" #define CS_ASPNETCORE_PROCESS_ARGUMENTS L"arguments" @@ -34,16 +33,16 @@ public: IHttpApplication *pHttpApplication ); - STRU* + std::wstring& QueryProcessPath() { - return &m_struProcessPath; + return m_strProcessPath; } - STRU* + std::wstring& QueryArguments() { - return &m_struArguments; + return m_strArguments; } APP_HOSTING_MODEL @@ -52,10 +51,10 @@ public: return m_hostingModel; } - STRU* + std::wstring& QueryHandlerVersion() { - return &m_struHandlerVersion; + return m_strHandlerVersion; } BOOL @@ -77,10 +76,10 @@ public: private: - STRU m_struArguments; - STRU m_struProcessPath; - APP_HOSTING_MODEL m_hostingModel; - STRU m_struHandlerVersion; + std::wstring m_strArguments; + std::wstring m_strProcessPath; + APP_HOSTING_MODEL m_hostingModel; + std::wstring m_strHandlerVersion; BOOL m_fStdoutLogEnabled; STRU m_struStdoutLogFile; }; diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp index 2d9fe9150c..b4ed0fbaad 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp @@ -51,19 +51,10 @@ Return value: } ASPNET_CORE_PROXY_MODULE::ASPNET_CORE_PROXY_MODULE( -) : m_pApplicationInfo(nullptr), m_pApplication(nullptr), m_pHandler(nullptr) +) : m_pApplicationInfo(nullptr), m_pHandler(nullptr) { } -ASPNET_CORE_PROXY_MODULE::~ASPNET_CORE_PROXY_MODULE() -{ - if (m_pApplicationInfo != NULL) - { - m_pApplicationInfo->DereferenceApplicationInfo(); - m_pApplicationInfo = NULL; - } -} - __override REQUEST_NOTIFICATION_STATUS ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( @@ -72,9 +63,7 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( ) { HRESULT hr = S_OK; - APPLICATION_MANAGER *pApplicationManager = NULL; REQUEST_NOTIFICATION_STATUS retVal = RQ_NOTIFICATION_CONTINUE; - STRU struExeLocation; try { @@ -83,13 +72,11 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS)); } - pApplicationManager = APPLICATION_MANAGER::GetInstance(); + auto pApplicationManager = APPLICATION_MANAGER::GetInstance(); FINISHED_IF_FAILED(pApplicationManager->GetOrCreateApplicationInfo( - pHttpContext, - &m_pApplicationInfo)); - - DBG_ASSERT(pHttpContext); + *pHttpContext, + m_pApplicationInfo)); std::unique_ptr pApplication; FINISHED_IF_FAILED(m_pApplicationInfo->GetOrCreateApplication(pHttpContext, pApplication)); diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h index a66a0fd3e1..06e333d595 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h @@ -15,7 +15,7 @@ class ASPNET_CORE_PROXY_MODULE : public CHttpModule ASPNET_CORE_PROXY_MODULE(); - ~ASPNET_CORE_PROXY_MODULE(); + ~ASPNET_CORE_PROXY_MODULE() = default; void * operator new(size_t size, IModuleAllocator * pPlacement) { @@ -46,9 +46,7 @@ class ASPNET_CORE_PROXY_MODULE : public CHttpModule ); private: - - APPLICATION_INFO *m_pApplicationInfo; - IAPPLICATION *m_pApplication; + std::shared_ptr m_pApplicationInfo; std::unique_ptr m_pHandler; }; diff --git a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h index d6b50f2381..1d607210e2 100644 --- a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h +++ b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h @@ -52,6 +52,13 @@ public: HandleType* operator&() { return &m_handle; } + HandleType release() noexcept + { + auto value = m_handle; + m_handle = traits::DefaultHandle; + return value; + } + private: HandleType m_handle; }; diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp index e31351a764..7184dbfde8 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp @@ -65,7 +65,7 @@ void SetDebugFlags(const std::wstring &debugValue) { try { - if (!debugValue.empty()) + if (!debugValue.empty() && debugValue.find_first_not_of(L"0123456789") == std::wstring::npos) { const auto value = std::stoi(debugValue); diff --git a/src/AspNetCoreModuleV2/CommonLib/file_utility.cpp b/src/AspNetCoreModuleV2/CommonLib/file_utility.cpp index 0d12c32259..a971821208 100644 --- a/src/AspNetCoreModuleV2/CommonLib/file_utility.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/file_utility.cpp @@ -165,34 +165,3 @@ FILE_UTILITY::EnsureDirectoryPathExist( Finished: return hr; } - -BOOL -FILE_UTILITY::CheckIfFileExists( - _In_ PCWSTR pszFilePath -) -{ - HANDLE hFileHandle = INVALID_HANDLE_VALUE; - SECURITY_ATTRIBUTES saAttr; - BOOL fFileExists = FALSE; - - saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); - saAttr.bInheritHandle = TRUE; - saAttr.lpSecurityDescriptor = NULL; - - hFileHandle = CreateFile(pszFilePath, - GENERIC_READ, - FILE_SHARE_READ, - &saAttr, - OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, - NULL); - - fFileExists = hFileHandle != INVALID_HANDLE_VALUE || GetLastError() == ERROR_SHARING_VIOLATION; - - if (fFileExists) - { - CloseHandle(hFileHandle); - } - - return fFileExists; -} diff --git a/src/AspNetCoreModuleV2/CommonLib/file_utility.h b/src/AspNetCoreModuleV2/CommonLib/file_utility.h index 900e787cd7..cc541b712a 100644 --- a/src/AspNetCoreModuleV2/CommonLib/file_utility.h +++ b/src/AspNetCoreModuleV2/CommonLib/file_utility.h @@ -25,12 +25,6 @@ public: EnsureDirectoryPathExist( _In_ LPCWSTR pszPath ); - - static - BOOL - CheckIfFileExists( - PCWSTR pszFilePath - ); private: static HRESULT diff --git a/src/AspNetCoreModuleV2/CommonLib/iapplication.h b/src/AspNetCoreModuleV2/CommonLib/iapplication.h index 1f69a39bbb..222230c672 100644 --- a/src/AspNetCoreModuleV2/CommonLib/iapplication.h +++ b/src/AspNetCoreModuleV2/CommonLib/iapplication.h @@ -14,8 +14,8 @@ enum APPLICATION_STATUS struct APPLICATION_PARAMETER { - LPCSTR pzName; - PVOID pValue; + LPCSTR pzName; + const void *pValue; }; class IAPPLICATION diff --git a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp index 25e8054318..451221255b 100644 --- a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp +++ b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp @@ -146,6 +146,8 @@ EnsureOutOfProcessInitializtion() FINISHED(S_OK); } + g_fOutOfProcessInitialize = TRUE; + g_hWinHttpModule = GetModuleHandle(TEXT("winhttp.dll")); g_hAspNetCoreModule = GetModuleHandle(TEXT("aspnetcorev2.dll")); diff --git a/test/Common.FunctionalTests/ConfigurationChangeTests.cs b/test/Common.FunctionalTests/ConfigurationChangeTests.cs index 064df80451..d1e5f62ff1 100644 --- a/test/Common.FunctionalTests/ConfigurationChangeTests.cs +++ b/test/Common.FunctionalTests/ConfigurationChangeTests.cs @@ -1,12 +1,18 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.IO; +using System.Linq; using System.Net; +using System.Net.Http; +using System.Net.Sockets; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.AspNetCore.Server.IntegrationTesting.IIS; using Microsoft.AspNetCore.Testing.xunit; +using Microsoft.Extensions.Logging; using Xunit; namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests @@ -14,6 +20,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests [Collection(PublishedSitesCollection.Name)] public class ConfigurationChangeTests : IISFunctionalTestBase { + private static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(100); private readonly PublishedSitesFixture _fixture; public ConfigurationChangeTests(PublishedSitesFixture fixture) @@ -66,13 +73,54 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests await deploymentResult.AssertStarts(); deploymentResult.ModifyWebConfig(element => element - .GetOrAdd("system.webServer") + .Descendants("system.webServer") + .Single() .GetOrAdd("aspNetCore") .SetAttributeValue("hostingModel", "inprocess")); // Have to retry here to allow ANCM to receive notification and react to it - // Verify that worker process gets restarted with new process id + // Verify that inprocess application was created and tried to start await deploymentResult.HttpClient.RetryRequestAsync("/HelloWorld", r => r.StatusCode == HttpStatusCode.InternalServerError); + + StopServer(); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, "Could not find the assembly 'aspnetcorev2_inprocess.dll'"); + } + + [ConditionalTheory] + [InlineData(HostingModel.InProcess)] + [InlineData(HostingModel.OutOfProcess)] + public async Task ConfigurationTouchedStress(HostingModel hostingModel) + { + var deploymentResult = await DeployAsync(_fixture.GetBaseDeploymentParameters(hostingModel, publish: true)); + + await deploymentResult.AssertStarts(); + var load = Helpers.StressLoad(deploymentResult.HttpClient, "/HelloWorld", response => { + var statusCode = (int)response.StatusCode; + Assert.True(statusCode == 200 || statusCode == 503, "Status code was " + statusCode); + }); + + for (int i = 0; i < 100; i++) + { + // ModifyWebConfig might fail if web.config is being read by IIS + RetryHelper.RetryOperation( + () => deploymentResult.ModifyWebConfig(element => {}), + e => Logger.LogError($"Failed to touch web.config : {e.Message}"), + retryCount: 3, + retryDelayMilliseconds: RetryDelay.Milliseconds); + } + + try + { + await load; + } + catch (HttpRequestException ex) when (ex.InnerException is IOException | ex.InnerException is SocketException) + { + // IOException in InProcess is fine, just means process stopped + if (hostingModel != HostingModel.InProcess) + { + throw; + } + } } } } diff --git a/test/CommonLibTests/hostfxr_utility_tests.cpp b/test/CommonLibTests/hostfxr_utility_tests.cpp index 7997220726..232f7627b8 100644 --- a/test/CommonLibTests/hostfxr_utility_tests.cpp +++ b/test/CommonLibTests/hostfxr_utility_tests.cpp @@ -98,11 +98,11 @@ TEST(GetAbsolutePathToDotnetFromProgramFiles, BackupWorks) if (is64Bit) { - fDotnetInProgramFiles = FILE_UTILITY::CheckIfFileExists(L"C:/Program Files/dotnet/dotnet.exe"); + fDotnetInProgramFiles = std::filesystem::is_regular_file(L"C:/Program Files/dotnet/dotnet.exe"); } else { - fDotnetInProgramFiles = FILE_UTILITY::CheckIfFileExists(L"C:/Program Files (x86)/dotnet/dotnet.exe"); + fDotnetInProgramFiles = std::filesystem::is_regular_file(L"C:/Program Files (x86)/dotnet/dotnet.exe"); } auto dotnetPath = HOSTFXR_UTILITY::GetAbsolutePathToDotnetFromProgramFiles(); From 36e5aceb3c1cc4d677c75a11caf248cb98a8c4fa Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 22 Aug 2018 09:53:25 -0700 Subject: [PATCH 2/4] Serve 503 if server process is shutting down (#1293) --- .../OutOfProcessRequestHandler/forwardinghandler.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp index e06b15f805..881c6393ad 100644 --- a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp +++ b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp @@ -344,6 +344,10 @@ Failure: { pResponse->SetStatus(400, "Bad Request", 0, hr); } + else if (hr == E_APPLICATION_EXITING) + { + pResponse->SetStatus(503, "Service Unavailable", 0, S_OK, nullptr, TRUE); + } else if (fFailedToStartKestrel && !m_pApplication->QueryConfig()->QueryDisableStartUpErrorPage()) { HTTP_DATA_CHUNK DataChunk; From eebbb6a6022ce609dcf141906148eaf32b1e570f Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 22 Aug 2018 12:04:04 -0700 Subject: [PATCH 3/4] Support portable.exe apps and better error reporting (#1287) --- build/testsite.props | 2 +- .../AspNetCore/HandlerResolver.cpp | 14 +- .../CommonLib/CommonLib.vcxproj | 2 + .../CommonLib/HandleWrapper.cpp | 1 + .../CommonLib/HandleWrapper.h | 9 +- .../CommonLib/StringHelpers.cpp | 15 + .../CommonLib/StringHelpers.h | 20 + .../CommonLib/aspnetcore_msg.mc | 44 +- src/AspNetCoreModuleV2/CommonLib/exceptions.h | 3 +- .../CommonLib/hostfxr_utility.cpp | 462 ++++++------------ .../CommonLib/hostfxr_utility.h | 120 +++-- .../CommonLib/hostfxroptions.cpp | 90 ++-- .../CommonLib/hostfxroptions.h | 59 ++- src/AspNetCoreModuleV2/CommonLib/resources.h | 9 +- .../inprocessapplication.cpp | 12 +- .../Utilities/EventLogHelpers.cs | 16 +- .../Utilities/FunctionalTestsBase.cs | 5 - test/CommonLibTests/hostfxr_utility_tests.cpp | 94 ++-- .../InProcess/StartupTests.cs | 33 +- 19 files changed, 415 insertions(+), 595 deletions(-) create mode 100644 src/AspNetCoreModuleV2/CommonLib/StringHelpers.cpp create mode 100644 src/AspNetCoreModuleV2/CommonLib/StringHelpers.h diff --git a/build/testsite.props b/build/testsite.props index 1588b54226..ad7204aab4 100644 --- a/build/testsite.props +++ b/build/testsite.props @@ -48,7 +48,7 @@ - + False diff --git a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp index 83c34a30c0..51b8cb611e 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/HandlerResolver.cpp @@ -60,7 +60,7 @@ HandlerResolver::LoadRequestHandlerAssembly(IHttpApplication &pApplication, ASPN pConfiguration.QueryArguments().c_str(), options)); - location = options->GetExeLocation(); + location = options->GetDotnetExeLocation(); RETURN_IF_FAILED(LoggingHelpers::CreateLoggingProvider( pConfiguration.QueryStdoutLogEnabled(), @@ -225,7 +225,7 @@ HandlerResolver::FindNativeAssemblyFromHostfxr( DWORD dwBufferSize = s_initialGetNativeSearchDirectoriesBufferSize; DWORD dwRequiredBufferSize = 0; - RETURN_LAST_ERROR_IF_NULL(m_hHostFxrDll = LoadLibraryW(hostfxrOptions.GetHostFxrLocation())); + RETURN_LAST_ERROR_IF_NULL(m_hHostFxrDll = LoadLibraryW(hostfxrOptions.GetHostFxrLocation().c_str())); auto pFnHostFxrSearchDirectories = reinterpret_cast(GetProcAddress(m_hHostFxrDll, "hostfxr_get_native_search_directories")); if (pFnHostFxrSearchDirectories == nullptr) @@ -233,7 +233,7 @@ HandlerResolver::FindNativeAssemblyFromHostfxr( EventLog::Error( ASPNETCORE_EVENT_GENERAL_ERROR, ASPNETCORE_EVENT_HOSTFXR_DLL_INVALID_VERSION_MSG, - hostfxrOptions.GetHostFxrLocation() + hostfxrOptions.GetHostFxrLocation().c_str() ); RETURN_IF_FAILED(E_FAIL); } @@ -243,9 +243,13 @@ HandlerResolver::FindNativeAssemblyFromHostfxr( while (TRUE) { + DWORD hostfxrArgc; + std::unique_ptr hostfxrArgv; + + hostfxrOptions.GetArguments(hostfxrArgc, hostfxrArgv); const auto intHostFxrExitCode = pFnHostFxrSearchDirectories( - hostfxrOptions.GetArgc(), - hostfxrOptions.GetArgv(), + hostfxrArgc, + hostfxrArgv.get(), struNativeSearchPaths.data(), dwBufferSize, &dwRequiredBufferSize diff --git a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj index 47a5373ee1..84b7f360cc 100644 --- a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj +++ b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj @@ -219,6 +219,7 @@ + @@ -243,6 +244,7 @@ Create Create + diff --git a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.cpp b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.cpp index fde7d2fd1f..4960b06f4e 100644 --- a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.cpp @@ -5,3 +5,4 @@ // Workaround for VC++ bug https://developercommunity.visualstudio.com/content/problem/33928/constexpr-failing-on-nullptr-v141-compiler-regress.html const HANDLE InvalidHandleTraits::DefaultHandle = INVALID_HANDLE_VALUE; +const HANDLE FindFileHandleTraits::DefaultHandle = INVALID_HANDLE_VALUE; diff --git a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h index 1d607210e2..d3afa64551 100644 --- a/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h +++ b/src/AspNetCoreModuleV2/CommonLib/HandleWrapper.h @@ -4,7 +4,7 @@ #pragma once #include -#include +#include "ntassert.h" struct InvalidHandleTraits { @@ -20,6 +20,13 @@ struct NullHandleTraits static void Close(HANDLE handle) { CloseHandle(handle); } }; +struct FindFileHandleTraits +{ + using HandleType = HANDLE; + static const HANDLE DefaultHandle; + static void Close(HANDLE handle) { FindClose(handle); } +}; + struct ModuleHandleTraits { using HandleType = HMODULE; diff --git a/src/AspNetCoreModuleV2/CommonLib/StringHelpers.cpp b/src/AspNetCoreModuleV2/CommonLib/StringHelpers.cpp new file mode 100644 index 0000000000..8e574677cd --- /dev/null +++ b/src/AspNetCoreModuleV2/CommonLib/StringHelpers.cpp @@ -0,0 +1,15 @@ +// 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. + +#include "StringHelpers.h" + +bool ends_with(const std::wstring &source, const std::wstring &suffix, bool ignoreCase) +{ + if (source.length() < suffix.length()) + { + return false; + } + + const auto offset = source.length() - suffix.length(); + return CSTR_EQUAL == CompareStringOrdinal(source.c_str() + offset, static_cast(suffix.length()), suffix.c_str(), static_cast(suffix.length()), ignoreCase); +} diff --git a/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h b/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h new file mode 100644 index 0000000000..4999bec7a7 --- /dev/null +++ b/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h @@ -0,0 +1,20 @@ +// 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. + +#pragma once + +#include + +[[nodiscard]] +bool ends_with(const std::wstring &source, const std::wstring &suffix, bool ignoreCase = false); + +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); +} + diff --git a/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc b/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc index 5b4c244c63..f0e3d66de3 100644 --- a/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc +++ b/src/AspNetCoreModuleV2/CommonLib/aspnetcore_msg.mc @@ -38,12 +38,6 @@ Language=English %1 . -Messageid=1002 -SymbolicName=ASPNETCORE_EVENT_PROCESS_CRASH -Language=English -%1 -. - Messageid=1003 SymbolicName=ASPNETCORE_EVENT_RAPID_FAIL_COUNT_EXCEEDED Language=English @@ -110,42 +104,12 @@ Language=English %1 . -Messageid=1014 -SymbolicName=ASPNETCORE_EVENT_INPROCESS_FULL_FRAMEWORK_APP -Language=English -%1 -. - -Messageid=1015 -SymbolicName=ASPNETCORE_EVENT_PORTABLE_APP_DOTNET_MISSING -Language=English -%1 -. - -Messageid=1016 -SymbolicName=ASPNETCORE_EVENT_HOSTFXR_DIRECTORY_NOT_FOUND -Language=English -%1 -. - -Messageid=1017 -SymbolicName=ASPNETCORE_EVENT_HOSTFXR_DLL_NOT_FOUND -Language=English -%1 -. - Messageid=1018 SymbolicName=ASPNETCORE_EVENT_INPROCESS_THREAD_EXCEPTION Language=English %1 . -Messageid=1019 -SymbolicName=ASPNETCORE_EVENT_APPLICATION_EXE_NOT_FOUND -Language=English -%1 -. - Messageid=1020 SymbolicName=ASPNETCORE_EVENT_PROCESS_START_FAILURE Language=English @@ -164,12 +128,6 @@ Language=English %1 . -Messageid=1023 -SymbolicName=ASPNETCORE_EVENT_APP_IN_SHUTDOWN -Language=English -%1 -. - Messageid=1024 SymbolicName=ASPNETCORE_EVENT_MONITOR_APPOFFLINE_ERROR Language=English @@ -213,7 +171,7 @@ Language=English . Messageid=1031 -SymbolicName=ASPNETCORE_EVENT_INVALID_PROCESS_PATH +SymbolicName=ASPNETCORE_EVENT_INPROCESS_START_ERROR Language=English %1 . diff --git a/src/AspNetCoreModuleV2/CommonLib/exceptions.h b/src/AspNetCoreModuleV2/CommonLib/exceptions.h index bd6f30af33..31873bc743 100644 --- a/src/AspNetCoreModuleV2/CommonLib/exceptions.h +++ b/src/AspNetCoreModuleV2/CommonLib/exceptions.h @@ -43,6 +43,7 @@ #define CATCH_RETURN() catch (...) { RETURN_CAUGHT_EXCEPTION(); } #define LOG_IF_FAILED(hr) LogHResultFailed(LOCATION_INFO, hr) +#define LOG_LAST_ERROR() LogLastErrorIf(LOCATION_INFO, true) #define LOG_LAST_ERROR_IF(condition) LogLastErrorIf(LOCATION_INFO, condition) #define SUCCEEDED_LOG(hr) SUCCEEDED(LOG_IF_FAILED(hr)) #define FAILED_LOG(hr) FAILED(LOG_IF_FAILED(hr)) @@ -74,7 +75,7 @@ __declspec(noinline) inline VOID ReportException(LOCATION_ARGUMENTS std::exception& exception) { - DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, LOCATION_FORMAT "Unhandled exception: %s", LOCATION_CALL exception.what()); + DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, "Exception : %s caught at" LOCATION_FORMAT, exception.what(), LOCATION_CALL_ONLY); } __declspec(noinline) inline HRESULT LogHResultFailed(LOCATION_ARGUMENTS HRESULT hr) diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp index 3d360b324d..c96133596d 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp @@ -3,185 +3,78 @@ #include "hostfxr_utility.h" -#include #include -#include "EventLog.h" -#include "ntassert.h" #include "fx_ver.h" #include "debugutil.h" #include "exceptions.h" #include "HandleWrapper.h" #include "Environment.h" -#include "file_utility.h" +#include "StringHelpers.h" namespace fs = std::filesystem; -// -// Runs a standalone appliction. -// The folder structure looks like this: -// Application/ -// hostfxr.dll -// Application.exe -// Application.dll -// etc. -// We get the full path to hostfxr.dll and Application.dll and run hostfxr_main, -// passing in Application.dll. -// Assuming we don't need Application.exe as the dll is the actual application. -// -HRESULT -HOSTFXR_UTILITY::GetStandaloneHostfxrParameters( - PCWSTR pwzExeAbsolutePath, // includes .exe file extension. - PCWSTR pcwzApplicationPhysicalPath, - PCWSTR pcwzArguments, - _Inout_ STRU* pStruHostFxrDllLocation, - _Out_ DWORD* pdwArgCount, - _Out_ BSTR** ppwzArgv -) -{ - LOG_INFOF("Resolving standalone hostfxr parameters for application: %S arguments: %S path: %S", - pwzExeAbsolutePath, - pcwzArguments, - pwzExeAbsolutePath); - - const fs::path exePath(pwzExeAbsolutePath); - - if (!exePath.has_extension()) - { - LOG_INFOF("Exe path has not extension, returning"); - - return false; - } - - const fs::path physicalPath(pcwzApplicationPhysicalPath); - const fs::path hostFxrLocation = physicalPath / "hostfxr.dll"; - - LOG_INFOF("Checking hostfxr.dll at %S", hostFxrLocation.c_str()); - - if (!is_regular_file(hostFxrLocation)) - { - fs::path runtimeConfigLocation = exePath; - runtimeConfigLocation.replace_extension(L".runtimeconfig.json"); - - LOG_INFOF("Checking runtimeconfig.json at %S", runtimeConfigLocation.c_str()); - if (!is_regular_file(runtimeConfigLocation)) - { - EventLog::Error( - ASPNETCORE_EVENT_INPROCESS_FULL_FRAMEWORK_APP, - ASPNETCORE_EVENT_INPROCESS_FULL_FRAMEWORK_APP_MSG, - pcwzApplicationPhysicalPath, - 0); - return E_FAIL; - } - - EventLog::Error( - ASPNETCORE_EVENT_APPLICATION_EXE_NOT_FOUND, - ASPNETCORE_EVENT_APPLICATION_EXE_NOT_FOUND_MSG, - pcwzApplicationPhysicalPath, - 0); - return E_FAIL; - } - - fs::path dllPath = exePath; - dllPath.replace_extension(".dll"); - - if (!is_regular_file(dllPath)) - { - LOG_INFOF("Application dll at %S was not found", dllPath.c_str()); - return E_FAIL; - } - - auto arguments = std::wstring(dllPath) + L" " + pcwzArguments; - - RETURN_IF_FAILED(pStruHostFxrDllLocation->Copy(hostFxrLocation.c_str())); - - RETURN_IF_FAILED(ParseHostfxrArguments( - arguments.c_str(), - pwzExeAbsolutePath, - pcwzApplicationPhysicalPath, - pdwArgCount, - ppwzArgv)); - - return S_OK; -} - -BOOL -HOSTFXR_UTILITY::IsDotnetExecutable(const std::filesystem::path & dotnetPath) -{ - auto name = dotnetPath.filename(); - name.replace_extension(""); - return _wcsnicmp(name.c_str(), L"dotnet", 6) == 0; -} - -HRESULT +void HOSTFXR_UTILITY::GetHostFxrParameters( - _In_ PCWSTR pcwzProcessPath, - _In_ PCWSTR pcwzApplicationPhysicalPath, - _In_ PCWSTR pcwzArguments, - _Inout_ STRU *pStruHostFxrDllLocation, - _Inout_ STRU *pStruExeAbsolutePath, - _Out_ DWORD *pdwArgCount, - _Out_ BSTR **pbstrArgv + const fs::path &processPath, + const fs::path &applicationPhysicalPath, + const std::wstring &applicationArguments, + fs::path &hostFxrDllPath, + fs::path &dotnetExePath, + std::vector &arguments ) { - HRESULT hr = S_OK; + LOG_INFOF("Resolving hostfxr parameters for application: '%S' arguments: '%S' path: '%S'", + processPath.c_str(), + applicationArguments.c_str(), + applicationPhysicalPath.c_str()); - LOG_INFOF("Resolving hostfxr parameters for application: %S arguments: %S path: %S", - pcwzProcessPath, - pcwzArguments, - pcwzApplicationPhysicalPath); + fs::path expandedProcessPath = Environment::ExpandEnvironmentVariables(processPath); + const auto expandedApplicationArguments = Environment::ExpandEnvironmentVariables(applicationArguments); - const fs::path applicationPhysicalPath = pcwzApplicationPhysicalPath; - fs::path processPath = Environment::ExpandEnvironmentVariables(pcwzProcessPath); - std::wstring arguments = Environment::ExpandEnvironmentVariables(pcwzArguments); + LOG_INFOF("Expanded hostfxr parameters for application: '%S' arguments: '%S'", + expandedProcessPath.c_str(), + expandedApplicationArguments.c_str()); + + LOG_INFOF("Known dotnet.exe location: '%S'", dotnetExePath.c_str()); + + if (!expandedProcessPath.has_extension()) + { + // The only executable extension inprocess supports + expandedProcessPath.replace_extension(".exe"); + } + else if (!ends_with(expandedProcessPath, L".exe", true)) + { + throw StartupParametersResolutionException(format(L"Process path '%s' doesn't have '.exe' extension.", expandedProcessPath.c_str())); + } // Check if the absolute path is to dotnet or not. - if (IsDotnetExecutable(processPath)) + if (IsDotnetExecutable(expandedProcessPath)) { - LOG_INFOF("Process path %S is dotnet, treating application as portable", processPath.c_str()); - // - // The processPath ends with dotnet.exe or dotnet - // like: C:\Program Files\dotnet\dotnet.exe, C:\Program Files\dotnet\dotnet, dotnet.exe, or dotnet. - // Get the absolute path to dotnet. If the path is already an absolute path, it will return that path - // - // Make sure to append the dotnet.exe path correctly here (pass in regular path)? - auto fullProcessPath = GetAbsolutePathToDotnet(applicationPhysicalPath, processPath); - if (!fullProcessPath.has_value()) + LOG_INFOF("Process path '%S' is dotnet, treating application as portable", expandedProcessPath.c_str()); + + if (dotnetExePath.empty()) { - hr = HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); - EventLog::Error( - ASPNETCORE_EVENT_INVALID_PROCESS_PATH, - ASPNETCORE_EVENT_INVALID_PROCESS_PATH_MSG, - processPath.c_str(), - hr); - return hr; + dotnetExePath = GetAbsolutePathToDotnet(applicationPhysicalPath, expandedProcessPath); } - processPath = fullProcessPath.value(); + hostFxrDllPath = GetAbsolutePathToHostFxr(dotnetExePath); - auto hostFxrPath = GetAbsolutePathToHostFxr(processPath); - if (!hostFxrPath.has_value()) - { - hr = HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); - return hr; - } - - RETURN_IF_FAILED(HOSTFXR_UTILITY::ParseHostfxrArguments( - arguments.c_str(), - processPath.c_str(), - pcwzApplicationPhysicalPath, - pdwArgCount, - pbstrArgv)); - - RETURN_IF_FAILED(pStruHostFxrDllLocation->Copy(hostFxrPath->c_str())); - RETURN_IF_FAILED(pStruExeAbsolutePath->Copy(processPath.c_str())); + ParseHostfxrArguments( + expandedApplicationArguments, + dotnetExePath, + applicationPhysicalPath, + arguments, + true); } else { - LOG_INFOF("Process path %S is not dotnet, treating application as standalone", processPath.c_str()); + LOG_INFOF("Process path '%S' is not dotnet, treating application as standalone or portable with bootstrapper", expandedProcessPath.c_str()); - if (processPath.is_relative()) + auto executablePath = expandedProcessPath; + + if (executablePath.is_relative()) { - processPath = applicationPhysicalPath / processPath; + executablePath = applicationPhysicalPath / expandedProcessPath; } // @@ -189,17 +82,42 @@ HOSTFXR_UTILITY::GetHostFxrParameters( // like: C:\test\MyApp.Exe or MyApp.Exe // Check if the file exists, and if it does, get the parameters for a standalone application // - if (is_regular_file(processPath)) + if (is_regular_file(executablePath)) { - RETURN_IF_FAILED(GetStandaloneHostfxrParameters( - processPath.c_str(), - pcwzApplicationPhysicalPath, - arguments.c_str(), - pStruHostFxrDllLocation, - pdwArgCount, - pbstrArgv)); + auto applicationDllPath = executablePath; + applicationDllPath.replace_extension(".dll"); - RETURN_IF_FAILED(pStruExeAbsolutePath->Copy(processPath.c_str())); + LOG_INFOF("Checking application.dll at %S", applicationDllPath.c_str()); + if (!is_regular_file(applicationDllPath)) + { + throw StartupParametersResolutionException(format(L"Application .dll was not found at %s", applicationDllPath.c_str())); + } + + hostFxrDllPath = executablePath.parent_path() / "hostfxr.dll"; + LOG_INFOF("Checking hostfxr.dll at %S", hostFxrDllPath.c_str()); + if (is_regular_file(hostFxrDllPath)) + { + LOG_INFOF("hostfxr.dll found app local at '%S', treating application as standalone", hostFxrDllPath.c_str()); + } + else + { + LOG_INFOF("hostfxr.dll found app local at '%S', treating application as portable with launcher", hostFxrDllPath.c_str()); + + // passing "dotnet" here because we don't know where dotnet.exe should come from + // so trying all fallbacks is appropriate + if (dotnetExePath.empty()) + { + dotnetExePath = GetAbsolutePathToDotnet(applicationPhysicalPath, L"dotnet"); + } + executablePath = dotnetExePath; + hostFxrDllPath = GetAbsolutePathToHostFxr(dotnetExePath); + } + + ParseHostfxrArguments( + applicationDllPath.generic_wstring() + L" " + expandedApplicationArguments, + executablePath, + applicationPhysicalPath, + arguments); } else { @@ -207,140 +125,75 @@ 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. // - hr = HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); - EventLog::Error( - ASPNETCORE_EVENT_INVALID_PROCESS_PATH, - ASPNETCORE_EVENT_INVALID_PROCESS_PATH_MSG, - processPath.c_str(), - hr); - return hr; + throw StartupParametersResolutionException(format(L"Executable was not found at '%s'", executablePath.c_str())); } } - - return S_OK; } -// -// Forms the argument list in HOSTFXR_PARAMETERS. -// Sets the ArgCount and Arguments. -// Arg structure: -// argv[0] = Path to exe activating hostfxr. -// argv[1] = L"exec" -// argv[2] = absolute path to dll. -// -HRESULT +BOOL +HOSTFXR_UTILITY::IsDotnetExecutable(const std::filesystem::path & dotnetPath) +{ + return ends_with(dotnetPath, L"dotnet.exe", true); +} + +void HOSTFXR_UTILITY::ParseHostfxrArguments( - PCWSTR pwzArgumentsFromConfig, - PCWSTR pwzExePath, - PCWSTR pcwzApplicationPhysicalPath, - _Out_ DWORD* pdwArgCount, - _Out_ BSTR** pbstrArgv + const std::wstring &applicationArguments, + const fs::path &applicationExePath, + const fs::path &applicationPhysicalPath, + std::vector &arguments, + bool expandDllPaths ) { - DBG_ASSERT(pdwArgCount != NULL); - DBG_ASSERT(pbstrArgv != NULL); - DBG_ASSERT(pwzExePath != NULL); + LOG_INFOF("Resolving hostfxr_main arguments application: '%S' arguments: '%S' path: %S", applicationExePath.c_str(), applicationArguments.c_str(), applicationPhysicalPath.c_str()); - HRESULT hr = S_OK; - INT argc = 0; - BSTR* argv = NULL; - LPWSTR* pwzArgs = NULL; - STRU struTempPath; - INT intArgsProcessed = 0; + arguments = std::vector(); + arguments.push_back(applicationExePath); - LOG_INFOF("Resolving hostfxr_main arguments application: %S arguments: %S path: %S", - pwzExePath, - pwzArgumentsFromConfig, - pcwzApplicationPhysicalPath); - - // If we call CommandLineToArgvW with an empty string, argc is 5 for some interesting reason. - // Protectively guard against this by check if the string is null or empty. - if (pwzArgumentsFromConfig == NULL || wcscmp(pwzArgumentsFromConfig, L"") == 0) + if (applicationArguments.empty()) { - hr = E_INVALIDARG; - goto Finished; + throw StartupParametersResolutionException(L"Application arguments are empty."); } - pwzArgs = CommandLineToArgvW(pwzArgumentsFromConfig, &argc); - - if (pwzArgs == NULL) + int argc = 0; + auto pwzArgs = std::unique_ptr(CommandLineToArgvW(applicationArguments.c_str(), &argc)); + if (!pwzArgs) { - hr = LOG_IF_FAILED(HRESULT_FROM_WIN32(GetLastError())); - goto Failure; + throw StartupParametersResolutionException(format(L"Unable parse command line argumens '%s' or '%s'", applicationArguments.c_str())); } - argv = new BSTR[argc + 1]; + for (int intArgsProcessed = 0; intArgsProcessed < argc; intArgsProcessed++) + { + std::wstring argument = pwzArgs[intArgsProcessed]; - argv[0] = SysAllocString(pwzExePath); - if (argv[0] == NULL) - { - hr = E_OUTOFMEMORY; - goto Failure; - } - // Try to convert the application dll from a relative to an absolute path - // Don't record this failure as pwzArgs[0] may already be an absolute path to the dll. - for (intArgsProcessed = 0; intArgsProcessed < argc; intArgsProcessed++) - { - DBG_ASSERT(pwzArgs[intArgsProcessed] != NULL); - struTempPath.Copy(pwzArgs[intArgsProcessed]); - if (struTempPath.EndsWith(L".dll")) + // Try expanding arguments ending in .dll to a full paths + if (expandDllPaths && ends_with(argument, L".dll", true)) { - if (SUCCEEDED(FILE_UTILITY::ConvertPathToFullPath(pwzArgs[intArgsProcessed], pcwzApplicationPhysicalPath, &struTempPath))) + fs::path argumentAsPath = argument; + if (argumentAsPath.is_relative()) { - argv[intArgsProcessed + 1] = SysAllocString(struTempPath.QueryStr()); - } - else - { - argv[intArgsProcessed + 1] = SysAllocString(pwzArgs[intArgsProcessed]); - } - if (argv[intArgsProcessed + 1] == NULL) - { - hr = E_OUTOFMEMORY; - goto Failure; - } - } - else - { - argv[intArgsProcessed + 1] = SysAllocString(pwzArgs[intArgsProcessed]); - if (argv[intArgsProcessed + 1] == NULL) - { - hr = E_OUTOFMEMORY; - goto Failure; + argumentAsPath = applicationPhysicalPath / argumentAsPath; + if (exists(argumentAsPath)) + { + LOG_INFOF("Converted argument '%S' to %S", argument.c_str(), argumentAsPath.c_str()); + argument = argumentAsPath; + } } } - LOG_INFOF("Argument[%d] = %S", - intArgsProcessed + 1, - argv[intArgsProcessed + 1]); + arguments.push_back(argument); } - *pbstrArgv = argv; - *pdwArgCount = argc + 1; - - goto Finished; - -Failure: - if (argv != NULL) + for (size_t i = 0; i < arguments.size(); i++) { - // intArgsProcess - 1 here as if we fail to allocated the ith string - // we don't want to free it. - for (INT i = 0; i < intArgsProcessed - 1; i++) - { - SysFreeString(argv[i]); - } + LOG_INFOF("Argument[%d] = %S", i, arguments[i].c_str()); } - - delete[] argv; - -Finished: - if (pwzArgs != NULL) - { - LocalFree(pwzArgs); - } - return hr; } -std::optional +// The processPath ends with dotnet.exe or dotnet +// like: C:\Program Files\dotnet\dotnet.exe, C:\Program Files\dotnet\dotnet, dotnet.exe, or dotnet. +// Get the absolute path to dotnet. If the path is already an absolute path, it will return that path +fs::path HOSTFXR_UTILITY::GetAbsolutePathToDotnet( const fs::path & applicationPath, const fs::path & requestedPath @@ -357,21 +210,11 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( // // If we are given an absolute path to dotnet.exe, we are done // - if (is_regular_file(requestedPath)) + if (is_regular_file(processPath)) { - LOG_INFOF("Found dotnet.exe at %S", requestedPath.c_str()); + LOG_INFOF("Found dotnet.exe at %S", processPath.c_str()); - return std::make_optional(requestedPath); - } - - auto pathWithExe = requestedPath; - pathWithExe.concat(L".exe"); - - if (is_regular_file(pathWithExe)) - { - LOG_INFOF("Found dotnet.exe at %S", pathWithExe.c_str()); - - return std::make_optional(pathWithExe); + return processPath; } // At this point, we are calling where.exe to find dotnet. @@ -382,7 +225,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( { LOG_INFOF("Absolute path to dotnet.exe was not found at %S", requestedPath.c_str()); - return std::nullopt; + throw StartupParametersResolutionException(format(L"Could not find dotnet.exe at '%s'", processPath.c_str())); } const auto dotnetViaWhere = InvokeWhereToFindDotnet(); @@ -390,7 +233,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( { LOG_INFOF("Found dotnet.exe via where.exe invocation at %S", dotnetViaWhere.value().c_str()); - return dotnetViaWhere; + return dotnetViaWhere.value(); } const auto programFilesLocation = GetAbsolutePathToDotnetFromProgramFiles(); @@ -398,14 +241,17 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( { LOG_INFOF("Found dotnet.exe in Program Files at %S", programFilesLocation.value().c_str()); - return programFilesLocation; + return programFilesLocation.value(); } LOG_INFOF("dotnet.exe not found"); - return std::nullopt; + throw StartupParametersResolutionException(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())); } -std::optional +fs::path HOSTFXR_UTILITY::GetAbsolutePathToHostFxr( const fs::path & dotnetPath ) @@ -417,23 +263,14 @@ HOSTFXR_UTILITY::GetAbsolutePathToHostFxr( if (!is_directory(hostFxrBase)) { - EventLog::Error(ASPNETCORE_EVENT_HOSTFXR_DIRECTORY_NOT_FOUND, ASPNETCORE_EVENT_HOSTFXR_DIRECTORY_NOT_FOUND_MSG, hostFxrBase.c_str(), HRESULT_FROM_WIN32(ERROR_BAD_ENVIRONMENT)); - - return std::nullopt; + throw StartupParametersResolutionException(format(L"Unable to find hostfxr directory at %s", hostFxrBase.c_str())); } - auto searchPattern = std::wstring(hostFxrBase) + L"\\*"; - FindDotNetFolders(searchPattern.c_str(), versionFolders); + FindDotNetFolders(hostFxrBase, versionFolders); if (versionFolders.empty()) { - EventLog::Error( - ASPNETCORE_EVENT_HOSTFXR_DIRECTORY_NOT_FOUND, - ASPNETCORE_EVENT_HOSTFXR_DIRECTORY_NOT_FOUND_MSG, - hostFxrBase.c_str(), - HRESULT_FROM_WIN32(ERROR_BAD_ENVIRONMENT)); - - return std::nullopt; + throw StartupParametersResolutionException(format(L"Hostfxr directory '%s' doesn't contain any version subdirectories", hostFxrBase.c_str())); } const auto highestVersion = FindHighestDotNetVersion(versionFolders); @@ -441,24 +278,18 @@ HOSTFXR_UTILITY::GetAbsolutePathToHostFxr( if (!is_regular_file(hostFxrPath)) { - EventLog::Error( - ASPNETCORE_EVENT_HOSTFXR_DLL_NOT_FOUND, - ASPNETCORE_EVENT_HOSTFXR_DLL_NOT_FOUND_MSG, - hostFxrPath.c_str(), - HRESULT_FROM_WIN32(ERROR_FILE_INVALID)); - - return std::nullopt; + throw StartupParametersResolutionException(format(L"hostfxr.dll not found at '%s'", hostFxrPath.c_str())); } LOG_INFOF("hostfxr.dll located at %S", hostFxrPath.c_str()); - return std::make_optional(hostFxrPath); + return hostFxrPath; } // // Tries to call where.exe to find the location of dotnet.exe. // Will check that the bitness of dotnet matches the current // worker process bitness. -// Returns true if a valid dotnet was found, else false. +// Returns true if a valid dotnet was found, else false.R // std::optional HOSTFXR_UTILITY::InvokeWhereToFindDotnet() @@ -638,7 +469,6 @@ HOSTFXR_UTILITY::FindHighestDotNetVersion( fx_ver_t fx_ver(-1, -1, -1); if (fx_ver_t::parse(dir, &fx_ver, false)) { - // TODO using max instead of std::max works max_ver = max(max_ver, fx_ver); } } @@ -648,16 +478,16 @@ HOSTFXR_UTILITY::FindHighestDotNetVersion( VOID HOSTFXR_UTILITY::FindDotNetFolders( - _In_ PCWSTR pszPath, - _Out_ std::vector & pvFolders + const std::filesystem::path &path, + _Out_ std::vector &pvFolders ) { - HANDLE handle = NULL; - WIN32_FIND_DATAW data = { 0 }; - - handle = FindFirstFileExW(pszPath, FindExInfoStandard, &data, FindExSearchNameMatch, NULL, 0); + WIN32_FIND_DATAW data = {}; + const auto searchPattern = std::wstring(path) + L"\\*"; + HandleWrapper handle = FindFirstFileExW(searchPattern.c_str(), FindExInfoStandard, &data, FindExSearchNameMatch, nullptr, 0); if (handle == INVALID_HANDLE_VALUE) { + LOG_LAST_ERROR(); return; } @@ -665,6 +495,4 @@ HOSTFXR_UTILITY::FindDotNetFolders( { pvFolders.emplace_back(data.cFileName); } while (FindNextFileW(handle, &data)); - - FindClose(handle); } diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h index 126867a865..e12153aaf6 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h @@ -7,7 +7,7 @@ #include #include #include -#include "stringu.h" +#include typedef INT(*hostfxr_get_native_search_directories_fn) (CONST INT argc, CONST PCWSTR* argv, PWSTR buffer, DWORD buffer_size, DWORD* required_buffer_size); typedef INT(*hostfxr_main_fn) (CONST DWORD argc, CONST PCWSTR argv[]); @@ -19,76 +19,88 @@ class HOSTFXR_UTILITY public: static - HRESULT - GetStandaloneHostfxrParameters( - PCWSTR pwzExeAbsolutePath, // includes .exe file extension. - PCWSTR pcwzApplicationPhysicalPath, - PCWSTR pcwzArguments, - _Inout_ STRU* pStruHostFxrDllLocation, - _Out_ DWORD* pdwArgCount, - _Out_ BSTR** ppwzArgv - ); - - static - HRESULT - ParseHostfxrArguments( - PCWSTR pwzArgumentsFromConfig, - PCWSTR pwzExePath, - PCWSTR pcwzApplicationPhysicalPath, - _Out_ DWORD* pdwArgCount, - _Out_ BSTR** pbstrArgv - ); - - static - BOOL - IsDotnetExecutable( - _In_ const std::filesystem::path & dotnetPath - ); - - static - HRESULT + void GetHostFxrParameters( - _In_ PCWSTR pcwzProcessPath, - _In_ PCWSTR pcwzApplicationPhysicalPath, - _In_ PCWSTR pcwzArguments, - _Inout_ STRU *pStruHostFxrDllLocation, - _Inout_ STRU *struExeAbsolutePath, - _Out_ DWORD *pdwArgCount, - _Out_ BSTR **ppwzArgv + const std::filesystem::path &processPath, + const std::filesystem::path &applicationPhysicalPath, + const std::wstring &applicationArguments, + std::filesystem::path &hostFxrDllPath, + std::filesystem::path &dotnetExePath, + std::vector &arguments ); - static - VOID - FindDotNetFolders( - _In_ PCWSTR pszPath, - _Out_ std::vector & pvFolders - ); + 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 - std::wstring - FindHighestDotNetVersion( - _In_ std::vector & vFolders - ); - - static - std::optional - GetAbsolutePathToHostFxr( - _In_ const std::filesystem::path & dotnetPath + void + ParseHostfxrArguments( + const std::wstring &arugments, + const std::filesystem::path &exePath, + const std::filesystem::path &applicationPhysicalPath, + std::vector &arguments, + bool expandDllPaths = false ); static std::optional GetAbsolutePathToDotnetFromProgramFiles(); +private: + + static + BOOL + IsDotnetExecutable( + const std::filesystem::path & dotnetPath + ); + + static + VOID + FindDotNetFolders( + const std::filesystem::path& path, + std::vector & pvFolders + ); + + static + std::wstring + FindHighestDotNetVersion( + std::vector & vFolders + ); + + static + std::filesystem::path + GetAbsolutePathToHostFxr( + const std::filesystem::path & dotnetPath + ); + static std::optional InvokeWhereToFindDotnet(); static - std::optional + std::filesystem::path GetAbsolutePathToDotnet( - _In_ const std::filesystem::path & applicationPath, - _In_ const std::filesystem::path & requestedPath + const std::filesystem::path & applicationPath, + const std::filesystem::path & requestedPath ); + + struct LocalFreeDeleter + { + void operator ()(LPWSTR* ptr) const + { + LocalFree(ptr); + } + }; }; diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp index e8f2983028..7445724d98 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp @@ -6,82 +6,48 @@ #include "hostfxr_utility.h" #include "debugutil.h" #include "exceptions.h" +#include "EventLog.h" HRESULT HOSTFXR_OPTIONS::Create( - _In_ PCWSTR pcwzExeLocation, + _In_ PCWSTR pcwzDotnetExePath, _In_ PCWSTR pcwzProcessPath, _In_ PCWSTR pcwzApplicationPhysicalPath, _In_ PCWSTR pcwzArguments, _Out_ std::unique_ptr& ppWrapper) { - STRU struHostFxrDllLocation; - STRU struExeAbsolutePath; - STRU struExeLocation; - BSTR* pwzArgv; - DWORD dwArgCount; + std::filesystem::path knownDotnetLocation; - if (pcwzExeLocation != NULL) + if (pcwzDotnetExePath != nullptr) { - RETURN_IF_FAILED(struExeLocation.Copy(pcwzExeLocation)); + knownDotnetLocation = pcwzDotnetExePath; } + try + { + std::filesystem::path hostFxrDllPath; + std::vector arguments; + HOSTFXR_UTILITY::GetHostFxrParameters( + pcwzProcessPath, + pcwzApplicationPhysicalPath, + pcwzArguments, + hostFxrDllPath, + knownDotnetLocation, + arguments); - // If the exe was not provided by the shim, reobtain the hostfxr parameters (which finds dotnet). - if (struExeLocation.IsEmpty()) - { - RETURN_IF_FAILED(HOSTFXR_UTILITY::GetHostFxrParameters( - pcwzProcessPath, - pcwzApplicationPhysicalPath, - pcwzArguments, - &struHostFxrDllLocation, - &struExeAbsolutePath, - &dwArgCount, - &pwzArgv)); + ppWrapper = std::make_unique(knownDotnetLocation, hostFxrDllPath, arguments); } - else if (HOSTFXR_UTILITY::IsDotnetExecutable(struExeLocation.QueryStr())) + catch (HOSTFXR_UTILITY::StartupParametersResolutionException &resolutionException) { - RETURN_IF_FAILED(HOSTFXR_UTILITY::ParseHostfxrArguments( - pcwzArguments, - pcwzExeLocation, - pcwzApplicationPhysicalPath, - &dwArgCount, - &pwzArgv)); - } - else - { - RETURN_IF_FAILED(HOSTFXR_UTILITY::GetStandaloneHostfxrParameters( - pcwzExeLocation, - pcwzApplicationPhysicalPath, - pcwzArguments, - &struHostFxrDllLocation, - &dwArgCount, - &pwzArgv)); - } + OBSERVE_CAUGHT_EXCEPTION(); - ppWrapper = std::make_unique(); - RETURN_IF_FAILED(ppWrapper->Populate(struHostFxrDllLocation.QueryStr(), struExeAbsolutePath.QueryStr(), dwArgCount, pwzArgv)); + EventLog::Error( + ASPNETCORE_EVENT_INPROCESS_START_ERROR, + ASPNETCORE_EVENT_INPROCESS_START_ERROR_MSG, + pcwzApplicationPhysicalPath, + resolutionException.get_message().c_str()); + + return E_FAIL; + } + CATCH_RETURN(); return S_OK; } - - -HRESULT HOSTFXR_OPTIONS::Populate(PCWSTR hostFxrLocation, PCWSTR struExeLocation, DWORD argc, BSTR argv[]) -{ - HRESULT hr; - - m_argc = argc; - m_argv = argv; - - if (FAILED(hr = m_hostFxrLocation.Copy(hostFxrLocation))) - { - goto Finished; - } - - if (FAILED(hr = m_exeLocation.Copy(struExeLocation))) - { - goto Finished; - } - - Finished: - - return hr; -} diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h index 99b972cde8..50bdfe1ded 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxroptions.h @@ -5,41 +5,45 @@ #include #include - -#include "stringu.h" +#include +#include +#include +#include class HOSTFXR_OPTIONS { public: - HOSTFXR_OPTIONS() {} + HOSTFXR_OPTIONS( + std::filesystem::path dotnetExeLocation, + std::filesystem::path hostFxrLocation, + std::vector arguments + ) + : m_dotnetExeLocation(std::move(dotnetExeLocation)), + m_hostFxrLocation(std::move(hostFxrLocation)), + m_arguments(std::move(arguments)) + {} - ~HOSTFXR_OPTIONS() + void + GetArguments(DWORD &hostfxrArgc, std::unique_ptr &hostfxrArgv) const { - delete[] m_argv; + hostfxrArgc = static_cast(m_arguments.size()); + hostfxrArgv = std::unique_ptr(new PCWSTR[hostfxrArgc]); + for (DWORD i = 0; i < hostfxrArgc; ++i) + { + hostfxrArgv[i] = m_arguments[i].c_str(); + } } - DWORD - GetArgc() const - { - return m_argc; - } - - BSTR* - GetArgv() const - { - return m_argv; - } - - PCWSTR + const std::filesystem::path& GetHostFxrLocation() const { - return m_hostFxrLocation.QueryStr(); + return m_hostFxrLocation; } - PCWSTR - GetExeLocation() const + const std::filesystem::path& + GetDotnetExeLocation() const { - return m_exeLocation.QueryStr(); + return m_dotnetExeLocation; } static @@ -51,12 +55,7 @@ public: _Out_ std::unique_ptr& ppWrapper); private: - - HRESULT Populate(PCWSTR hostFxrLocation, PCWSTR struExeLocation, DWORD argc, BSTR argv[]); - - STRU m_exeLocation; - STRU m_hostFxrLocation; - - DWORD m_argc; - BSTR* m_argv; + const std::filesystem::path m_dotnetExeLocation; + const std::filesystem::path m_hostFxrLocation; + const std::vector m_arguments; }; diff --git a/src/AspNetCoreModuleV2/CommonLib/resources.h b/src/AspNetCoreModuleV2/CommonLib/resources.h index 82f22c52c0..b2ea6f5b85 100644 --- a/src/AspNetCoreModuleV2/CommonLib/resources.h +++ b/src/AspNetCoreModuleV2/CommonLib/resources.h @@ -11,7 +11,6 @@ #define ASPNETCORE_EVENT_PROVIDER L"IIS AspNetCore Module V2" #define ASPNETCORE_IISEXPRESS_EVENT_PROVIDER L"IIS Express AspNetCore Module V2" -#define ASPNETCORE_EVENT_MSG_BUFFER_SIZE 256 #define ASPNETCORE_EVENT_PROCESS_START_SUCCESS_MSG L"Application '%s' started process '%d' successfully and process '%d' is listening on port '%d'." #define ASPNETCORE_EVENT_RAPID_FAIL_COUNT_EXCEEDED_MSG L"Maximum rapid fail count per minute of '%d' exceeded." #define ASPNETCORE_EVENT_PROCESS_START_ERROR_MSG L"Application '%s' with physical root '%s' failed to start process with commandline '%s' at stage '%s', ErrorCode = '0x%x', assigned port %d, retryCounter '%d'." @@ -39,15 +38,9 @@ #define ASPNETCORE_EVENT_RECYCLE_CONFIGURATION_MSG L"Application '%s' was recycled due to configuration change" #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_INPROCESS_FULL_FRAMEWORK_APP_MSG L"Application '%s' was compiled for .NET Framework. Please compile for .NET core to run the inprocess application or change the process mode to out of process. ErrorCode = '0x%x'." -#define ASPNETCORE_EVENT_PORTABLE_APP_DOTNET_MISSING_MSG L"Could not find dotnet.exe on the system PATH environment variable for portable application '%s'. Check that a valid path to dotnet is on the PATH and the bitness of dotnet matches the bitness of the IIS worker process. ErrorCode = '0x%x'." -#define ASPNETCORE_EVENT_HOSTFXR_DIRECTORY_NOT_FOUND_MSG L"Could not find the hostfxr directory '%s' in the dotnet directory. ErrorCode = '0x%x'." -#define ASPNETCORE_EVENT_HOSTFXR_DLL_NOT_FOUND_MSG L"Could not find hostfxr.dll in '%s'. ErrorCode = '0x%x'." #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_APPLICATION_EXE_NOT_FOUND_LEVEL EVENTLOG_ERROR_TYPE -#define ASPNETCORE_EVENT_APPLICATION_EXE_NOT_FOUND_MSG L"Could not find application executable in '%s'. ErrorCode = '0x%x'." #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_INVALID_PROCESS_PATH_MSG L"Invalid or unknown processPath provided in web.config: processPath = '%s', ErrorCode = '0x%x'." #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." +#define ASPNETCORE_EVENT_INPROCESS_START_ERROR_MSG L"Application '%s' wasn't able to start. %s" diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp index 56eb63577c..182ee53148 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp @@ -392,11 +392,10 @@ IN_PROCESS_APPLICATION::ExecuteApplication( { HRESULT hr; HMODULE hModule = nullptr; - DWORD hostfxrArgc = 0; - BSTR *hostfxrArgv = NULL; 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; @@ -428,10 +427,7 @@ IN_PROCESS_APPLICATION::ExecuteApplication( m_pConfig->QueryArguments()->QueryStr(), hostFxrOptions )); - - hostfxrArgc = hostFxrOptions->GetArgc(); - hostfxrArgv = hostFxrOptions->GetArgv(); - + hostFxrOptions->GetArguments(hostfxrArgc, hostfxrArgv); FINISHED_IF_FAILED(SetEnvironementVariablesOnWorkerProcess()); } @@ -457,7 +453,7 @@ IN_PROCESS_APPLICATION::ExecuteApplication( // set the callbacks s_Application = this; - hr = RunDotnetApplication(hostfxrArgc, hostfxrArgv, pProc); + hr = RunDotnetApplication(hostfxrArgc, hostfxrArgv.get(), pProc); Finished: diff --git a/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs b/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs index c449863e4b..c4ca40436a 100644 --- a/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs +++ b/test/Common.FunctionalTests/Utilities/EventLogHelpers.cs @@ -1,7 +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.Linq; using System.Text; using System.Text.RegularExpressions; using Microsoft.AspNetCore.Server.IntegrationTesting.IIS; @@ -12,6 +11,7 @@ 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) { Assert.True(deploymentResult.HostProcess.HasExited); @@ -23,9 +23,19 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests builder.Append(context.Message); } - var eventLogRegex = new Regex($"Event Log: (.*?){expectedRegexMatchString}(.*?)End Event Log Message.", RegexOptions.Singleline); + 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)) + { + count++; + } + } - Assert.Matches(eventLogRegex, builder.ToString()); + Assert.True(count > 0, $"'{expectedRegexMatchString}' didn't match any event log messaged"); + Assert.True(count < 2, $"'{expectedRegexMatchString}' matched more then one event log message"); } } } diff --git a/test/Common.FunctionalTests/Utilities/FunctionalTestsBase.cs b/test/Common.FunctionalTests/Utilities/FunctionalTestsBase.cs index b9e80dd27f..e0d49615a5 100644 --- a/test/Common.FunctionalTests/Utilities/FunctionalTestsBase.cs +++ b/test/Common.FunctionalTests/Utilities/FunctionalTestsBase.cs @@ -28,11 +28,6 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting parameters.EnvironmentVariables[DebugEnvironmentVariable] = "console"; } - if (parameters.ApplicationPublisher == null) - { - throw new InvalidOperationException("All tests should use ApplicationPublisher"); - } - return IISApplicationDeployerFactory.Create(parameters, LoggerFactory); } diff --git a/test/CommonLibTests/hostfxr_utility_tests.cpp b/test/CommonLibTests/hostfxr_utility_tests.cpp index 232f7627b8..7d323868f8 100644 --- a/test/CommonLibTests/hostfxr_utility_tests.cpp +++ b/test/CommonLibTests/hostfxr_utility_tests.cpp @@ -2,80 +2,75 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. #include "stdafx.h" -#include "file_utility.h" +#include +#include +#include +#include "hostfxr_utility.h" +#include "Environment.h" TEST(ParseHostFxrArguments, BasicHostFxrArguments) { - DWORD retVal = 0; - BSTR* bstrArray; + std::vector bstrArray; PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; - HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( + + HOSTFXR_UTILITY::ParseHostfxrArguments( L"exec \"test.dll\"", // args exeStr, // exe path L"invalid", // physical path to application - &retVal, // arg count - &bstrArray); // args array. + bstrArray); // args array. - EXPECT_EQ(hr, S_OK); - EXPECT_EQ(DWORD(3), retVal); - ASSERT_STREQ(exeStr, bstrArray[0]); - ASSERT_STREQ(L"exec", bstrArray[1]); - ASSERT_STREQ(L"test.dll", bstrArray[2]); + EXPECT_EQ(3, bstrArray.size()); + ASSERT_STREQ(exeStr, bstrArray[0].c_str()); + ASSERT_STREQ(L"exec", bstrArray[1].c_str()); + ASSERT_STREQ(L"test.dll", bstrArray[2].c_str()); } TEST(ParseHostFxrArguments, NoExecProvided) { - DWORD retVal = 0; - BSTR* bstrArray; + std::vector bstrArray; PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; - HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( + HOSTFXR_UTILITY::ParseHostfxrArguments( L"test.dll", // args exeStr, // exe path L"ignored", // physical path to application - &retVal, // arg count - &bstrArray); // args array. + bstrArray); // args array. - EXPECT_EQ(hr, S_OK); - EXPECT_EQ(DWORD(2), retVal); - ASSERT_STREQ(exeStr, bstrArray[0]); - ASSERT_STREQ(L"test.dll", bstrArray[1]); + EXPECT_EQ(DWORD(2), bstrArray.size()); + ASSERT_STREQ(exeStr, bstrArray[0].c_str()); + ASSERT_STREQ(L"test.dll", bstrArray[1].c_str()); } TEST(ParseHostFxrArguments, ConvertDllToAbsolutePath) { - DWORD retVal = 0; - BSTR* bstrArray; + std::vector bstrArray; PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; - - HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( - L"exec \"test.dll\"", // args + // we need to use existing dll so let's use ntdll that we know exists everywhere + auto system32 = Environment::ExpandEnvironmentVariables(L"%WINDIR%\\System32"); + HOSTFXR_UTILITY::ParseHostfxrArguments( + L"exec \"ntdll.dll\"", // args exeStr, // exe path - L"C:/test", // physical path to application - &retVal, // arg count - &bstrArray); // args array. + system32, // physical path to application + bstrArray, // args array. + true); // expandDllPaths - EXPECT_EQ(hr, S_OK); - EXPECT_EQ(DWORD(3), retVal); - ASSERT_STREQ(exeStr, bstrArray[0]); - ASSERT_STREQ(L"exec", bstrArray[1]); - ASSERT_STREQ(L"C:\\test\\test.dll", bstrArray[2]); + EXPECT_EQ(DWORD(3), bstrArray.size()); + ASSERT_STREQ(exeStr, bstrArray[0].c_str()); + ASSERT_STREQ(L"exec", bstrArray[1].c_str()); + ASSERT_STREQ((system32 + L"\\ntdll.dll").c_str(), bstrArray[2].c_str()); } TEST(ParseHostFxrArguments, ProvideNoArgs_InvalidArgs) { - DWORD retVal = 0; - BSTR* bstrArray; + std::vector bstrArray; PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; - HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( + ASSERT_THROW(HOSTFXR_UTILITY::ParseHostfxrArguments( L"", // args exeStr, // exe path L"ignored", // physical path to application - &retVal, // arg count - &bstrArray); // args array. - - EXPECT_EQ(E_INVALIDARG, hr); + bstrArray), // args array. + HOSTFXR_UTILITY::StartupParametersResolutionException); } TEST(GetAbsolutePathToDotnetFromProgramFiles, BackupWorks) @@ -118,19 +113,16 @@ TEST(GetAbsolutePathToDotnetFromProgramFiles, BackupWorks) TEST(GetHostFxrArguments, InvalidParams) { - DWORD retVal = 0; - BSTR* bstrArray; - STRU struHostFxrDllLocation; - STRU struExeLocation; + std::vector bstrArray; + std::filesystem::path struHostFxrDllLocation; + std::filesystem::path struExeLocation; - HRESULT hr = HOSTFXR_UTILITY::GetHostFxrParameters( + EXPECT_THROW(HOSTFXR_UTILITY::GetHostFxrParameters( L"bogus", // processPath L"", // application physical path, ignored. L"ignored", //arguments - NULL, - &struExeLocation, - &retVal, // arg count - &bstrArray); // args array. - - EXPECT_EQ(HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), hr); + struHostFxrDllLocation, + struExeLocation, + bstrArray), // args array. + HOSTFXR_UTILITY::StartupParametersResolutionException); } diff --git a/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs b/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs index e6b48f28bf..73bea61017 100644 --- a/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs +++ b/test/IISExpress.FunctionalTests/InProcess/StartupTests.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Linq; using System.Net; +using System.Text.RegularExpressions; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities; using Microsoft.AspNetCore.Server.IntegrationTesting; @@ -40,14 +41,16 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests } [ConditionalTheory] - [InlineData("bogus")] - [InlineData("c:\\random files\\dotnet.exe")] - [InlineData(".\\dotnet.exe")] - public async Task InvalidProcessPath_ExpectServerError(string path) + [InlineData("bogus", "", @"Executable was not found at '.*?\\bogus.exe")] + [InlineData("c:\\random files\\dotnet.exe", "something.dll", @"Could not find dotnet.exe at '.*?\\dotnet.exe'")] + [InlineData(".\\dotnet.exe", "something.dll", @"Could not find dotnet.exe at '.*?\\.\\dotnet.exe'")] + [InlineData("dotnet.exe", "", @"Application arguments are empty.")] + [InlineData("dotnet.zip", "", @"Process path 'dotnet.zip' doesn't have '.exe' extension.")] + public async Task InvalidProcessPath_ExpectServerError(string path, string arguments, string subError) { var deploymentParameters = _fixture.GetBaseDeploymentParameters(publish: true); deploymentParameters.WebConfigActionList.Add(WebConfigHelpers.AddOrModifyAspNetCoreSection("processPath", path)); - + deploymentParameters.WebConfigActionList.Add(WebConfigHelpers.AddOrModifyAspNetCoreSection("arguments", arguments)); var deploymentResult = await DeployAsync(deploymentParameters); @@ -57,7 +60,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests StopServer(); - EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, @"Invalid or unknown processPath provided in web\.config: processPath = '.+', ErrorCode = '0x80070002'\."); + EventLogHelpers.VerifyEventLogEvent(deploymentResult, TestSink, $@"Application '{Regex.Escape(deploymentResult.ContentRoot)}\\' wasn't able to start. {subError}"); } [ConditionalFact] @@ -136,6 +139,24 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal("Hello World", responseText); } + [ConditionalFact] + public async Task StartsWithPortableAndBootstraperExe() + { + var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.InProcessTestSite, publish: true); + // rest publisher as it doesn't support additional parameters + deploymentParameters.ApplicationPublisher = null; + // ReferenceTestTasks is workaround for https://github.com/dotnet/sdk/issues/2482 + deploymentParameters.AdditionalPublishParameters = "-p:RuntimeIdentifier=win7-x64 -p:UseAppHost=true -p:SelfContained=false -p:ReferenceTestTasks=false"; + deploymentParameters.RestoreOnPublish = true; + var deploymentResult = await DeployAsync(deploymentParameters); + + Assert.True(File.Exists(Path.Combine(deploymentResult.ContentRoot, "InProcessWebSite.exe"))); + Assert.False(File.Exists(Path.Combine(deploymentResult.ContentRoot, "hostfxr.dll"))); + Assert.Contains("InProcessWebSite.exe", File.ReadAllText(Path.Combine(deploymentResult.ContentRoot, "web.config"))); + + await deploymentResult.AssertStarts(); + } + [ConditionalFact] public async Task DetectsOveriddenServer() { From 3b3f1283063db02821da4cd76e5c12be7dfc933b Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 22 Aug 2018 15:39:59 -0700 Subject: [PATCH 4/4] Enable app verifier on VSTS; Fix debug log handle use (#1291) --- src/AspNetCoreModuleV2/CommonLib/debugutil.cpp | 6 ++---- test/Common.FunctionalTests/AppOfflineTests.cs | 2 +- tools/SetupTestEnvironment.ps1 | 11 ++++++----- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp index 7184dbfde8..45e1d7ceab 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp @@ -14,7 +14,6 @@ #include "atlbase.h" #include "config_utility.h" -inline HANDLE g_hStandardOutput = INVALID_HANDLE_VALUE; inline HANDLE g_logFile = INVALID_HANDLE_VALUE; inline HMODULE g_hModule; inline SRWLOCK g_logFileLock; @@ -149,7 +148,6 @@ VOID DebugInitialize(HMODULE hModule) { g_hModule = hModule; - g_hStandardOutput = GetStdHandle(STD_OUTPUT_HANDLE); HKEY hKey; InitializeSRWLock(&g_logFileLock); @@ -290,10 +288,10 @@ DebugPrint( OutputDebugStringA( strOutput.QueryStr() ); DWORD nBytesWritten = 0; - if (IsEnabled(ASPNETCORE_DEBUG_FLAG_CONSOLE)) { - WriteFile(g_hStandardOutput, strOutput.QueryStr(), strOutput.QueryCB(), &nBytesWritten, nullptr); + auto outputHandle = GetStdHandle(STD_OUTPUT_HANDLE); + WriteFile(outputHandle, strOutput.QueryStr(), strOutput.QueryCB(), &nBytesWritten, nullptr); } if (g_logFile != INVALID_HANDLE_VALUE) diff --git a/test/Common.FunctionalTests/AppOfflineTests.cs b/test/Common.FunctionalTests/AppOfflineTests.cs index 41bf8874d9..53afd34504 100644 --- a/test/Common.FunctionalTests/AppOfflineTests.cs +++ b/test/Common.FunctionalTests/AppOfflineTests.cs @@ -81,7 +81,7 @@ namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests DeletePublishOutput(deploymentResult); } - [ConditionalFact] + [ConditionalFact(Skip = "https://github.com/aspnet/IISIntegration/issues/933")] public async Task AppOfflineDroppedWhileSiteFailedToStartInRequestHandler_SiteStops_InProcess() { var deploymentResult = await DeployApp(HostingModel.InProcess); diff --git a/tools/SetupTestEnvironment.ps1 b/tools/SetupTestEnvironment.ps1 index 6fabf1704b..cc9fbed394 100644 --- a/tools/SetupTestEnvironment.ps1 +++ b/tools/SetupTestEnvironment.ps1 @@ -3,7 +3,7 @@ param($Mode) function Setup-appverif($application) { appverif.exe -enable Exceptions Handles Heaps Leak Locks Memory Threadpool TLS SRWLock -for $application - $onlyLog = 0x181; + $onlyLog = 0x1E1; $codes = @( # Exceptions 0x650, @@ -62,8 +62,8 @@ if (!(Test-Path $cdb)) if ($Mode -eq "Setup") { - #Setup-appverif w3wp.exe - #Setup-appverif iisexpress.exe + Setup-appverif w3wp.exe + Setup-appverif iisexpress.exe if (!(Test-Path $ldHive )) { @@ -81,14 +81,15 @@ if ($Mode -eq "Shutdown") { Remove-Item $ldHive -Recurse -Force - #Shutdown-appverif w3wp.exe - #Shutdown-appverif iisexpress.exe + Shutdown-appverif w3wp.exe + Shutdown-appverif iisexpress.exe foreach ($dump in (Get-ChildItem -Path $DumpFolder -Filter "*.dmp")) { if (Test-Path $cdb) { & $cdb -z $dump.FullName -y "https://msdl.microsoft.com/download/symbols" -c ".loadby sos coreclr;!sym noisy;.reload /f;.dumpcab -a $($dump.FullName).cab;q;" + Remove-Item $dump.FullName } } }