From 074264cd3a1a1558f9a88f3c580df915f086f516 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Tue, 17 Jul 2018 08:56:18 -0700 Subject: [PATCH] Use less global variables (#1064) --- .../AspNetCore/applicationinfo.cpp | 69 +++++++++---------- .../AspNetCore/applicationinfo.h | 23 +++++-- .../AspNetCore/applicationmanager.cpp | 55 +++++---------- .../AspNetCore/applicationmanager.h | 33 +++++---- src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp | 24 +------ .../AspNetCore/globalmodule.cpp | 2 + .../AspNetCore/proxymodule.cpp | 4 +- .../AspNetCore/proxymodule.h | 2 - src/AspNetCoreModuleV2/AspNetCore/stdafx.h | 16 +---- src/AspNetCoreModuleV2/CommonLib/EventLog.h | 2 + 10 files changed, 100 insertions(+), 130 deletions(-) diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp index 2072e33af6..edfc085f22 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp @@ -12,12 +12,21 @@ #include "SRWExclusiveLock.h" #include "GlobalVersionUtility.h" #include "exceptions.h" -#include "HandleWrapper.h" #include "PollingAppOfflineApplication.h" +#include "EventLog.h" + +extern HINSTANCE g_hModule; const PCWSTR APPLICATION_INFO::s_pwzAspnetcoreInProcessRequestHandlerName = L"aspnetcorev2_inprocess.dll"; const PCWSTR APPLICATION_INFO::s_pwzAspnetcoreOutOfProcessRequestHandlerName = L"aspnetcorev2_outofprocess.dll"; +SRWLOCK APPLICATION_INFO::s_requestHandlerLoadLock {}; +bool APPLICATION_INFO::s_fAspnetcoreRHAssemblyLoaded = false; +bool APPLICATION_INFO::s_fAspnetcoreRHLoadedError = false; +HMODULE APPLICATION_INFO::s_hAspnetCoreRH = nullptr; + +PFN_ASPNETCORE_CREATE_APPLICATION APPLICATION_INFO::s_pfnAspNetCoreCreateApplication = nullptr; + APPLICATION_INFO::~APPLICATION_INFO() { if (m_pApplication != NULL) @@ -39,23 +48,14 @@ APPLICATION_INFO::~APPLICATION_INFO() HRESULT APPLICATION_INFO::Initialize( - _In_ IHttpServer *pServer, - _In_ IHttpApplication *pApplication + _In_ IHttpApplication &pApplication ) { - HRESULT hr = S_OK; + m_pConfiguration = new ASPNETCORE_SHIM_CONFIG(); + RETURN_IF_FAILED(m_pConfiguration->Populate(&m_pServer, &pApplication)); + RETURN_IF_FAILED(m_struInfoKey.Copy(pApplication.GetApplicationId())); - DBG_ASSERT(pServer); - DBG_ASSERT(pApplication); - - // todo: make sure Initialize should be called only once - m_pServer = pServer; - FINISHED_IF_NULL_ALLOC(m_pConfiguration = new ASPNETCORE_SHIM_CONFIG()); - FINISHED_IF_FAILED(m_pConfiguration->Populate(m_pServer, pApplication)); - FINISHED_IF_FAILED(m_struInfoKey.Copy(pApplication->GetApplicationId())); - -Finished: - return hr; + return S_OK; } @@ -125,7 +125,7 @@ APPLICATION_INFO::EnsureApplicationCreated( }; LOG_INFO("Creating handler application"); FINISHED_IF_FAILED(m_pfnAspNetCoreCreateApplication( - m_pServer, + &m_pServer, pHttpContext->GetApplication(), parameters.data(), static_cast(parameters.size()), @@ -157,19 +157,18 @@ APPLICATION_INFO::FindRequestHandlerAssembly(STRU& location) PCWSTR pstrHandlerDllName; STACK_STRU(struFileName, 256); - if (g_fAspnetcoreRHLoadedError) + if (s_fAspnetcoreRHLoadedError) { FINISHED(E_APPLICATION_ACTIVATION_EXEC_FAILURE); } - else if (!g_fAspnetcoreRHAssemblyLoaded) + else if (!s_fAspnetcoreRHAssemblyLoaded) { - SRWExclusiveLock lock(g_srwLock); - - if (g_fAspnetcoreRHLoadedError) + SRWExclusiveLock lock(s_requestHandlerLoadLock); + if (s_fAspnetcoreRHLoadedError) { FINISHED(E_APPLICATION_ACTIVATION_EXEC_FAILURE); } - if (g_fAspnetcoreRHAssemblyLoaded) + if (s_fAspnetcoreRHAssemblyLoaded) { FINISHED(S_OK); } @@ -184,9 +183,9 @@ APPLICATION_INFO::FindRequestHandlerAssembly(STRU& location) } // Try to see if RH is already loaded - g_hAspnetCoreRH = GetModuleHandle(pstrHandlerDllName); + s_hAspnetCoreRH = GetModuleHandle(pstrHandlerDllName); - if (g_hAspnetCoreRH == NULL) + if (s_hAspnetCoreRH == NULL) { if (m_pConfiguration->QueryHostingModel() == APP_HOSTING_MODEL::HOSTING_IN_PROCESS) { @@ -229,22 +228,22 @@ APPLICATION_INFO::FindRequestHandlerAssembly(STRU& location) WLOG_INFOF(L"Loading request handler: %s", struFileName.QueryStr()); - g_hAspnetCoreRH = LoadLibraryW(struFileName.QueryStr()); + s_hAspnetCoreRH = LoadLibraryW(struFileName.QueryStr()); - if (g_hAspnetCoreRH == NULL) + if (s_hAspnetCoreRH == NULL) { FINISHED(HRESULT_FROM_WIN32(GetLastError())); } } - g_pfnAspNetCoreCreateApplication = (PFN_ASPNETCORE_CREATE_APPLICATION) - GetProcAddress(g_hAspnetCoreRH, "CreateApplication"); - if (g_pfnAspNetCoreCreateApplication == NULL) + s_pfnAspNetCoreCreateApplication = (PFN_ASPNETCORE_CREATE_APPLICATION) + GetProcAddress(s_hAspnetCoreRH, "CreateApplication"); + if (s_pfnAspNetCoreCreateApplication == NULL) { FINISHED(HRESULT_FROM_WIN32(GetLastError())); } - g_fAspnetcoreRHAssemblyLoaded = TRUE; + s_fAspnetcoreRHAssemblyLoaded = TRUE; } Finished: @@ -252,11 +251,11 @@ Finished: // Question: we remember the load failure so that we will not try again. // User needs to check whether the fuction pointer is NULL // - m_pfnAspNetCoreCreateApplication = g_pfnAspNetCoreCreateApplication; + m_pfnAspNetCoreCreateApplication = s_pfnAspNetCoreCreateApplication; - if (!g_fAspnetcoreRHLoadedError && FAILED(hr)) + if (!s_fAspnetcoreRHLoadedError && FAILED(hr)) { - g_fAspnetcoreRHLoadedError = TRUE; + s_fAspnetcoreRHLoadedError = TRUE; } return hr; } @@ -457,7 +456,7 @@ APPLICATION_INFO::RecycleApplication() if (m_pConfiguration->QueryHostingModel() == HOSTING_IN_PROCESS) { // In process application failed to start for whatever reason, need to recycle the work process - m_pServer->RecycleProcess(L"AspNetCore InProcess Recycle Process on Demand"); + m_pServer.RecycleProcess(L"AspNetCore InProcess Recycle Process on Demand"); } } @@ -466,7 +465,7 @@ APPLICATION_INFO::RecycleApplication() if (!g_fRecycleProcessCalled) { g_fRecycleProcessCalled = TRUE; - g_pHttpServer->RecycleProcess(L"On Demand by AspNetCore Module for recycle application failure"); + m_pServer.RecycleProcess(L"On Demand by AspNetCore Module for recycle application failure"); } } else diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h index 3d4b901755..49badd578a 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationinfo.h @@ -23,14 +23,13 @@ HRESULT ); extern BOOL g_fRecycleProcessCalled; -extern PFN_ASPNETCORE_CREATE_APPLICATION g_pfnAspNetCoreCreateApplication; class APPLICATION_INFO { public: - APPLICATION_INFO() : - m_pServer(NULL), + APPLICATION_INFO(_In_ IHttpServer &pServer) : + m_pServer(pServer), m_cRefs(1), m_fValid(FALSE), m_fAppCreationAttempted(FALSE), @@ -48,11 +47,17 @@ public: virtual ~APPLICATION_INFO(); + + static + void + StaticInitialize() + { + InitializeSRWLock(&s_requestHandlerLoadLock); + } HRESULT Initialize( - _In_ IHttpServer *pServer, - _In_ IHttpApplication *pApplication + _In_ IHttpApplication &pApplication ); VOID @@ -125,11 +130,17 @@ private: ASPNETCORE_SHIM_CONFIG *m_pConfiguration; IAPPLICATION *m_pApplication; SRWLOCK m_srwLock; - IHttpServer *m_pServer; + IHttpServer &m_pServer; PFN_ASPNETCORE_CREATE_APPLICATION m_pfnAspNetCoreCreateApplication; static const PCWSTR s_pwzAspnetcoreInProcessRequestHandlerName; static const PCWSTR s_pwzAspnetcoreOutOfProcessRequestHandlerName; + + static SRWLOCK s_requestHandlerLoadLock; + static bool s_fAspnetcoreRHAssemblyLoaded; + static bool s_fAspnetcoreRHLoadedError; + static HMODULE s_hAspnetCoreRH; + static PFN_ASPNETCORE_CREATE_APPLICATION s_pfnAspNetCoreCreateApplication; }; class APPLICATION_INFO_HASH : diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp index c65222b10e..a7f08e5d3e 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp @@ -8,6 +8,9 @@ #include "resources.h" #include "SRWExclusiveLock.h" #include "exceptions.h" +#include "EventLog.h" + +extern BOOL g_fInShutdown; // The application manager is a singleton across ANCM. APPLICATION_MANAGER* APPLICATION_MANAGER::sm_pApplicationManager = NULL; @@ -18,7 +21,6 @@ APPLICATION_MANAGER* APPLICATION_MANAGER::sm_pApplicationManager = NULL; // HRESULT APPLICATION_MANAGER::GetOrCreateApplicationInfo( - _In_ IHttpServer* pServer, _In_ IHttpContext* pHttpContext, _Out_ APPLICATION_INFO ** ppApplicationInfo ) @@ -32,56 +34,37 @@ APPLICATION_MANAGER::GetOrCreateApplicationInfo( STACK_STRU ( strEventMsg, 256 ); - DBG_ASSERT(pServer); DBG_ASSERT(pHttpContext); DBG_ASSERT(ppApplicationInfo); *ppApplicationInfo = NULL; + IHttpApplication &pApplication = *pHttpContext->GetApplication(); - if (!m_fDebugInitialize) - { - SRWExclusiveLock lock(m_srwLock); - if (!m_fDebugInitialize) - { - DebugInitializeFromConfig(*pServer, *pHttpContext->GetApplication()); - m_fDebugInitialize = TRUE; - } - } // The configuration path is unique for each application and is used for the // key in the applicationInfoHash. - pszApplicationId = pHttpContext->GetApplication()->GetApplicationId(); + pszApplicationId = pApplication.GetApplicationId(); // When accessing the m_pApplicationInfoHash, we need to acquire the application manager // lock to avoid races on setting state. + SRWSharedLock lock(m_srwLock); + if (!m_fDebugInitialize) { - SRWSharedLock lock(m_srwLock); - if (g_fInShutdown) - { - FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS)); - } - m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo); + DebugInitializeFromConfig(m_pHttpServer, pApplication); + m_fDebugInitialize = TRUE; } + if (g_fInShutdown) + { + FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS)); + } + + m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo); + if (*ppApplicationInfo == NULL) { - pApplicationInfo = new APPLICATION_INFO(); + pApplicationInfo = new APPLICATION_INFO(m_pHttpServer); - FINISHED_IF_FAILED(pApplicationInfo->Initialize(pServer, pHttpContext->GetApplication())); - - SRWExclusiveLock lock(m_srwLock); - - if (g_fInShutdown) - { - // Already in shuting down. No need to create the application - FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS)); - } - m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo); - - if (*ppApplicationInfo != NULL) - { - // someone else created the application - FINISHED(S_OK); - } + FINISHED_IF_FAILED(pApplicationInfo->Initialize(pApplication)); hostingModel = pApplicationInfo->QueryConfig()->QueryHostingModel(); @@ -309,7 +292,7 @@ Finished: if (!g_fRecycleProcessCalled) { g_fRecycleProcessCalled = TRUE; - g_pHttpServer->RecycleProcess(L"AspNetCore Recycle Process on Demand Due Application Recycle Error"); + m_pHttpServer.RecycleProcess(L"AspNetCore Recycle Process on Demand Due Application Recycle Error"); } } diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h index cd565e2c01..f01df15844 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h +++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.h @@ -27,15 +27,9 @@ public: static APPLICATION_MANAGER* - GetInstance( - VOID - ) + GetInstance() { - if ( sm_pApplicationManager == NULL ) - { - sm_pApplicationManager = new APPLICATION_MANAGER(); - } - + assert(sm_pApplicationManager); return sm_pApplicationManager; } @@ -68,7 +62,6 @@ public: HRESULT GetOrCreateApplicationInfo( - _In_ IHttpServer* pServer, _In_ IHttpContext* pHttpContext, _Out_ APPLICATION_INFO ** ppApplicationInfo ); @@ -92,6 +85,16 @@ public: } } + static HRESULT StaticInitialize(IHttpServer& pHttpServer) + { + assert(!sm_pApplicationManager); + sm_pApplicationManager = new APPLICATION_MANAGER(pHttpServer); + RETURN_IF_FAILED(sm_pApplicationManager->Initialize()); + + APPLICATION_INFO::StaticInitialize(); + return S_OK; + } + HRESULT Initialize() { if(m_pApplicationInfoHash == NULL) @@ -107,12 +110,11 @@ public: } private: - // - // we currently limit the size of m_pstrErrorInfo to 5000, be careful if you want to change its payload - // - APPLICATION_MANAGER() : m_pApplicationInfoHash(NULL), + APPLICATION_MANAGER(IHttpServer& pHttpServer) : + m_pApplicationInfoHash(NULL), m_hostingModel(HOSTING_UNKNOWN), - m_fDebugInitialize(FALSE) + m_fDebugInitialize(FALSE), + m_pHttpServer(pHttpServer) { InitializeSRWLock(&m_srwLock); } @@ -120,6 +122,7 @@ private: APPLICATION_INFO_HASH *m_pApplicationInfoHash; static APPLICATION_MANAGER *sm_pApplicationManager; SRWLOCK m_srwLock; - APP_HOSTING_MODEL m_hostingModel; + APP_HOSTING_MODEL m_hostingModel; BOOL m_fDebugInitialize; + IHttpServer &m_pHttpServer; }; diff --git a/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp b/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp index d185ad4be4..5369fbc527 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/dllmain.cpp @@ -13,20 +13,10 @@ DECLARE_DEBUG_PRINT_OBJECT("aspnetcorev2.dll"); -HTTP_MODULE_ID g_pModuleId = NULL; -IHttpServer * g_pHttpServer = NULL; HANDLE g_hEventLog = NULL; BOOL g_fRecycleProcessCalled = FALSE; -PCWSTR g_pszModuleName = NULL; HINSTANCE g_hModule; -HMODULE g_hAspnetCoreRH = NULL; -BOOL g_fAspnetcoreRHAssemblyLoaded = FALSE; -BOOL g_fAspnetcoreRHLoadedError = FALSE; BOOL g_fInShutdown = FALSE; -DWORD g_dwActiveServerProcesses = 0; -SRWLOCK g_srwLock; - -PFN_ASPNETCORE_CREATE_APPLICATION g_pfnAspNetCoreCreateApplication; VOID StaticCleanup() @@ -100,17 +90,10 @@ HRESULT BOOL fDisableANCM = FALSE; ASPNET_CORE_PROXY_MODULE_FACTORY * pFactory = NULL; ASPNET_CORE_GLOBAL_MODULE * pGlobalModule = NULL; - APPLICATION_MANAGER * pApplicationManager = NULL; UNREFERENCED_PARAMETER(dwServerVersion); - InitializeSRWLock(&g_srwLock); - - g_pModuleId = pModuleInfo->GetId(); - g_pszModuleName = pModuleInfo->GetName(); - g_pHttpServer = pHttpServer; - - if (g_pHttpServer->IsCommandLineLaunch()) + if (pHttpServer->IsCommandLineLaunch()) { g_hEventLog = RegisterEventSource(NULL, ASPNETCORE_IISEXPRESS_EVENT_PROVIDER); } @@ -175,13 +158,12 @@ HRESULT 0)); pFactory = NULL; - pApplicationManager = APPLICATION_MANAGER::GetInstance(); - FINISHED_IF_FAILED(pApplicationManager->Initialize()); + FINISHED_IF_FAILED(APPLICATION_MANAGER::StaticInitialize(*pHttpServer)); pGlobalModule = NULL; - pGlobalModule = new ASPNET_CORE_GLOBAL_MODULE(pApplicationManager); + pGlobalModule = new ASPNET_CORE_GLOBAL_MODULE(APPLICATION_MANAGER::GetInstance()); FINISHED_IF_FAILED(pModuleInfo->SetGlobalNotifications( pGlobalModule, diff --git a/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp b/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp index 05dee36b15..0ecbba4afb 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/globalmodule.cpp @@ -3,6 +3,8 @@ #include "globalmodule.h" +extern BOOL g_fInShutdown; + ASPNET_CORE_GLOBAL_MODULE::ASPNET_CORE_GLOBAL_MODULE( APPLICATION_MANAGER* pApplicationManager) { diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp index f27a1571c3..962d7202d1 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp @@ -7,6 +7,9 @@ #include "applicationinfo.h" #include "acache.h" #include "exceptions.h" + +extern BOOL g_fInShutdown; + __override HRESULT ASPNET_CORE_PROXY_MODULE_FACTORY::GetHttpModule( @@ -83,7 +86,6 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( pApplicationManager = APPLICATION_MANAGER::GetInstance(); FINISHED_IF_FAILED(pApplicationManager->GetOrCreateApplicationInfo( - g_pHttpServer, pHttpContext, &m_pApplicationInfo)); diff --git a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h index 5864dd0c6f..a66a0fd3e1 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h +++ b/src/AspNetCoreModuleV2/AspNetCore/proxymodule.h @@ -8,8 +8,6 @@ #include "irequesthandler.h" extern HTTP_MODULE_ID g_pModuleId; -extern IHttpServer *g_pHttpServer; -extern HMODULE g_hAspnetCoreRH; class ASPNET_CORE_PROXY_MODULE : public CHttpModule { diff --git a/src/AspNetCoreModuleV2/AspNetCore/stdafx.h b/src/AspNetCoreModuleV2/AspNetCore/stdafx.h index 1191022c01..29c485bc9c 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/stdafx.h +++ b/src/AspNetCoreModuleV2/AspNetCore/stdafx.h @@ -9,8 +9,8 @@ // #define WIN32_LEAN_AND_MEAN -#define NTDDI_VERSION 0x06010000 -#define WINVER 0x0601 +#define NTDDI_VERSION NTDDI_WIN7 +#define WINVER _WIN32_WINNT_WIN7 #define _WIN32_WINNT 0x0601 #include @@ -20,16 +20,4 @@ #include "stringu.h" #include "stringa.h" -extern PVOID g_pModuleId; -extern BOOL g_fAspnetcoreRHAssemblyLoaded; -extern BOOL g_fAspnetcoreRHLoadedError; -extern BOOL g_fInShutdown; -extern BOOL g_fEnableReferenceCountTracing; -extern DWORD g_dwActiveServerProcesses; -extern HINSTANCE g_hModule; -extern HMODULE g_hAspnetCoreRH; -extern SRWLOCK g_srwLock; -extern PCWSTR g_pwzAspnetcoreRequestHandlerName; -extern HANDLE g_hEventLog; - #pragma warning( error : 4091) diff --git a/src/AspNetCoreModuleV2/CommonLib/EventLog.h b/src/AspNetCoreModuleV2/CommonLib/EventLog.h index 8782eff6d4..3efbe38b35 100644 --- a/src/AspNetCoreModuleV2/CommonLib/EventLog.h +++ b/src/AspNetCoreModuleV2/CommonLib/EventLog.h @@ -6,4 +6,6 @@ #include "Utility.h" #include "resources.h" +extern HANDLE g_hEventLog; + #define EVENTLOG(log, name, ...) UTILITY::LogEventF(log, ASPNETCORE_EVENT_ ## name ## _LEVEL, ASPNETCORE_EVENT_ ## name, ASPNETCORE_EVENT_ ## name ## _MSG, __VA_ARGS__)