From c8dd63c845abbb27e9aa9db92669e33610167d4c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 16 Nov 2017 16:02:51 -0500 Subject: [PATCH 1/6] Improve Windows event logging to see actual batch contents --- .../windows/windows_worker_platform.cpp | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/worker/windows/windows_worker_platform.cpp b/src/worker/windows/windows_worker_platform.cpp index 9dfcc702..c4a8a8b8 100644 --- a/src/worker/windows/windows_worker_platform.cpp +++ b/src/worker/windows/windows_worker_platform.cpp @@ -329,25 +329,43 @@ class WindowsWorkerPlatform : public WorkerPlatform } EntryKind kind = stat->get_entry_kind(); + ostream &logline = LOGGER; + logline << "Event at [" << path << "] "; + switch (info->Action) { - case FILE_ACTION_ADDED: messages.created(move(path), kind); break; - case FILE_ACTION_MODIFIED: messages.modified(move(path), kind); break; - case FILE_ACTION_REMOVED: messages.deleted(move(path), kind); break; + case FILE_ACTION_ADDED: + logline << "FILE_ACTION_ADDED " << kind << "." << endl; + messages.created(move(path), kind); + break; + case FILE_ACTION_MODIFIED: + logline << "FILE_ACTION_MODIFIED " << kind << "." << endl; + messages.modified(move(path), kind); + break; + case FILE_ACTION_REMOVED: + logline << "FILE_ACTION_REMOVED " << kind << "." << endl; + messages.deleted(move(path), kind); + break; case FILE_ACTION_RENAMED_OLD_NAME: + logline << "FILE_ACTION_RENAMED_OLD_NAME " << kind << "." << endl; old_path_seen = true; old_path = move(path); break; case FILE_ACTION_RENAMED_NEW_NAME: + logline << "FILE_ACTION_RENAMED_NEW_NAME "; if (old_path_seen) { // Old name received first + logline << kind << "." << endl; messages.renamed(move(old_path), move(path), kind); old_path_seen = false; } else { // No old name. Treat it as a creation + logline << "(unpaired) " << kind << "." << endl; messages.created(move(path), kind); } break; default: + logline << " with unexpected action " << info->Action << "." << endl; + ostringstream out; out << "Unexpected action " << info->Action << " reported by ReadDirectoryChangesW for " << path; return error_result(out.str()); From 39742b149b66b9e311d82fa0d9e44e78a0417c81 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 16 Nov 2017 16:29:04 -0500 Subject: [PATCH 2/6] Okay, we can't reliably assert that kind on Windows --- test/events/kind-change.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/events/kind-change.test.js b/test/events/kind-change.test.js index a8ee6b5d..6c6ce3d7 100644 --- a/test/events/kind-change.test.js +++ b/test/events/kind-change.test.js @@ -135,7 +135,7 @@ const {EventMatcher} = require('../matcher'); await fs.mkdir(reusedPath) await until('delete and create events arrive', matcher.allEvents( - {action: 'deleted', kind: 'file', path: reusedPath}, + {action: 'deleted', path: reusedPath}, {action: 'created', kind: 'directory', path: reusedPath} )) }) From 94f8b63f05299d355f393b47bb43a43eb629a9b7 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 16 Nov 2017 16:29:35 -0500 Subject: [PATCH 3/6] Track rename pairs across batches on Windows --- src/worker/windows/subscription.cpp | 3 ++- src/worker/windows/subscription.h | 12 +++++++++++ .../windows/windows_worker_platform.cpp | 20 +++++++------------ 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/worker/windows/subscription.cpp b/src/worker/windows/subscription.cpp index 4e3909d8..30098748 100644 --- a/src/worker/windows/subscription.cpp +++ b/src/worker/windows/subscription.cpp @@ -33,7 +33,8 @@ Subscription::Subscription(ChannelID channel, recursive{recursive}, buffer_size{DEFAULT_BUFFER_SIZE}, buffer{new BYTE[buffer_size]}, - written{new BYTE[buffer_size]} + written{new BYTE[buffer_size]}, + old_path_seen{false} { ZeroMemory(&overlapped, sizeof(OVERLAPPED)); overlapped.hEvent = this; diff --git a/src/worker/windows/subscription.h b/src/worker/windows/subscription.h index 5cde3ed2..1f5358fb 100644 --- a/src/worker/windows/subscription.h +++ b/src/worker/windows/subscription.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "../../message.h" #include "../../result.h" @@ -43,6 +44,14 @@ class Subscription const bool &is_terminating() const { return terminating; } + const std::string &get_old_path() const { return old_path; } + + void set_old_path(std::string &&old_path) { this->old_path = std::move(old_path); } + + const bool &was_old_path_seen() const { return old_path_seen; } + + void set_old_path_seen(bool old_path_seen) { this->old_path_seen = old_path_seen; } + private: CommandID command; ChannelID channel; @@ -57,6 +66,9 @@ class Subscription DWORD buffer_size; std::unique_ptr buffer; std::unique_ptr written; + + std::string old_path; + bool old_path_seen; }; #endif diff --git a/src/worker/windows/windows_worker_platform.cpp b/src/worker/windows/windows_worker_platform.cpp index c4a8a8b8..34a84db1 100644 --- a/src/worker/windows/windows_worker_platform.cpp +++ b/src/worker/windows/windows_worker_platform.cpp @@ -215,14 +215,12 @@ class WindowsWorkerPlatform : public WorkerPlatform MessageBuffer buffer; ChannelMessageBuffer messages(buffer, channel); size_t num_events = 0; - bool old_path_seen = false; - string old_path; while (true) { PFILE_NOTIFY_INFORMATION info = reinterpret_cast(base); num_events++; - Result<> pr = process_event_payload(info, sub, messages, old_path_seen, old_path); + Result<> pr = process_event_payload(info, sub, messages); if (pr.is_error()) { LOGGER << "Skipping entry " << pr << "." << endl; } @@ -301,11 +299,7 @@ class WindowsWorkerPlatform : public WorkerPlatform return out; } - Result<> process_event_payload(PFILE_NOTIFY_INFORMATION info, - Subscription *sub, - ChannelMessageBuffer &messages, - bool &old_path_seen, - string &old_path) + Result<> process_event_payload(PFILE_NOTIFY_INFORMATION info, Subscription *sub, ChannelMessageBuffer &messages) { ChannelID channel = sub->get_channel(); wstring relpathw{info->FileName, info->FileNameLength / sizeof(WCHAR)}; @@ -347,16 +341,16 @@ class WindowsWorkerPlatform : public WorkerPlatform break; case FILE_ACTION_RENAMED_OLD_NAME: logline << "FILE_ACTION_RENAMED_OLD_NAME " << kind << "." << endl; - old_path_seen = true; - old_path = move(path); + sub->set_old_path_seen(true); + sub->set_old_path(move(path)); break; case FILE_ACTION_RENAMED_NEW_NAME: logline << "FILE_ACTION_RENAMED_NEW_NAME "; - if (old_path_seen) { + if (sub->was_old_path_seen()) { // Old name received first logline << kind << "." << endl; - messages.renamed(move(old_path), move(path), kind); - old_path_seen = false; + messages.renamed(string(sub->get_old_path()), move(path), kind); + sub->set_old_path_seen(false); } else { // No old name. Treat it as a creation logline << "(unpaired) " << kind << "." << endl; From b92e8e8283816da0cc29a76142cb9c62aa8844cf Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 17 Nov 2017 10:49:32 -0500 Subject: [PATCH 4/6] Update cache entries for parent directory renames --- src/worker/windows/windows_worker_platform.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/worker/windows/windows_worker_platform.cpp b/src/worker/windows/windows_worker_platform.cpp index 913a2f32..ec9ea5d6 100644 --- a/src/worker/windows/windows_worker_platform.cpp +++ b/src/worker/windows/windows_worker_platform.cpp @@ -355,6 +355,7 @@ class WindowsWorkerPlatform : public WorkerPlatform if (sub->was_old_path_seen()) { // Old name received first logline << kind << "." << endl; + cache.update_for_rename(sub->get_old_path(), path); messages.renamed(string(sub->get_old_path()), move(path), kind); sub->set_old_path_seen(false); } else { From 18002d8542ef40dc06ae938b11df6f114e82a717 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 17 Nov 2017 11:18:50 -0500 Subject: [PATCH 5/6] Infer kinds from either end of the rename event --- src/worker/windows/subscription.h | 18 +++++++++++++++--- src/worker/windows/windows_worker_platform.cpp | 9 ++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/worker/windows/subscription.h b/src/worker/windows/subscription.h index 1f5358fb..73667460 100644 --- a/src/worker/windows/subscription.h +++ b/src/worker/windows/subscription.h @@ -44,14 +44,25 @@ class Subscription const bool &is_terminating() const { return terminating; } + void remember_old_path(std::string &&old_path, EntryKind kind) + { + this->old_path = std::move(old_path); + this->old_path_kind = kind; + this->old_path_seen = true; + } + + void clear_old_path() { + old_path.clear(); + old_path_kind = KIND_UNKNOWN; + old_path_seen = false; + } + const std::string &get_old_path() const { return old_path; } - void set_old_path(std::string &&old_path) { this->old_path = std::move(old_path); } + const EntryKind &get_old_path_kind() const { return old_path_kind; } const bool &was_old_path_seen() const { return old_path_seen; } - void set_old_path_seen(bool old_path_seen) { this->old_path_seen = old_path_seen; } - private: CommandID command; ChannelID channel; @@ -68,6 +79,7 @@ class Subscription std::unique_ptr written; std::string old_path; + EntryKind old_path_kind; bool old_path_seen; }; diff --git a/src/worker/windows/windows_worker_platform.cpp b/src/worker/windows/windows_worker_platform.cpp index ec9ea5d6..0edf53e5 100644 --- a/src/worker/windows/windows_worker_platform.cpp +++ b/src/worker/windows/windows_worker_platform.cpp @@ -347,17 +347,20 @@ class WindowsWorkerPlatform : public WorkerPlatform break; case FILE_ACTION_RENAMED_OLD_NAME: logline << "FILE_ACTION_RENAMED_OLD_NAME " << kind << "." << endl; - sub->set_old_path_seen(true); - sub->set_old_path(move(path)); + sub->remember_old_path(move(path), kind); break; case FILE_ACTION_RENAMED_NEW_NAME: logline << "FILE_ACTION_RENAMED_NEW_NAME "; if (sub->was_old_path_seen()) { // Old name received first + if (kind == KIND_UNKNOWN) { + kind = sub->get_old_path_kind(); + } + logline << kind << "." << endl; cache.update_for_rename(sub->get_old_path(), path); messages.renamed(string(sub->get_old_path()), move(path), kind); - sub->set_old_path_seen(false); + sub->clear_old_path(); } else { // No old name. Treat it as a creation logline << "(unpaired) " << kind << "." << endl; From 4a26fb31be33eddfc7fde03b660efd01aab8d4d4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 17 Nov 2017 11:27:34 -0500 Subject: [PATCH 6/6] :shirt: --- src/worker/windows/subscription.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/worker/windows/subscription.h b/src/worker/windows/subscription.h index 73667460..fc1b5100 100644 --- a/src/worker/windows/subscription.h +++ b/src/worker/windows/subscription.h @@ -51,7 +51,8 @@ class Subscription this->old_path_seen = true; } - void clear_old_path() { + void clear_old_path() + { old_path.clear(); old_path_kind = KIND_UNKNOWN; old_path_seen = false;