From d63e8c5558fcd8edcb911e4b9de16ecb48650b3f Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Tue, 3 Apr 2018 12:50:24 -0700 Subject: [PATCH] CPP --- src/CommonLib/CommonLib.vcxproj | 2 + src/CommonLib/SRWLockWrapper.cpp | 13 + src/CommonLib/SRWLockWrapper.h | 9 + src/CommonLib/application.cpp | 2 +- src/CommonLib/application.h | 3 +- src/CommonLib/stdafx.h | 1 + .../inprocess/inprocessapplication.cpp | 274 ++++++++---------- .../inprocess/inprocessapplication.h | 2 + 8 files changed, 151 insertions(+), 155 deletions(-) create mode 100644 src/CommonLib/SRWLockWrapper.cpp create mode 100644 src/CommonLib/SRWLockWrapper.h diff --git a/src/CommonLib/CommonLib.vcxproj b/src/CommonLib/CommonLib.vcxproj index dfcc6725bb..1ab1ef971c 100644 --- a/src/CommonLib/CommonLib.vcxproj +++ b/src/CommonLib/CommonLib.vcxproj @@ -179,6 +179,7 @@ + @@ -188,6 +189,7 @@ + Create Create diff --git a/src/CommonLib/SRWLockWrapper.cpp b/src/CommonLib/SRWLockWrapper.cpp new file mode 100644 index 0000000000..3fd0be51f4 --- /dev/null +++ b/src/CommonLib/SRWLockWrapper.cpp @@ -0,0 +1,13 @@ +#include "stdafx.h" +#include "SRWLockWrapper.h" + +SRWLockWrapper::SRWLockWrapper(const SRWLOCK& lock) + : m_lock(lock) +{ + AcquireSRWLockExclusive(const_cast(&m_lock)); +} + +SRWLockWrapper::~SRWLockWrapper() +{ + ReleaseSRWLockExclusive(const_cast(&m_lock)); +} diff --git a/src/CommonLib/SRWLockWrapper.h b/src/CommonLib/SRWLockWrapper.h new file mode 100644 index 0000000000..2ae57cb2f8 --- /dev/null +++ b/src/CommonLib/SRWLockWrapper.h @@ -0,0 +1,9 @@ +#pragma once +class SRWLockWrapper +{ +public: + SRWLockWrapper(const SRWLOCK& lock); + ~SRWLockWrapper(); +private: + const SRWLOCK& m_lock; +}; diff --git a/src/CommonLib/application.cpp b/src/CommonLib/application.cpp index 74b82c9a9b..b68c093266 100644 --- a/src/CommonLib/application.cpp +++ b/src/CommonLib/application.cpp @@ -7,10 +7,10 @@ APPLICATION::APPLICATION( _In_ IHttpServer* pHttpServer, _In_ ASPNETCORE_CONFIG* pConfig) : m_cRefs(1), - m_pHttpServer(pHttpServer), m_pConfig(pConfig), m_status(APPLICATION_STATUS::UNKNOWN) { + UNREFERENCED_PARAMETER(pHttpServer); } APPLICATION::~APPLICATION() diff --git a/src/CommonLib/application.h b/src/CommonLib/application.h index 9230218042..43c9dafd0c 100644 --- a/src/CommonLib/application.h +++ b/src/CommonLib/application.h @@ -18,7 +18,7 @@ class APPLICATION { public: APPLICATION( - _In_ IHttpServer* pHttpServer, + _In_ IHttpServer* pHttpServer, _In_ ASPNETCORE_CONFIG* pConfig); virtual @@ -49,6 +49,5 @@ public: protected: mutable LONG m_cRefs; volatile APPLICATION_STATUS m_status; - IHttpServer* m_pHttpServer; ASPNETCORE_CONFIG* m_pConfig; }; diff --git a/src/CommonLib/stdafx.h b/src/CommonLib/stdafx.h index 4f8b6d27d4..2ef05addcc 100644 --- a/src/CommonLib/stdafx.h +++ b/src/CommonLib/stdafx.h @@ -21,6 +21,7 @@ #include "..\IISLib\dbgutil.h" #include "..\IISLib\ahutil.h" #include "..\IISLib\hashfn.h" +#include "SRWLockWrapper.h" #include "environmentvariablehash.h" #include "utility.h" #include "aspnetcoreconfig.h" diff --git a/src/RequestHandler/inprocess/inprocessapplication.cpp b/src/RequestHandler/inprocess/inprocessapplication.cpp index 812b4c2342..d8566262e1 100644 --- a/src/RequestHandler/inprocess/inprocessapplication.cpp +++ b/src/RequestHandler/inprocess/inprocessapplication.cpp @@ -6,6 +6,7 @@ IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( IHttpServer* pHttpServer, ASPNETCORE_CONFIG* pConfig) : APPLICATION(pHttpServer, pConfig), + m_pHttpServer(pHttpServer), m_ProcessExitCode(0), m_hLogFileHandle(INVALID_HANDLE_VALUE), m_hErrReadPipe(INVALID_HANDLE_VALUE), @@ -15,7 +16,8 @@ IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( m_fBlockCallbacksIntoManaged(FALSE), m_fInitialized(FALSE), m_fShutdownCalledFromNative(FALSE), - m_fShutdownCalledFromManaged(FALSE) + m_fShutdownCalledFromManaged(FALSE), + m_srwLock() { // is it guaranteed that we have already checked app offline at this point? // If so, I don't think there is much to do here. @@ -57,8 +59,8 @@ IN_PROCESS_APPLICATION::ShutDown( VOID ) { - HANDLE hThread = NULL; - HRESULT hr = S_OK; + HRESULT hr = S_OK; + CHandle hThread; DWORD dwThreadStatus = 0; DWORD dwTimeout = m_pConfig->QueryShutdownTimeLimitInMS(); @@ -67,15 +69,15 @@ IN_PROCESS_APPLICATION::ShutDown( dwTimeout = INFINITE; } - hThread = CreateThread( + hThread.Attach(CreateThread( NULL, // default security attributes 0, // default stack size (LPTHREAD_START_ROUTINE)DoShutDown, this, // thread function arguments 0, // default creation flags - NULL); // receive thread identifier + NULL)); // receive thread identifier - if (hThread == NULL) + if ((HANDLE)hThread == NULL) { hr = HRESULT_FROM_WIN32(GetLastError()); goto Finished; @@ -95,12 +97,6 @@ IN_PROCESS_APPLICATION::ShutDown( Finished: - if (hThread != NULL) - { - CloseHandle(hThread); - } - m_hThread = NULL; - if (FAILED(hr)) { UTILITY::LogEventF(g_hEventLog, @@ -126,7 +122,6 @@ IN_PROCESS_APPLICATION::ShutDownInternal() DWORD dwTimeout = m_pConfig->QueryShutdownTimeLimitInMS(); HANDLE handle = NULL; WIN32_FIND_DATA fileData; - BOOL fLocked = FALSE; if (IsDebuggerPresent()) { @@ -137,41 +132,43 @@ IN_PROCESS_APPLICATION::ShutDownInternal() m_status == APPLICATION_STATUS::STARTING || m_status == APPLICATION_STATUS::FAIL) { - goto Finished; + return; } - AcquireSRWLockExclusive(&m_srwLock); - fLocked = TRUE; - - if (m_fShutdownCalledFromNative || - m_status == APPLICATION_STATUS::STARTING || - m_status == APPLICATION_STATUS::FAIL) { - goto Finished; - } + SRWLockWrapper lock(m_srwLock); - // We need to keep track of when both managed and native initiate shutdown - // to avoid AVs. If shutdown has already been initiated in managed, we don't want to call into - // managed. We still need to wait on main exiting no matter what. m_fShutdownCalledFromNative - // is used for detecting redundant calls and blocking more requests to OnExecuteRequestHandler. - m_fShutdownCalledFromNative = TRUE; - m_status = APPLICATION_STATUS::SHUTDOWN; + if (m_fShutdownCalledFromNative || + m_status == APPLICATION_STATUS::STARTING || + m_status == APPLICATION_STATUS::FAIL) + { + return; + } + + // We need to keep track of when both managed and native initiate shutdown + // to avoid AVs. If shutdown has already been initiated in managed, we don't want to call into + // managed. We still need to wait on main exiting no matter what. m_fShutdownCalledFromNative + // is used for detecting redundant calls and blocking more requests to OnExecuteRequestHandler. + m_fShutdownCalledFromNative = TRUE; + m_status = APPLICATION_STATUS::SHUTDOWN; + + if (!m_fShutdownCalledFromManaged) + { + // We cannot call into managed if the dll is detaching from the process. + // Calling into managed code when the dll is detaching is strictly a bad idea, + // and usually results in an AV saying "The string binding is invalid" + if (!g_fProcessDetach) + { + m_ShutdownHandler(m_ShutdownHandlerContext); + m_ShutdownHandler = NULL; + } + } + + // Release the lock before we wait on the thread to exit. + } if (!m_fShutdownCalledFromManaged) { - // We cannot call into managed if the dll is detaching from the process. - // Calling into managed code when the dll is detaching is strictly a bad idea, - // and usually results in an AV saying "The string binding is invalid" - if (!g_fProcessDetach) - { - m_ShutdownHandler(m_ShutdownHandlerContext); - m_ShutdownHandler = NULL; - } - - ReleaseSRWLockExclusive(&m_srwLock); - fLocked = FALSE; - - // Release the lock before we wait on the thread to exit. if (m_hThread != NULL && GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && dwThreadStatus == STILL_ACTIVE) @@ -221,36 +218,32 @@ IN_PROCESS_APPLICATION::ShutDownInternal() // as nothing can be done DeleteFile(m_struLogFilePath.QueryStr()); } - -Finished: - - if (fLocked) - { - ReleaseSRWLockExclusive(&m_srwLock); - } - return; } +__override VOID IN_PROCESS_APPLICATION::Recycle( VOID ) { - BOOL fLockAcquired = FALSE; // We need to guarantee that recycle is only called once, as calling pHttpServer->RecycleProcess // multiple times can lead to AVs. if (m_fRecycleCalled) { - goto Finished; + return; } - AcquireSRWLockExclusive(&m_srwLock); - fLockAcquired = TRUE; - - if (m_fRecycleCalled) { - goto Finished; + SRWLockWrapper lock(m_srwLock); + + if (m_fRecycleCalled) + { + return; + } + + m_fRecycleCalled = true; } + if (!m_pHttpServer->IsCommandLineLaunch()) { // IIS scenario. @@ -260,20 +253,10 @@ IN_PROCESS_APPLICATION::Recycle( else { // IISExpress scenario - // Release the lock first as Shutdown will acquire lock later - ReleaseSRWLockExclusive(&m_srwLock); - fLockAcquired = FALSE; - // Shutdown the managed application and call exit to terminate current process ShutDown(); exit(0); } - -Finished: - if (fLockAcquired) - { - ReleaseSRWLockExclusive(&m_srwLock); - } } REQUEST_NOTIFICATION_STATUS @@ -392,7 +375,6 @@ IN_PROCESS_APPLICATION::SetStdOut( ) { HRESULT hr = S_OK; - BOOL fLocked = FALSE; STRU struPath; SYSTEMTIME systemTime; SECURITY_ATTRIBUTES saAttr = { 0 }; @@ -402,8 +384,7 @@ IN_PROCESS_APPLICATION::SetStdOut( if (!m_fDoneStdRedirect) { // Have not set stdout yet, redirect stdout to log file - AcquireSRWLockExclusive(&m_srwLock); - fLocked = TRUE; + SRWLockWrapper lock(m_srwLock); if (!m_fDoneStdRedirect) { saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); @@ -553,10 +534,6 @@ IN_PROCESS_APPLICATION::SetStdOut( Finished: m_fDoneStdRedirect = TRUE; - if (fLocked) - { - ReleaseSRWLockExclusive(&m_srwLock); - } if (FAILED(hr) && m_pConfig->QueryStdoutLogEnabled()) { UTILITY::LogEventF(g_hEventLog, @@ -651,7 +628,6 @@ IN_PROCESS_APPLICATION::LoadManagedApplication HRESULT hr = S_OK; DWORD dwTimeout; DWORD dwResult; - BOOL fLocked = FALSE; ReferenceApplication(); @@ -674,84 +650,84 @@ IN_PROCESS_APPLICATION::LoadManagedApplication // Set up stdout redirect SetStdOut(); - AcquireSRWLockExclusive(&m_srwLock); - fLocked = TRUE; - if (m_status != APPLICATION_STATUS::STARTING) { - if (m_status == APPLICATION_STATUS::FAIL) + SRWLockWrapper lock(m_srwLock); + if (m_status != APPLICATION_STATUS::STARTING) + { + if (m_status == APPLICATION_STATUS::FAIL) + { + hr = E_APPLICATION_ACTIVATION_EXEC_FAILURE; + } + else if (m_status == APPLICATION_STATUS::SHUTDOWN) + { + hr = HRESULT_FROM_WIN32(ERROR_SHUTDOWN_IS_SCHEDULED); + } + + goto Finished; + } + m_hThread = CreateThread( + NULL, // default security attributes + 0, // default stack size + (LPTHREAD_START_ROUTINE)ExecuteAspNetCoreProcess, + this, // thread function arguments + 0, // default creation flags + NULL); // receive thread identifier + + if (m_hThread == NULL) + { + hr = HRESULT_FROM_WIN32(GetLastError()); + goto Finished; + } + + m_pInitalizeEvent = CreateEvent( + NULL, // default security attributes + TRUE, // manual reset event + FALSE, // not set + NULL); // name + + if (m_pInitalizeEvent == NULL) + { + hr = HRESULT_FROM_WIN32(GetLastError()); + } + + // If the debugger is attached, never timeout + if (IsDebuggerPresent()) + { + dwTimeout = INFINITE; + } + else + { + dwTimeout = m_pConfig->QueryStartupTimeLimitInMS(); + } + + const HANDLE pHandles[2]{ m_hThread, m_pInitalizeEvent }; + + // Wait on either the thread to complete or the event to be set + dwResult = WaitForMultipleObjects(2, pHandles, FALSE, dwTimeout); + + // It all timed out + if (dwResult == WAIT_TIMEOUT) + { + // kill the backend thread as loading dotnet timedout + TerminateThread(m_hThread, 0); + hr = HRESULT_FROM_WIN32(dwResult); + goto Finished; + } + else if (dwResult == WAIT_FAILED) + { + hr = HRESULT_FROM_WIN32(GetLastError()); + goto Finished; + } + + // The thread ended it means that something failed + if (dwResult == WAIT_OBJECT_0) { hr = E_APPLICATION_ACTIVATION_EXEC_FAILURE; - } - else if (m_status == APPLICATION_STATUS::SHUTDOWN) - { - hr = HRESULT_FROM_WIN32(ERROR_SHUTDOWN_IS_SCHEDULED); + goto Finished; } - goto Finished; + m_status = APPLICATION_STATUS::RUNNING; } - m_hThread = CreateThread( - NULL, // default security attributes - 0, // default stack size - (LPTHREAD_START_ROUTINE)ExecuteAspNetCoreProcess, - this, // thread function arguments - 0, // default creation flags - NULL); // receive thread identifier - - if (m_hThread == NULL) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - goto Finished; - } - - m_pInitalizeEvent = CreateEvent( - NULL, // default security attributes - TRUE, // manual reset event - FALSE, // not set - NULL); // name - - if (m_pInitalizeEvent == NULL) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - } - - // If the debugger is attached, never timeout - if (IsDebuggerPresent()) - { - dwTimeout = INFINITE; - } - else - { - dwTimeout = m_pConfig->QueryStartupTimeLimitInMS(); - } - - const HANDLE pHandles[2]{ m_hThread, m_pInitalizeEvent }; - - // Wait on either the thread to complete or the event to be set - dwResult = WaitForMultipleObjects(2, pHandles, FALSE, dwTimeout); - - // It all timed out - if (dwResult == WAIT_TIMEOUT) - { - // kill the backend thread as loading dotnet timedout - TerminateThread(m_hThread, 0); - hr = HRESULT_FROM_WIN32(dwResult); - goto Finished; - } - else if (dwResult == WAIT_FAILED) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - goto Finished; - } - - // The thread ended it means that something failed - if (dwResult == WAIT_OBJECT_0) - { - hr = E_APPLICATION_ACTIVATION_EXEC_FAILURE; - goto Finished; - } - - m_status = APPLICATION_STATUS::RUNNING; - Finished: if (FAILED(hr)) @@ -766,12 +742,6 @@ Finished: m_pConfig->QueryApplicationPhysicalPath()->QueryStr(), hr); } - - if (fLocked) - { - ReleaseSRWLockExclusive(&m_srwLock); - } - DereferenceApplication(); return hr; diff --git a/src/RequestHandler/inprocess/inprocessapplication.h b/src/RequestHandler/inprocess/inprocessapplication.h index 9de0b5255f..a97fa0a3a9 100644 --- a/src/RequestHandler/inprocess/inprocessapplication.h +++ b/src/RequestHandler/inprocess/inprocessapplication.h @@ -110,6 +110,8 @@ private: VOID ); + IHttpServer* const m_pHttpServer; + // Thread executing the .NET Core process HANDLE m_hThread;