Author: Nico Weber Date: 2021-06-18T18:51:41-04:00 New Revision: fb32de9e97af0921242a021e30020ffacf7aa6e2
URL: https://github.com/llvm/llvm-project/commit/fb32de9e97af0921242a021e30020ffacf7aa6e2 DIFF: https://github.com/llvm/llvm-project/commit/fb32de9e97af0921242a021e30020ffacf7aa6e2.diff LOG: Re-Revert "DirectoryWatcher: add an implementation for Windows" This reverts commit 76f1baa7875acd88bdd4b431eed6e2d2decfc0fe. Also reverts 2 follow-ups: 1. Revert "DirectoryWatcher: also wait for the notifier thread" This reverts commit 527a1821e6f8e115db3335a3341c7ac491725a0d. 2. Revert "DirectoryWatcher: close a possible window of race on Windows" This reverts commit a6948da86ad7e78d66b26263c2681ef6385cc234. Makes tests hang, see comments on https://reviews.llvm.org/D88666 Added: Modified: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp clang/unittests/DirectoryWatcher/CMakeLists.txt Removed: ################################################################################ diff --git a/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp b/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp index 8a4f5a87d967c..25cbcf536388a 100644 --- a/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp +++ b/clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp @@ -6,12 +6,19 @@ // //===----------------------------------------------------------------------===// +// TODO: This is not yet an implementation, but it will make it so Windows +// builds don't fail. + #include "DirectoryScanner.h" #include "clang/DirectoryWatcher/DirectoryWatcher.h" + #include "llvm/ADT/STLExtras.h" -#include "llvm/Support/ConvertUTF.h" +#include "llvm/ADT/ScopeExit.h" +#include "llvm/Support/AlignOf.h" +#include "llvm/Support/Errno.h" +#include "llvm/Support/Mutex.h" #include "llvm/Support/Path.h" -#include "llvm/Support/Windows/WindowsSupport.h" +#include <atomic> #include <condition_variable> #include <mutex> #include <queue> @@ -21,283 +28,23 @@ namespace { -using DirectoryWatcherCallback = - std::function<void(llvm::ArrayRef<clang::DirectoryWatcher::Event>, bool)>; - using namespace llvm; using namespace clang; class DirectoryWatcherWindows : public clang::DirectoryWatcher { - OVERLAPPED Overlapped; - - std::vector<DWORD> Notifications; - - std::thread WatcherThread; - std::thread HandlerThread; - std::function<void(ArrayRef<DirectoryWatcher::Event>, bool)> Callback; - SmallString<MAX_PATH> Path; - HANDLE Terminate; - - std::mutex Mutex; - bool WatcherActive = false; - bool NotifierActive = false; - std::condition_variable Ready; - - class EventQueue { - std::mutex M; - std::queue<DirectoryWatcher::Event> Q; - std::condition_variable CV; - - public: - void emplace(DirectoryWatcher::Event::EventKind Kind, StringRef Path) { - { - std::unique_lock<std::mutex> L(M); - Q.emplace(Kind, Path); - } - CV.notify_one(); - } - - DirectoryWatcher::Event pop_front() { - std::unique_lock<std::mutex> L(M); - while (true) { - if (!Q.empty()) { - DirectoryWatcher::Event E = Q.front(); - Q.pop(); - return E; - } - CV.wait(L, [this]() { return !Q.empty(); }); - } - } - } Q; - public: - DirectoryWatcherWindows(HANDLE DirectoryHandle, bool WaitForInitialSync, - DirectoryWatcherCallback Receiver); - - ~DirectoryWatcherWindows() override; - - void InitialScan(); - void WatcherThreadProc(HANDLE DirectoryHandle); - void NotifierThreadProc(bool WaitForInitialSync); + ~DirectoryWatcherWindows() override { } + void InitialScan() { } + void EventReceivingLoop() { } + void StopWork() { } }; - -DirectoryWatcherWindows::DirectoryWatcherWindows( - HANDLE DirectoryHandle, bool WaitForInitialSync, - DirectoryWatcherCallback Receiver) - : Callback(Receiver), Terminate(INVALID_HANDLE_VALUE) { - // Pre-compute the real location as we will be handing over the directory - // handle to the watcher and performing synchronous operations. - { - DWORD Size = GetFinalPathNameByHandleW(DirectoryHandle, NULL, 0, 0); - std::unique_ptr<WCHAR[]> Buffer{new WCHAR[Size]}; - Size = GetFinalPathNameByHandleW(DirectoryHandle, Buffer.get(), Size, 0); - Buffer[Size] = L'\0'; - llvm::sys::windows::UTF16ToUTF8(Buffer.get(), Size, Path); - } - - size_t EntrySize = sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH * sizeof(WCHAR); - Notifications.resize((4 * EntrySize) / sizeof(DWORD)); - - memset(&Overlapped, 0, sizeof(Overlapped)); - Overlapped.hEvent = - CreateEventW(NULL, /*bManualReset=*/FALSE, /*bInitialState=*/FALSE, NULL); - assert(Overlapped.hEvent && "unable to create event"); - - Terminate = - CreateEventW(NULL, /*bManualReset=*/TRUE, /*bInitialState=*/FALSE, NULL); - - WatcherThread = std::thread([this, DirectoryHandle]() { - this->WatcherThreadProc(DirectoryHandle); - }); - - if (WaitForInitialSync) - InitialScan(); - - HandlerThread = std::thread([this, WaitForInitialSync]() { - this->NotifierThreadProc(WaitForInitialSync); - }); - - std::unique_lock<std::mutex> lock(Mutex); - Ready.wait(lock, [this] { - return this->WatcherActive && this->NotifierActive; - }); -} - -DirectoryWatcherWindows::~DirectoryWatcherWindows() { - // Signal the Watcher to exit. - SetEvent(Terminate); - HandlerThread.join(); - WatcherThread.join(); - CloseHandle(Terminate); - CloseHandle(Overlapped.hEvent); -} - -void DirectoryWatcherWindows::InitialScan() { - std::unique_lock<std::mutex> lock(Mutex); - Ready.wait(lock, [this] { return this->WatcherActive; }); - - Callback(getAsFileEvents(scanDirectory(Path.data())), /*IsInitial=*/true); -} - -void DirectoryWatcherWindows::WatcherThreadProc(HANDLE DirectoryHandle) { - { - std::unique_lock<std::mutex> lock(Mutex); - WatcherActive = true; - } - Ready.notify_one(); - - while (true) { - // We do not guarantee subdirectories, but macOS already provides - // subdirectories, might as well as ... - BOOL WatchSubtree = TRUE; - DWORD NotifyFilter = FILE_NOTIFY_CHANGE_FILE_NAME - | FILE_NOTIFY_CHANGE_DIR_NAME - | FILE_NOTIFY_CHANGE_SIZE - | FILE_NOTIFY_CHANGE_LAST_WRITE - | FILE_NOTIFY_CHANGE_CREATION; - - DWORD BytesTransferred; - if (!ReadDirectoryChangesW(DirectoryHandle, Notifications.data(), - Notifications.size() * sizeof(DWORD), - WatchSubtree, NotifyFilter, &BytesTransferred, - &Overlapped, NULL)) { - Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated, - ""); - break; - } - - HANDLE Handles[2] = { Terminate, Overlapped.hEvent }; - switch (WaitForMultipleObjects(2, Handles, FALSE, INFINITE)) { - case WAIT_OBJECT_0: // Terminate Request - case WAIT_FAILED: // Failure - Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated, - ""); - (void)CloseHandle(DirectoryHandle); - return; - case WAIT_TIMEOUT: // Spurious wakeup? - continue; - case WAIT_OBJECT_0 + 1: // Directory change - break; - } - - if (!GetOverlappedResult(DirectoryHandle, &Overlapped, &BytesTransferred, - FALSE)) { - Q.emplace(DirectoryWatcher::Event::EventKind::WatchedDirRemoved, - ""); - Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated, - ""); - break; - } - - // There was a buffer underrun on the kernel side. We may have lost - // events, please re-synchronize. - if (BytesTransferred == 0) { - Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated, - ""); - break; - } - - for (FILE_NOTIFY_INFORMATION *I = - (FILE_NOTIFY_INFORMATION *)Notifications.data(); - I; - I = I->NextEntryOffset - ? (FILE_NOTIFY_INFORMATION *)((CHAR *)I + I->NextEntryOffset) - : NULL) { - DirectoryWatcher::Event::EventKind Kind = - DirectoryWatcher::Event::EventKind::WatcherGotInvalidated; - switch (I->Action) { - case FILE_ACTION_ADDED: - case FILE_ACTION_MODIFIED: - case FILE_ACTION_RENAMED_NEW_NAME: - Kind = DirectoryWatcher::Event::EventKind::Modified; - break; - case FILE_ACTION_REMOVED: - case FILE_ACTION_RENAMED_OLD_NAME: - Kind = DirectoryWatcher::Event::EventKind::Removed; - break; - } - - SmallString<MAX_PATH> filename; - sys::windows::UTF16ToUTF8(I->FileName, I->FileNameLength / sizeof(WCHAR), - filename); - Q.emplace(Kind, filename); - } - } - - (void)CloseHandle(DirectoryHandle); -} - -void DirectoryWatcherWindows::NotifierThreadProc(bool WaitForInitialSync) { - // If we did not wait for the initial sync, then we should perform the - // scan when we enter the thread. - if (!WaitForInitialSync) - this->InitialScan(); - - { - std::unique_lock<std::mutex> lock(Mutex); - NotifierActive = true; - } - Ready.notify_one(); - - while (true) { - DirectoryWatcher::Event E = Q.pop_front(); - Callback(E, /*IsInitial=*/false); - if (E.Kind == DirectoryWatcher::Event::EventKind::WatcherGotInvalidated) - break; - } -} - -auto error(DWORD ErrorCode) { - DWORD Flags = FORMAT_MESSAGE_ALLOCATE_BUFFER - | FORMAT_MESSAGE_FROM_SYSTEM - | FORMAT_MESSAGE_IGNORE_INSERTS; - - LPSTR Buffer; - if (!FormatMessageA(Flags, NULL, ErrorCode, - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPSTR)&Buffer, - 0, NULL)) { - return make_error<llvm::StringError>("error " + utostr(ErrorCode), - inconvertibleErrorCode()); - } - std::string Message{Buffer}; - LocalFree(Buffer); - return make_error<llvm::StringError>(Message, inconvertibleErrorCode()); -} - } // namespace llvm::Expected<std::unique_ptr<DirectoryWatcher>> -clang::DirectoryWatcher::create(StringRef Path, - DirectoryWatcherCallback Receiver, - bool WaitForInitialSync) { - if (Path.empty()) - llvm::report_fatal_error( - "DirectoryWatcher::create can not accept an empty Path."); - - if (!sys::fs::is_directory(Path)) - llvm::report_fatal_error( - "DirectoryWatcher::create can not accept a filepath."); - - SmallVector<wchar_t, MAX_PATH> WidePath; - if (sys::windows::UTF8ToUTF16(Path, WidePath)) - return llvm::make_error<llvm::StringError>( - "unable to convert path to UTF-16", llvm::inconvertibleErrorCode()); - - DWORD DesiredAccess = FILE_LIST_DIRECTORY; - DWORD ShareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; - DWORD CreationDisposition = OPEN_EXISTING; - DWORD FlagsAndAttributes = FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED; - - HANDLE DirectoryHandle = - CreateFileW(WidePath.data(), DesiredAccess, ShareMode, - /*lpSecurityAttributes=*/NULL, CreationDisposition, - FlagsAndAttributes, NULL); - if (DirectoryHandle == INVALID_HANDLE_VALUE) - return error(GetLastError()); - - // NOTE: We use the watcher instance as a RAII object to discard the handles - // for the directory in case of an error. Hence, this is early allocated, - // with the state being written directly to the watcher. - return std::make_unique<DirectoryWatcherWindows>( - DirectoryHandle, WaitForInitialSync, Receiver); +clang::DirectoryWatcher::create( + StringRef Path, + std::function<void(llvm::ArrayRef<DirectoryWatcher::Event>, bool)> Receiver, + bool WaitForInitialSync) { + return llvm::Expected<std::unique_ptr<DirectoryWatcher>>( + llvm::errorCodeToError(std::make_error_code(std::errc::not_supported))); } diff --git a/clang/unittests/DirectoryWatcher/CMakeLists.txt b/clang/unittests/DirectoryWatcher/CMakeLists.txt index 84a1a9d40c250..0355525a86b0a 100644 --- a/clang/unittests/DirectoryWatcher/CMakeLists.txt +++ b/clang/unittests/DirectoryWatcher/CMakeLists.txt @@ -1,4 +1,4 @@ -if(APPLE OR CMAKE_SYSTEM_NAME MATCHES "Linux" OR CMAKE_SYSTEM_NAME STREQUAL Windows) +if(APPLE OR CMAKE_SYSTEM_NAME MATCHES "Linux") set(LLVM_LINK_COMPONENTS Support _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits