filesystem: refactor win32 watcher_impl to use absl mutex and fix buffer alignment - #46655
filesystem: refactor win32 watcher_impl to use absl mutex and fix buffer alignment#46655citrus7 wants to merge 1 commit into
Conversation
…fer alignment Signed-off-by: Jonathan Wu <jtwu@google.com>
bf224e8 to
066d431
Compare
yanavlasov
left a comment
yanavlasov
left a comment
There was a problem hiding this comment.
Also need format to be fixed.
/wait
| @@ -1,11 +1,10 @@ | |||
| #pragma once | |||
|
|
|||
| #include <concurrent_queue.h> | |||
There was a problem hiding this comment.
Was this header not available in Windows toolchain?
There was a problem hiding this comment.
Yea using this forces the linker to pull in libconcrt.lib which can conflict with libc++, this change should make this library a lot more portable.
| callback_map_[fii_key] = std::make_unique<DirectoryWatch>(); | ||
| callback_map_[fii_key]->dir_handle_ = dir_handle; | ||
| callback_map_[fii_key]->buffer_.resize(16384); | ||
| callback_map_[fii_key]->buffer_.resize(1024); |
There was a problem hiding this comment.
why is this changed to 1024?
before this PR, it was resize(16384) on a vector<uint8_t> (16 KiB), now it is resize(1024) on a vector (4 KiB). Do we want to preserve capacity e.g., resize(4096)
There was a problem hiding this comment.
ReadDirectoryChangesW requires the buffer to be DWORD-aligned (aligned to a 4-byte / 32-bit boundary). (https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-readdirectorychangesw)
Gemini seems to think that the old value was just a placeholder and incorrect.
3 participants
Additional Description:
Refactors the Win32 filesystem watcher (
WatcherImpl):<concurrent_queue.h>(concurrency::concurrent_queue) with a thread-safeThreadSafeQueueusingabsl::Mutex. This eliminates duplicate symbol link conflicts betweenlibc++and MSVClibconcrt.lib.DirectoryWatchbuffer type fromstd::vector<uint8_t>tostd::vector<DWORD>ensuring 4-byte DWORD alignment required byReadDirectoryChangesWandFILE_NOTIFY_INFORMATION.DirectoryWatch::watches_with anabsl::Mutex.callAndLogOnErrorwith exception handling and rate-limited logging.watcher_impl_test.ccto ensure file handles are cleanly closed before read events and flushes.Risk Level: medium (Win32 filesystem watcher refactor)
Testing:
bazel test //test/common/filesystem:watcher_impl_testDocs Changes: n/a
Release Notes: Included under bug_fixes for Win32 watcher
Platform Specific Features: Windows