diff --git a/src/AspNetCore/Inc/applicationinfo.h b/src/AspNetCore/Inc/applicationinfo.h index 7be36427f2..f47b51786b 100644 --- a/src/AspNetCore/Inc/applicationinfo.h +++ b/src/AspNetCore/Inc/applicationinfo.h @@ -4,6 +4,8 @@ #pragma once #define API_BUFFER_TOO_SMALL 0x80008098 +extern BOOL g_fRecycleProcessCalled; + typedef HRESULT (WINAPI * PFN_ASPNETCORE_CREATE_APPLICATION)( @@ -128,19 +130,28 @@ public: return m_pConfiguration; } - APPLICATION* - QueryApplication() + // + // ExtractApplication will increase the reference counter of the application + // Caller is responsible for dereference the application. + // Otherwise memory leak + // + VOID + ExtractApplication(APPLICATION** ppApplication) { - return m_pApplication; + AcquireSRWLockShared(&m_srwLock); + if (m_pApplication != NULL) + { + m_pApplication->ReferenceApplication(); + } + *ppApplication = m_pApplication; + ReleaseSRWLockShared(&m_srwLock); } VOID - ClearAndDereferenceApplication() - { - APPLICATION* pApplication = m_pApplication; - m_pApplication = NULL; - pApplication->DereferenceApplication(); - } + RecycleApplication(); + + VOID + ShutDownApplication(); HRESULT EnsureApplicationCreated(); @@ -156,6 +167,8 @@ private: HRESULT FindNativeAssemblyFromGlobalLocation(STRU* struFilename); HRESULT FindNativeAssemblyFromHostfxr(STRU* struFilename); + static VOID DoRecycleApplication(LPVOID lpParam); + mutable LONG m_cRefs; APPLICATION_INFO_KEY m_applicationInfoKey; BOOL m_fAppOfflineFound; diff --git a/src/AspNetCore/Inc/applicationmanager.h b/src/AspNetCore/Inc/applicationmanager.h index cf5a11d31c..55c00a56e9 100644 --- a/src/AspNetCore/Inc/applicationmanager.h +++ b/src/AspNetCore/Inc/applicationmanager.h @@ -55,18 +55,6 @@ public: _In_ PVOID pvContext ); - static - VOID - RecycleApplication( - _In_ APPLICATION_INFO * pEntry - ); - - static - void - DoRecycleApplication( - LPVOID lpParam - ); - static VOID ShutdownApplication( diff --git a/src/AspNetCore/Src/applicationinfo.cpp b/src/AspNetCore/Src/applicationinfo.cpp index 71f511ad15..18bd99d2d5 100644 --- a/src/AspNetCore/Src/applicationinfo.cpp +++ b/src/AspNetCore/Src/applicationinfo.cpp @@ -102,7 +102,8 @@ APPLICATION_INFO::UpdateAppOfflineFileHandle() &strFilePath); APP_OFFLINE_HTM *pOldAppOfflineHtm = NULL; APP_OFFLINE_HTM *pNewAppOfflineHtm = NULL; - BOOL fLocked = FALSE; + + ReferenceApplicationInfo(); if (INVALID_FILE_ATTRIBUTES == GetFileAttributes(strFilePath.QueryStr()) && GetLastError() == ERROR_FILE_NOT_FOUND) @@ -156,13 +157,6 @@ APPLICATION_INFO::UpdateAppOfflineFileHandle() // recycle the application if (m_pApplication != NULL) { - // Lock here to avoid races with the application manager calling shutdown on the application. - AcquireSRWLockExclusive(&m_srwLock); - fLocked = TRUE; - if (m_pApplication == NULL) - { - goto Finished; - } STACK_STRU(strEventMsg, 256); if (SUCCEEDED(strEventMsg.SafeSnwprintf( ASPNETCORE_EVENT_RECYCLE_APPOFFLINE_MSG, @@ -174,16 +168,11 @@ APPLICATION_INFO::UpdateAppOfflineFileHandle() strEventMsg.QueryStr()); } - APPLICATION_MANAGER::RecycleApplication(this); - + RecycleApplication(); } } -Finished: - if (fLocked) - { - ReleaseSRWLockExclusive(&m_srwLock); - } + DereferenceApplicationInfo(); } HRESULT @@ -239,19 +228,26 @@ APPLICATION_INFO::EnsureApplicationCreated() goto Finished; } - if (m_pfnAspNetCoreCreateApplication == NULL) + // + // in case of app offline, we don't want to create a new application now + // + if (!m_fAppOfflineFound) { - hr = HRESULT_FROM_WIN32(ERROR_INVALID_FUNCTION); - goto Finished; - } + if (m_pfnAspNetCoreCreateApplication == NULL) + { + hr = HRESULT_FROM_WIN32(ERROR_INVALID_FUNCTION); + goto Finished; + } - hr = m_pfnAspNetCoreCreateApplication(m_pServer, m_pConfiguration, &pApplication); - if (FAILED(hr)) - { - goto Finished; + hr = m_pfnAspNetCoreCreateApplication(m_pServer, m_pConfiguration, &pApplication); + if (FAILED(hr)) + { + goto Finished; + } + m_pApplication = pApplication; } - m_pApplication = pApplication; } + Finished: if (fLocked) { @@ -533,3 +529,110 @@ Finished: } return hr; } + +VOID +APPLICATION_INFO::RecycleApplication() +{ + APPLICATION* pApplication = NULL; + HANDLE hThread = INVALID_HANDLE_VALUE; + BOOL fLockAcquired = FALSE; + + if (m_pApplication != NULL) + { + AcquireSRWLockExclusive(&m_srwLock); + fLockAcquired = TRUE; + if (m_pApplication != NULL) + { + pApplication = m_pApplication; + if (pApplication->QueryConfig()->QueryHostingModel() == HOSTING_OUT_PROCESS) + { + // + // For inprocess, need to set m_pApplication to NULL first to + // avoid mapping new request to the recycled application. + // Outofprocess application instance will be created for new request + // For inprocess, as recycle will lead to shutdown later, leave m_pApplication + // to not block incoming requests till worker process shutdown + // + m_pApplication = NULL; + } + else + { + // + // For inprocess, need hold the application till shutdown is called + // Bump the reference counter as DoRecycleApplication will do dereference + // + pApplication->ReferenceApplication(); + } + + 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 + } + + if (hThread == NULL) + { + if (!g_fRecycleProcessCalled) + { + g_fRecycleProcessCalled = TRUE; + g_pHttpServer->RecycleProcess(L"On Demand by AspNetCore Module for recycle application failure"); + } + } + + if (fLockAcquired) + { + ReleaseSRWLockExclusive(&m_srwLock); + } + } +} + + +VOID +APPLICATION_INFO::DoRecycleApplication( + LPVOID lpParam) +{ + APPLICATION* pApplication = static_cast(lpParam); + + // No lock required + + if (pApplication != NULL) + { + // Recycle will call shutdown for out of process + pApplication->Recycle(); + + // Decrement the ref count as we reference it in RecycleApplication. + pApplication->DereferenceApplication(); + } +} + + +VOID +APPLICATION_INFO::ShutDownApplication() +{ + APPLICATION* pApplication = NULL; + BOOL fLockAcquired = FALSE; + + // pApplication can be NULL due to app_offline + if (m_pApplication != NULL) + { + AcquireSRWLockExclusive(&m_srwLock); + fLockAcquired = TRUE; + if (m_pApplication != NULL) + { + pApplication = m_pApplication; + + // Set m_pApplication to NULL first to prevent anyone from using it + m_pApplication = NULL; + pApplication->ShutDown(); + pApplication->DereferenceApplication(); + } + + if (fLockAcquired) + { + ReleaseSRWLockExclusive(&m_srwLock); + } + } +} diff --git a/src/AspNetCore/Src/applicationmanager.cxx b/src/AspNetCore/Src/applicationmanager.cxx index beee7456ed..fd83a19818 100644 --- a/src/AspNetCore/Src/applicationmanager.cxx +++ b/src/AspNetCore/Src/applicationmanager.cxx @@ -370,7 +370,7 @@ APPLICATION_MANAGER::RecycleApplicationFromManager( DBG_ASSERT(pRecord != NULL); // RecycleApplication is called on a separate thread. - RecycleApplication(pRecord); + pRecord->RecycleApplication(); pRecord->DereferenceApplicationInfo(); path = context.MultiSz.Next(path); } @@ -397,7 +397,11 @@ Finished: strEventMsg.QueryStr()); } // Need to recycle the process as we cannot recycle the application - g_pHttpServer->RecycleProcess(L"AspNetCore Recycle Process on Demand Due Application Recycle Error"); + if (!g_fRecycleProcessCalled) + { + g_fRecycleProcessCalled = TRUE; + g_pHttpServer->RecycleProcess(L"AspNetCore Recycle Process on Demand Due Application Recycle Error"); + } } return hr; @@ -448,64 +452,7 @@ APPLICATION_MANAGER::ShutdownApplication( ) { UNREFERENCED_PARAMETER(pvContext); - APPLICATION* pApplication = pEntry->QueryApplication(); - pApplication->ReferenceApplication(); + DBG_ASSERT(pEntry != NULL); - // Remove the application from the applicationInfo. - pEntry->ClearAndDereferenceApplication(); - pApplication->ShutDown(); - pApplication->DereferenceApplication(); -} - -// -// Function used by DoRecycleApplication thread to do the real shutdown -// -// static -VOID -APPLICATION_MANAGER::DoRecycleApplication( - LPVOID lpParam) -{ - APPLICATION* pApplication = static_cast(lpParam); - - // Recycle will call shutdown for out of process - pApplication->Recycle(); - - // Decrement the ref count as we reference it in RecycleApplication. - pApplication->DereferenceApplication(); -} - -// -// Function used to recycle an application -// -// static -VOID -APPLICATION_MANAGER::RecycleApplication( - _In_ APPLICATION_INFO * pEntry -) -{ - APPLICATION* pApplication = pEntry->QueryApplication(); - DBG_ASSERT(pApplication != NULL); - - // Reference the application first - pApplication->ReferenceApplication(); - - if (pApplication->QueryConfig()->QueryHostingModel() == HOSTING_OUT_PROCESS) - { - // Need to set m_pApplication to NULL first - // to avoid mapping new request to the recycled application - // A new application instance will be created for new request - pEntry->ClearAndDereferenceApplication(); - } - - // Reset application pointer to NULL - // The destructor of ApplictionInfo will not call ShutDown again - HANDLE 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 - - CloseHandle(hThread); + pEntry->ShutDownApplication(); } diff --git a/src/AspNetCore/Src/proxymodule.cxx b/src/AspNetCore/Src/proxymodule.cxx index b2402d789c..7b827da4be 100644 --- a/src/AspNetCore/Src/proxymodule.cxx +++ b/src/AspNetCore/Src/proxymodule.cxx @@ -138,18 +138,18 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( } // make sure assmebly is loaded and application is created - hr = m_pApplicationInfo->EnsureApplicationCreated(); if (FAILED(hr)) { goto Finished; } - pApplication = m_pApplicationInfo->QueryApplication(); - DBG_ASSERT(pApplication); + + m_pApplicationInfo->ExtractApplication(&pApplication); // make sure application is in running state // cannot recreate the application as we cannot reload clr for inprocess - if (pApplication->QueryStatus() != APPLICATION_STATUS::RUNNING && + if (pApplication != NULL && + pApplication->QueryStatus() != APPLICATION_STATUS::RUNNING && pApplication->QueryStatus() != APPLICATION_STATUS::STARTING) { hr = HRESULT_FROM_WIN32(ERROR_SERVER_DISABLED); @@ -166,6 +166,7 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( { goto Finished; } + retVal = m_pHandler->OnExecuteRequestHandler(); Finished: @@ -175,6 +176,10 @@ Finished: retVal = RQ_NOTIFICATION_FINISH_REQUEST; } + if (pApplication != NULL) + { + pApplication->DereferenceApplication(); + } return retVal; }