Skip to content

Improve ANCM file watcher thread management #49696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 3, 2023
Merged

Improve ANCM file watcher thread management #49696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8ec5807
File watcher thread management improvements.
adityamandaleeka Jul 28, 2023
7d1cf68
wip
adityamandaleeka Jul 28, 2023
c7f0f65
Add logging for when WaitForMonitor doesn't see thread exit.
adityamandaleeka Jul 28, 2023
2dfd1c1
Add env var to fall back to termination.
adityamandaleeka Jul 29, 2023
384559f
Wait for watcher thread indefinitely if not using rude termination.
adityamandaleeka Aug 1, 2023
4dd8091
Wish our local build wasn't broken.
adityamandaleeka Aug 1, 2023
6382c2b
Add log message while waiting.
adityamandaleeka Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 99 additions & 71 deletions src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,56 +9,80 @@
#include <EventLog.h>

FILE_WATCHER::FILE_WATCHER() :
m_hCompletionPort(NULL),
m_hChangeNotificationThread(NULL),
m_fThreadExit(FALSE),
m_fShadowCopyEnabled(FALSE),
m_hCompletionPort(nullptr),
m_hChangeNotificationThread(nullptr),
m_fThreadExit(false),
m_fShadowCopyEnabled(false),
m_copied(false)
{
m_pDoneCopyEvent = CreateEvent(
nullptr, // default security attributes
TRUE, // manual reset event
FALSE, // not set
nullptr); // name

m_pShutdownEvent = CreateEvent(
nullptr, // default security attributes
TRUE, // manual reset event
FALSE, // not set
nullptr); // name

// Use of TerminateThread for the file watcher thread was eliminated in favor of an event-based
// approach. Out of an abundance of caution, we are temporarily adding an environment variable
// to allow falling back to TerminateThread usage. If all goes well, this will be removed in a
// future release.
m_fRudeThreadTermination = false;
auto enableThreadTerminationValue = Environment::GetEnvironmentVariableValue(L"ASPNETCORE_FILE_WATCHER_THREAD_TERMINATION");
if (enableThreadTerminationValue.has_value())
{
m_fRudeThreadTermination = (enableThreadTerminationValue.value() == L"1");
}
}

FILE_WATCHER::~FILE_WATCHER()
{
StopMonitor();
WaitForMonitor(20); // wait for 1 second total
WaitForWatcherThreadExit();
}

void FILE_WATCHER::WaitForMonitor(DWORD dwRetryCounter)
void FILE_WATCHER::WaitForWatcherThreadExit()
{
if (m_hChangeNotificationThread != NULL)
if (m_hChangeNotificationThread == nullptr)
{
DWORD dwExitCode = STILL_ACTIVE;
return;
}

while (!m_fThreadExit && dwRetryCounter > 0)
if (m_fRudeThreadTermination)
{
// This is the old behavior, which is now opt-in using an environment variable. Wait for
// the thread to exit, but if it doesn't exit soon enough, terminate it.
const int totalWaitTimeMs = 10000;
const int waitIntervalMs = 50;
const int iterations = totalWaitTimeMs / waitIntervalMs;
for (int i = 0; i < iterations && !m_fThreadExit; i++)
{
if (GetExitCodeThread(m_hChangeNotificationThread, &dwExitCode))
{
if (dwExitCode == STILL_ACTIVE)
{
// the file watcher thread will set m_fThreadExit before exit
WaitForSingleObject(m_hChangeNotificationThread, 50);
}
}
else
// Check if the thread has exited.
DWORD result = WaitForSingleObject(m_hChangeNotificationThread, waitIntervalMs);
if (result == WAIT_OBJECT_0)
{
// fail to get thread status
// call terminitethread
TerminateThread(m_hChangeNotificationThread, 1);
m_fThreadExit = TRUE;
// The thread has exited.
m_fThreadExit = true;
break;
}
dwRetryCounter--;
}

if (!m_fThreadExit)
{
LOG_INFO(L"File watcher thread did not exit. Forcing termination.");
TerminateThread(m_hChangeNotificationThread, 1);
}
}
else
{
// Wait for the thread to exit.
LOG_INFO(L"Waiting for file watcher thread to exit.");
WaitForSingleObject(m_hChangeNotificationThread, INFINITE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WaitForSingleObject(m_hChangeNotificationThread, INFINITE);
LOG_INFO(L"Waiting for File watcher thread to exit.");
WaitForSingleObject(m_hChangeNotificationThread, INFINITE);

So we know if it's hanging. The thread itself logs when it exits.

}
}

HRESULT
Expand All @@ -74,18 +98,18 @@ FILE_WATCHER::Create(
m_fShadowCopyEnabled = !shadowCopyPath.empty();
m_shutdownTimeout = shutdownTimeout;

RETURN_LAST_ERROR_IF_NULL(m_hCompletionPort = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0));
RETURN_LAST_ERROR_IF_NULL(m_hCompletionPort = CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 0));

RETURN_LAST_ERROR_IF_NULL(m_hChangeNotificationThread = CreateThread(NULL,
RETURN_LAST_ERROR_IF_NULL(m_hChangeNotificationThread = CreateThread(nullptr,
0,
(LPTHREAD_START_ROUTINE)ChangeNotificationThread,
this,
0,
NULL));

if (pszDirectoryToMonitor == NULL ||
pszFileNameToMonitor == NULL ||
pApplication == NULL)
if (pszDirectoryToMonitor == nullptr ||
pszFileNameToMonitor == nullptr ||
pApplication == nullptr)
{
DBG_ASSERT(FALSE);
return HRESULT_FROM_WIN32(ERROR_INVALID_PARAMETER);
Expand All @@ -107,7 +131,7 @@ FILE_WATCHER::Create(
_strDirectoryName.QueryStr(),
FILE_LIST_DIRECTORY,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
nullptr,
OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED,
NULL);
Expand Down Expand Up @@ -146,36 +170,41 @@ Win32 error

--*/
{
FILE_WATCHER* pFileMonitor;
BOOL fSuccess = FALSE;
DWORD cbCompletion = 0;
OVERLAPPED* pOverlapped = NULL;
DWORD dwErrorStatus;
ULONG_PTR completionKey;

FILE_WATCHER* pFileMonitor = (FILE_WATCHER*)pvArg;

LOG_INFO(L"Starting file watcher thread");
pFileMonitor = (FILE_WATCHER*)pvArg;
DBG_ASSERT(pFileMonitor != NULL);
DBG_ASSERT(pFileMonitor != nullptr);

HANDLE events[2] = { pFileMonitor->m_hCompletionPort, pFileMonitor->m_pShutdownEvent };

while (TRUE)
DWORD dwEvent = 0;
while (true)
{
fSuccess = GetQueuedCompletionStatus(
// Wait for either a change notification or a shutdown event.
dwEvent = WaitForMultipleObjects(ARRAYSIZE(events), events, FALSE, INFINITE) - WAIT_OBJECT_0;

if (dwEvent == 1)
{
// Shutdown event.
break;
}

DWORD cbCompletion = 0;
OVERLAPPED* pOverlapped = nullptr;
ULONG_PTR completionKey;

BOOL success = GetQueuedCompletionStatus(
pFileMonitor->m_hCompletionPort,
&cbCompletion,
&completionKey,
&pOverlapped,
INFINITE);

DBG_ASSERT(fSuccess);
dwErrorStatus = fSuccess ? ERROR_SUCCESS : GetLastError();

if (completionKey == FILE_WATCHER_SHUTDOWN_KEY)
{
break;
}
DBG_ASSERT(success);
(void)success;

DBG_ASSERT(pOverlapped != NULL);
if (pOverlapped != NULL)
DBG_ASSERT(pOverlapped != nullptr);
if (pOverlapped != nullptr)
{
pFileMonitor->HandleChangeCompletion(cbCompletion);

Expand All @@ -187,11 +216,9 @@ Win32 error
pFileMonitor->Monitor();
}
}
pOverlapped = NULL;
cbCompletion = 0;
}

pFileMonitor->m_fThreadExit = TRUE;
pFileMonitor->m_fThreadExit = true;

if (pFileMonitor->m_fShadowCopyEnabled)
{
Expand All @@ -205,7 +232,6 @@ Win32 error
ExitThread(0);
}


HRESULT
FILE_WATCHER::HandleChangeCompletion(
_In_ DWORD cbCompletion
Expand Down Expand Up @@ -247,7 +273,7 @@ HRESULT
//
// There could be a FCN overflow
// Let assume the file got changed instead of checking files
// Othersie we have to cache the file info
// Otherwise we have to cache the file info
//
if (cbCompletion == 0)
{
Expand All @@ -256,9 +282,9 @@ HRESULT
else
{
auto pNotificationInfo = (FILE_NOTIFY_INFORMATION*)_buffDirectoryChanges.QueryPtr();
DBG_ASSERT(pNotificationInfo != NULL);
DBG_ASSERT(pNotificationInfo != nullptr);

while (pNotificationInfo != NULL)
while (pNotificationInfo != nullptr)
{
//
// check whether the monitored file got changed
Expand All @@ -276,19 +302,23 @@ HRESULT
//
// Look for changes to dlls when shadow copying is enabled.
//
std::wstring notification(pNotificationInfo->FileName, pNotificationInfo->FileNameLength / sizeof(WCHAR));
std::filesystem::path notificationPath(notification);
if (m_fShadowCopyEnabled && notificationPath.extension().compare(L".dll") == 0)

if (m_fShadowCopyEnabled)
{
fDllChanged = TRUE;
std::wstring notification(pNotificationInfo->FileName, pNotificationInfo->FileNameLength / sizeof(WCHAR));
std::filesystem::path notificationPath(notification);
if (notificationPath.extension().compare(L".dll") == 0)
{
fDllChanged = TRUE;
}
}

//
// Advance to next notification
//
if (pNotificationInfo->NextEntryOffset == 0)
{
pNotificationInfo = NULL;
pNotificationInfo = nullptr;
}
else
{
Expand Down Expand Up @@ -326,8 +356,8 @@ FILE_WATCHER::TimerCallback(
_In_ PTP_TIMER Timer
)
{
Instance;
Timer;
UNREFERENCED_PARAMETER(Instance);
UNREFERENCED_PARAMETER(Timer);
CopyAndShutdown((FILE_WATCHER*)Context);
}

Expand All @@ -342,7 +372,7 @@ DWORD WINAPI FILE_WATCHER::CopyAndShutdown(FILE_WATCHER* watcher)

watcher->m_copied = true;

LOG_INFO(L"Starting copy on shutdown in filewatcher, creating directory.");
LOG_INFO(L"Starting copy on shutdown in file watcher, creating directory.");

auto directoryNameInt = 0;
auto currentShadowCopyDirectory = std::filesystem::path(watcher->m_shadowCopyPath);
Expand Down Expand Up @@ -402,9 +432,7 @@ FILE_WATCHER::RunNotificationCallback(
HRESULT
FILE_WATCHER::Monitor(VOID)
{
HRESULT hr = S_OK;
DWORD cbRead;

ZeroMemory(&_overlapped, sizeof(_overlapped));

RETURN_LAST_ERROR_IF(!ReadDirectoryChangesW(_hDirectory,
Expand All @@ -414,15 +442,15 @@ FILE_WATCHER::Monitor(VOID)
FILE_NOTIFY_VALID_MASK & ~FILE_NOTIFY_CHANGE_LAST_ACCESS,
&cbRead,
&_overlapped,
NULL));
nullptr));

// Check if file exist because ReadDirectoryChangesW would not fire events for existing files
if (GetFileAttributes(_strFullName.QueryStr()) != INVALID_FILE_ATTRIBUTES)
{
PostQueuedCompletionStatus(m_hCompletionPort, 0, 0, &_overlapped);
}

return hr;
return S_OK;
}

VOID
Expand All @@ -440,9 +468,9 @@ FILE_WATCHER::StopMonitor()

LOG_INFO(L"Stopping file watching.");

// signal the file watch thread to exit
PostQueuedCompletionStatus(m_hCompletionPort, 0, FILE_WATCHER_SHUTDOWN_KEY, NULL);
WaitForMonitor(200);
// Signal the file watcher thread to exit
SetEvent(m_pShutdownEvent);
WaitForWatcherThreadExit();

if (m_fShadowCopyEnabled)
{
Expand Down
8 changes: 5 additions & 3 deletions src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class FILE_WATCHER{

~FILE_WATCHER();

void WaitForMonitor(DWORD dwRetryCounter);
void WaitForWatcherThreadExit();

HRESULT Create(
_In_ PCWSTR pszDirectoryToMonitor,
Expand Down Expand Up @@ -59,8 +59,9 @@ class FILE_WATCHER{
HandleWrapper<NullHandleTraits> m_hCompletionPort;
HandleWrapper<NullHandleTraits> m_hChangeNotificationThread;
HandleWrapper<NullHandleTraits> _hDirectory;
HandleWrapper<NullHandleTraits> m_pDoneCopyEvent;
volatile BOOL m_fThreadExit;
HandleWrapper<NullHandleTraits> m_pDoneCopyEvent;
HandleWrapper<NullHandleTraits> m_pShutdownEvent;
std::atomic_bool m_fThreadExit;
STTIMER m_Timer;
SRWLOCK m_copyLock{};
BOOL m_copied;
Expand All @@ -75,4 +76,5 @@ class FILE_WATCHER{
DWORD m_shutdownTimeout;
OVERLAPPED _overlapped;
std::unique_ptr<AppOfflineTrackingApplication, IAPPLICATION_DELETER> _pApplication;
bool m_fRudeThreadTermination;
};