From 2492d0b38f819c13022a431c5aa41c2b9231486a Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Sun, 2 Jun 2024 00:51:11 +0200 Subject: [PATCH] Refactor `WindowsFileSystemWatcher` to properly handle file modifications --- .../Platform/Base/FileSystemWatcherBase.h | 9 +- .../Platform/Mac/MacFileSystemWatcher.cpp | 12 +- .../Windows/WindowsFileSystemWatcher.cpp | 192 +++++++++--------- .../Windows/WindowsFileSystemWatcher.h | 12 +- 4 files changed, 116 insertions(+), 109 deletions(-) diff --git a/Source/Engine/Platform/Base/FileSystemWatcherBase.h b/Source/Engine/Platform/Base/FileSystemWatcherBase.h index df57695bd..81e28335b 100644 --- a/Source/Engine/Platform/Base/FileSystemWatcherBase.h +++ b/Source/Engine/Platform/Base/FileSystemWatcherBase.h @@ -11,10 +11,11 @@ /// enum class FileSystemAction { - Unknown, - Create, - Delete, - Modify, + Unknown = 0, + Create = 1, + Delete = 2, + Modify = 4, + Rename = 8, }; /// diff --git a/Source/Engine/Platform/Mac/MacFileSystemWatcher.cpp b/Source/Engine/Platform/Mac/MacFileSystemWatcher.cpp index 1d4a64f78..ba3358d35 100644 --- a/Source/Engine/Platform/Mac/MacFileSystemWatcher.cpp +++ b/Source/Engine/Platform/Mac/MacFileSystemWatcher.cpp @@ -35,16 +35,18 @@ void DirectoryWatchCallback( ConstFSEventStreamRef StreamRef, void* FileWatcherP { action = FileSystemAction::Create; } - - if (renamed || modified) + if (renamed) { - action = FileSystemAction::Delete; + action = FileSystemAction::Rename; } - - if (removed) + if (rmodified) { action = FileSystemAction::Modify; } + if (removed) + { + action = FileSystemAction::Delete; + } const String resolvedPath = AppleUtils::ToString((CFStringRef)CFArrayGetValueAtIndex(EventPathArray,EventIndex)); diff --git a/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.cpp b/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.cpp index cb7eaeaf1..3194d8533 100644 --- a/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.cpp +++ b/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.cpp @@ -11,117 +11,45 @@ #include "Engine/Core/Collections/Array.h" #include "../Win32/IncludeWindowsHeaders.h" -BOOL RefreshWatch(WindowsFileSystemWatcher* watcher); - namespace FileSystemWatchers { CriticalSection Locker; Array> Watchers; Win32Thread* Thread = nullptr; + Windows::HANDLE IoHandle = INVALID_HANDLE_VALUE; bool ThreadActive; int32 Run() { + DWORD numBytes = 0; + LPOVERLAPPED overlapped; + ULONG_PTR compKey = 0; while (ThreadActive) { - SleepEx(INFINITE, true); + if (GetQueuedCompletionStatus(IoHandle, &numBytes, &compKey, &overlapped, INFINITE) && overlapped && numBytes != 0) + { + // Send further to the specific watcher + Locker.Lock(); + for (auto watcher : Watchers) + { + if ((OVERLAPPED*)&watcher->Overlapped == overlapped) + { + watcher->NotificationCompletion(); + break; + } + } + Locker.Unlock(); + } } return 0; } - - static void CALLBACK StopProc(ULONG_PTR arg) - { - ThreadActive = false; - } - - static void CALLBACK AddDirectoryProc(ULONG_PTR arg) - { - const auto watcher = (FileSystemWatcher*)arg; - RefreshWatch(watcher); - } }; -VOID CALLBACK NotificationCompletion(DWORD dwErrorCode, DWORD dwNumberOfBytesTransfered, LPOVERLAPPED lpOverlapped) -{ - auto watcher = (FileSystemWatcher*)lpOverlapped->hEvent; - if (dwErrorCode == ERROR_OPERATION_ABORTED || - dwNumberOfBytesTransfered <= 0 || - !watcher) - { - return; - } - - // Swap buffers - watcher->CurrentBuffer = (watcher->CurrentBuffer + 1) % 2; - - // Get the new read issued as fast as possible - if (!watcher->StopNow) - { - RefreshWatch(watcher); - } - - // Process notifications - auto notify = (FILE_NOTIFY_INFORMATION*)watcher->Buffer[(watcher->CurrentBuffer + 1) % 2]; - do - { - // Convert action type - auto action = FileSystemAction::Unknown; - switch (notify->Action) - { - case FILE_ACTION_RENAMED_NEW_NAME: - case FILE_ACTION_ADDED: - action = FileSystemAction::Create; - break; - case FILE_ACTION_RENAMED_OLD_NAME: - case FILE_ACTION_REMOVED: - action = FileSystemAction::Delete; - break; - case FILE_ACTION_MODIFIED: - action = FileSystemAction::Modify; - break; - default: - action = FileSystemAction::Unknown; - break; - } - if (action != FileSystemAction::Unknown) - { - // Build path - String path(notify->FileName, notify->FileNameLength / sizeof(WCHAR)); - path = watcher->Directory / path; - - // Send event - watcher->OnEvent(path, action); - } - - // Move to the next notify - notify = (FILE_NOTIFY_INFORMATION*)((byte*)notify + notify->NextEntryOffset); - } while (notify->NextEntryOffset != 0); -} - -// Refreshes the directory monitoring -BOOL RefreshWatch(WindowsFileSystemWatcher* watcher) -{ - DWORD dwBytesReturned = 0; - return ReadDirectoryChangesW( - watcher->DirectoryHandle, - watcher->Buffer[watcher->CurrentBuffer], - FileSystemWatcher::BufferSize, - watcher->WithSubDirs ? TRUE : FALSE, - FILE_NOTIFY_CHANGE_CREATION | FILE_NOTIFY_CHANGE_SIZE | FILE_NOTIFY_CHANGE_FILE_NAME, - &dwBytesReturned, - (OVERLAPPED*)&watcher->Overlapped, - NotificationCompletion - ); -} - WindowsFileSystemWatcher::WindowsFileSystemWatcher(const String& directory, bool withSubDirs) : FileSystemWatcherBase(directory, withSubDirs) - , StopNow(false) - , CurrentBuffer(0) { // Setup Platform::MemoryClear(&Overlapped, sizeof(Overlapped)); - ((OVERLAPPED&)Overlapped).hEvent = this; // Create directory handle for events handling DirectoryHandle = CreateFileW( @@ -144,25 +72,26 @@ WindowsFileSystemWatcher::WindowsFileSystemWatcher(const String& directory, bool FileSystemWatchers::Watchers.Add(this); if (!FileSystemWatchers::Thread) { + FileSystemWatchers::IoHandle = CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1); FileSystemWatchers::ThreadActive = true; FileSystemWatchers::Thread = ThreadSpawner::Start(FileSystemWatchers::Run, TEXT("File System Watchers"), ThreadPriority::BelowNormal); } + CreateIoCompletionPort(DirectoryHandle, FileSystemWatchers::IoHandle, 0, 1); FileSystemWatchers::Locker.Unlock(); - // Issue the first read - QueueUserAPC(FileSystemWatchers::AddDirectoryProc, FileSystemWatchers::Thread->GetHandle(), (ULONG_PTR)this); + // Initialize filesystem events tracking + ReadDirectoryChanges(); } WindowsFileSystemWatcher::~WindowsFileSystemWatcher() { FileSystemWatchers::Locker.Lock(); FileSystemWatchers::Watchers.Remove(this); + StopNow = true; FileSystemWatchers::Locker.Unlock(); if (DirectoryHandle != INVALID_HANDLE_VALUE) { - StopNow = true; - #if WINVER >= 0x600 CancelIoEx(DirectoryHandle, (OVERLAPPED*)&Overlapped); #else @@ -180,12 +109,85 @@ WindowsFileSystemWatcher::~WindowsFileSystemWatcher() if (FileSystemWatchers::Watchers.IsEmpty() && FileSystemWatchers::Thread) { FileSystemWatchers::ThreadActive = false; - QueueUserAPC(FileSystemWatchers::StopProc, FileSystemWatchers::Thread->GetHandle(), 0); + FileSystemWatchers::Locker.Unlock(); + PostQueuedCompletionStatus(FileSystemWatchers::IoHandle, 0, 0, nullptr); FileSystemWatchers::Thread->Join(); + FileSystemWatchers::Locker.Lock(); Delete(FileSystemWatchers::Thread); FileSystemWatchers::Thread = nullptr; + CloseHandle(FileSystemWatchers::IoHandle); + FileSystemWatchers::IoHandle = INVALID_HANDLE_VALUE; } FileSystemWatchers::Locker.Unlock(); } +void WindowsFileSystemWatcher::ReadDirectoryChanges() +{ + BOOL result = ReadDirectoryChangesW( + DirectoryHandle, + Buffer, + BufferSize, + WithSubDirs ? TRUE : FALSE, + FILE_NOTIFY_CHANGE_CREATION | FILE_NOTIFY_CHANGE_SIZE | FILE_NOTIFY_CHANGE_FILE_NAME, + nullptr, + (OVERLAPPED*)&Overlapped, + nullptr + ); + if (!result) + { + LOG_WIN32_LAST_ERROR; + Sleep(1); + } +} + +void WindowsFileSystemWatcher::NotificationCompletion() +{ + ScopeLock lock(Locker); + + // Process notifications + auto notify = (FILE_NOTIFY_INFORMATION*)Buffer; + do + { + // Convert action type + FileSystemAction action; + switch (notify->Action) + { + case FILE_ACTION_RENAMED_NEW_NAME: + case FILE_ACTION_RENAMED_OLD_NAME: + action = FileSystemAction::Rename; + break; + case FILE_ACTION_ADDED: + action = FileSystemAction::Create; + break; + case FILE_ACTION_REMOVED: + action = FileSystemAction::Delete; + break; + case FILE_ACTION_MODIFIED: + action = FileSystemAction::Modify; + break; + default: + action = FileSystemAction::Unknown; + break; + } + if (action != FileSystemAction::Unknown) + { + // Build path + String path(notify->FileName, notify->FileNameLength / sizeof(WCHAR)); + path = Directory / path; + + // Send event + OnEvent(path, action); + } + + // Move to the next notify + notify = (FILE_NOTIFY_INFORMATION*)((byte*)notify + notify->NextEntryOffset); + } while (notify->NextEntryOffset != 0); + + // Get the new read issued as fast as possible + if (!StopNow) + { + ReadDirectoryChanges(); + } +} + #endif diff --git a/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.h b/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.h index 36cb2c8db..fbe0a1a33 100644 --- a/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.h +++ b/Source/Engine/Platform/Windows/WindowsFileSystemWatcher.h @@ -13,7 +13,6 @@ class FLAXENGINE_API WindowsFileSystemWatcher : public FileSystemWatcherBase { public: - /// /// Initializes a new instance of the class. /// @@ -27,13 +26,16 @@ public: ~WindowsFileSystemWatcher(); public: - Windows::OVERLAPPED Overlapped; Windows::HANDLE DirectoryHandle; - bool StopNow; - int32 CurrentBuffer; + Win32Thread* Thread = nullptr; + Win32CriticalSection Locker; + bool StopNow = false; static const int32 BufferSize = 32 * 1024; - byte Buffer[2][BufferSize]; + alignas(Windows::DWORD) byte Buffer[BufferSize]; + + void ReadDirectoryChanges(); + void NotificationCompletion(); }; #endif