From 448a2afed86609e98588bd056092b58a94a5c373 Mon Sep 17 00:00:00 2001 From: pan-wang Date: Sat, 21 Oct 2017 13:40:45 -0700 Subject: [PATCH] fix the AV in recycle process, issue #192. (#201) * fix the AV in recycle process. this is due to we call Recycle again when the background thread exists * more fixes * reset hosting mode when all applications got removed --- build/Version.props | 2 +- src/AspNetCore/Inc/inprocessapplication.h | 1 + src/AspNetCore/Inc/resource.h | 2 +- src/AspNetCore/Src/applicationmanager.cxx | 4 + src/AspNetCore/Src/aspnetcoreconfig.cxx | 17 ++++- src/AspNetCore/Src/forwardinghandler.cxx | 3 +- src/AspNetCore/Src/inprocessapplication.cxx | 83 ++++++++++++--------- 7 files changed, 67 insertions(+), 45 deletions(-) diff --git a/build/Version.props b/build/Version.props index 27a5f2db48..f5ae3f4b01 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ 7 1 - 1968 + 1987 -RTM diff --git a/src/AspNetCore/Inc/inprocessapplication.h b/src/AspNetCore/Inc/inprocessapplication.h index c80ac757fb..88aeb1c2fd 100644 --- a/src/AspNetCore/Inc/inprocessapplication.h +++ b/src/AspNetCore/Inc/inprocessapplication.h @@ -95,6 +95,7 @@ private: BOOL m_fManagedAppLoaded; BOOL m_fLoadManagedAppError; + BOOL m_fInitialized; BOOL m_fIsWebSocketsConnection; static IN_PROCESS_APPLICATION* s_Application; diff --git a/src/AspNetCore/Inc/resource.h b/src/AspNetCore/Inc/resource.h index 5f98458d85..be91685c10 100644 --- a/src/AspNetCore/Inc/resource.h +++ b/src/AspNetCore/Inc/resource.h @@ -11,7 +11,7 @@ #define ASPNETCORE_EVENT_RAPID_FAIL_COUNT_EXCEEDED_MSG L"Maximum rapid fail count per minute of '%d' exceeded." #define ASPNETCORE_EVENT_PROCESS_START_INTERNAL_ERROR_MSG L"Application '%s' failed to parse processPath and arguments due to internal error, ErrorCode = '0x%x'." #define ASPNETCORE_EVENT_PROCESS_START_POSTCREATE_ERROR_MSG L"Application '%s' with physical root '%s' created process with commandline '%s'but failed to get its status, ErrorCode = '0x%x'." -#define ASPNETCORE_EVENT_PROCESS_START_ERROR_MSG L"Application '%s' with physical root '%s' failed to start process with commandline '%s', ErrorCode = '0x%x : %x." +#define ASPNETCORE_EVENT_PROCESS_START_ERROR_MSG L"Application '%s' with physical root '%s' failed to start process with commandline '%s', ErrorCode = '0x%x' processStatus code '%x'." #define ASPNETCORE_EVENT_PROCESS_START_WRONGPORT_ERROR_MSG L"Application '%s' with physical root '%s' created process with commandline '%s' but failed to listen on the given port '%d'" #define ASPNETCORE_EVENT_PROCESS_START_NOTREADY_ERROR_MSG L"Application '%s' with physical root '%s' created process with commandline '%s' but either crashed or did not reponse or did not listen on the given port '%d', ErrorCode = '0x%x'" #define ASPNETCORE_EVENT_INVALID_STDOUT_LOG_FILE_MSG L"Warning: Could not create stdoutLogFile %s, ErrorCode = %d." diff --git a/src/AspNetCore/Src/applicationmanager.cxx b/src/AspNetCore/Src/applicationmanager.cxx index 6c2ce6d235..f2640a55a5 100644 --- a/src/AspNetCore/Src/applicationmanager.cxx +++ b/src/AspNetCore/Src/applicationmanager.cxx @@ -221,6 +221,10 @@ APPLICATION_MANAGER::RecycleApplication( } AcquireSRWLockExclusive(&m_srwLock); m_pApplicationHash->DeleteKey(&key); + if (m_pApplicationHash->Count() == 0) + { + m_hostingModel = HOSTING_UNKNOWN; + } ReleaseSRWLockExclusive(&m_srwLock); Finished: diff --git a/src/AspNetCore/Src/aspnetcoreconfig.cxx b/src/AspNetCore/Src/aspnetcoreconfig.cxx index a63c3bf1ae..090c4e3f23 100644 --- a/src/AspNetCore/Src/aspnetcoreconfig.cxx +++ b/src/AspNetCore/Src/aspnetcoreconfig.cxx @@ -5,17 +5,26 @@ ASPNETCORE_CONFIG::~ASPNETCORE_CONFIG() { - if (QueryHostingModel() == HOSTING_IN_PROCESS && - !g_fRecycleProcessCalled && - !g_pHttpServer->IsCommandLineLaunch()) + if (QueryHostingModel() == HOSTING_IN_PROCESS && + !g_fRecycleProcessCalled && + (g_pHttpServer->GetAdminManager() != NULL)) { + // There is a bug in IHttpServer::RecycleProcess. It will hit AV when worker process + // has already been in recycling state. + // To workaround, do null check on GetAdminManager(). If it is NULL, worker process is in recycling + // Do not call RecycleProcess again + // RecycleProcess can olny be called once // In case of configuration change for in-process app // We want notify IIS first to let new request routed to new worker process - g_fRecycleProcessCalled = TRUE; + g_pHttpServer->RecycleProcess(L"AspNetCore Recycle Process on Configuration Change"); } + // It's safe for us to set this g_fRecycleProcessCalled + // as in_process scenario will always recycle the worker process for configuration change + g_fRecycleProcessCalled = TRUE; + m_struApplicationFullPath.Reset(); if (m_pEnvironmentVariables != NULL) { diff --git a/src/AspNetCore/Src/forwardinghandler.cxx b/src/AspNetCore/Src/forwardinghandler.cxx index 13ebc415b3..2ddc077f88 100644 --- a/src/AspNetCore/Src/forwardinghandler.cxx +++ b/src/AspNetCore/Src/forwardinghandler.cxx @@ -1043,7 +1043,6 @@ FORWARDING_HANDLER::OnExecuteRequestHandler( IHttpRequest *pRequest = m_pW3Context->GetRequest(); IHttpResponse *pResponse = m_pW3Context->GetResponse(); PROTOCOL_CONFIG *pProtocol = &sm_ProtocolConfig; - APPLICATION_MANAGER *pApplicationManager = NULL; SERVER_PROCESS *pServerProcess = NULL; USHORT cchHostName = 0; BOOL fSecure = FALSE; @@ -1427,7 +1426,7 @@ Failure: } else { - if (SUCCEEDED(pApplicationManager->Get502ErrorPage(&pDataChunk))) + if (SUCCEEDED(APPLICATION_MANAGER::GetInstance()->Get502ErrorPage(&pDataChunk))) { if (FAILED(hr = pResponse->WriteEntityChunkByReference(pDataChunk))) { diff --git a/src/AspNetCore/Src/inprocessapplication.cxx b/src/AspNetCore/Src/inprocessapplication.cxx index d65c2a23f7..f52118a95a 100644 --- a/src/AspNetCore/Src/inprocessapplication.cxx +++ b/src/AspNetCore/Src/inprocessapplication.cxx @@ -335,7 +335,8 @@ IN_PROCESS_APPLICATION* IN_PROCESS_APPLICATION::s_Application = NULL; IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION() : m_ProcessExitCode ( 0 ), m_fManagedAppLoaded ( FALSE ), - m_fLoadManagedAppError ( FALSE ) + m_fLoadManagedAppError ( FALSE ), + m_fInitialized ( FALSE ) { } @@ -510,6 +511,7 @@ IN_PROCESS_APPLICATION::Initialize( hr = HRESULT_FROM_WIN32(GetLastError()); goto Finished; } + m_fInitialized = TRUE; Finished: return hr; @@ -636,50 +638,57 @@ IN_PROCESS_APPLICATION::Recycle( VOID ) { - DWORD dwThreadStatus = 0; - DWORD dwTimeout = m_pConfiguration->QueryShutdownTimeLimitInMS(); - - AcquireSRWLockExclusive(&m_srwLock); - - if (!g_pHttpServer->IsCommandLineLaunch() && !g_fRecycleProcessCalled) + if (m_fInitialized) { - // IIS scenario. - // notify IIS first so that new request will be routed to new worker process - g_fRecycleProcessCalled = TRUE; - g_pHttpServer->RecycleProcess(L"AspNetCore Recycle Process on Demand"); - } - // First call into the managed server and shutdown - if (m_ShutdownHandler != NULL) - { - m_ShutdownHandler(m_ShutdownHandlerContext); - m_ShutdownHandler = NULL; - } + DWORD dwThreadStatus = 0; + DWORD dwTimeout = m_pConfiguration->QueryShutdownTimeLimitInMS(); - if (m_hThread != NULL && - GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && - dwThreadStatus == STILL_ACTIVE) - { - // wait for gracefullshut down, i.e., the exit of the background thread or timeout - if (WaitForSingleObject(m_hThread, dwTimeout) != WAIT_OBJECT_0) + AcquireSRWLockExclusive(&m_srwLock); + + if (!g_pHttpServer->IsCommandLineLaunch() && + !g_fRecycleProcessCalled && + (g_pHttpServer->GetAdminManager() != NULL)) { - // if the thread is still running, we need kill it first before exit to avoid AV - if (GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && dwThreadStatus == STILL_ACTIVE) + // IIS scenario. + // notify IIS first so that new request will be routed to new worker process + g_pHttpServer->RecycleProcess(L"AspNetCore Recycle Process on Demand"); + } + + g_fRecycleProcessCalled = TRUE; + + // First call into the managed server and shutdown + if (m_ShutdownHandler != NULL) + { + m_ShutdownHandler(m_ShutdownHandlerContext); + m_ShutdownHandler = NULL; + } + + if (m_hThread != NULL && + GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && + dwThreadStatus == STILL_ACTIVE) + { + // wait for gracefullshut down, i.e., the exit of the background thread or timeout + if (WaitForSingleObject(m_hThread, dwTimeout) != WAIT_OBJECT_0) { - TerminateThread(m_hThread, STATUS_CONTROL_C_EXIT); + // if the thread is still running, we need kill it first before exit to avoid AV + if (GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && dwThreadStatus == STILL_ACTIVE) + { + TerminateThread(m_hThread, STATUS_CONTROL_C_EXIT); + } } } - } - CloseHandle(m_hThread); - m_hThread = NULL; - s_Application = NULL; - ReleaseSRWLockExclusive(&m_srwLock); + CloseHandle(m_hThread); + m_hThread = NULL; + s_Application = NULL; - if (g_pHttpServer->IsCommandLineLaunch()) - { - // IISExpress scenario - // Can only call exit to terminate current process - exit(0); + ReleaseSRWLockExclusive(&m_srwLock); + if (g_pHttpServer && g_pHttpServer->IsCommandLineLaunch()) + { + // IISExpress scenario + // Can only call exit to terminate current process + exit(0); + } } }