From e0250b7d5c6fcb74602039afc8457d715e11c1b8 Mon Sep 17 00:00:00 2001 From: Joran Dirk Greef Date: Thu, 3 Dec 2015 09:33:36 +0200 Subject: [PATCH] win: fix path for removed and renamed fs events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous behavior on Windows was to set the path to NULL for removed and renamed fs events. This was because the path provided by ReadDirectoryChangesW might (in rare cases) be an 8.3 short name which could then no longer be converted to a long name after the path had been removed or renamed. This meant that the user had to detect which path was actually deleted or renamed and required the user to rescan the entire watched subtree, taking several seconds or more for large subtrees. However, ReadDirectoryChangesW is publicly documented to emit 8.3 short names if the original handle for the changed path was opened using an 8.3 short name, and libuv may already emit 8.3 short names for other events if the path cannot be explicitly resolved to a long name. This commit fixes the path for removed and renamed fs events, and does not set the path to NULL, even if the path might be an 8.3 short name. This makes it possible for the user to do a partial scan of the subtree, restricting the scan to paths which match the long form or 8.3 short name (even if some of these are false positive matches). This means that deletes and renames can now be detected accurately on Windows within a few milliseconds. Fixes: https://github.com/libuv/libuv/issues/634 Refs: https://github.com/libuv/libuv/pull/199 PR-URL: https://github.com/libuv/libuv/pull/639 Reviewed-By: Saúl Ibarra Corretgé --- src/win/fs-event.c | 23 ++++++++++++++++------- test/test-fs-event.c | 26 +++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/win/fs-event.c b/src/win/fs-event.c index ba68f78c..76ecfeba 100644 --- a/src/win/fs-event.c +++ b/src/win/fs-event.c @@ -381,9 +381,10 @@ void uv_process_fs_event_req(uv_loop_t* loop, uv_req_t* req, if (handle->dirw) { /* - * We attempt to convert the file name to its long form for - * events that still point to valid files on disk. - * For removed and renamed events, we do not provide the file name. + * We attempt to resolve the long form of the file name explicitly. + * We only do this for file names that might still exist on disk. + * If this fails, we use the name given by ReadDirectoryChangesW. + * This may be the long form or the 8.3 short name in some cases. */ if (file_info->Action != FILE_ACTION_REMOVED && file_info->Action != FILE_ACTION_RENAMED_OLD_NAME) { @@ -438,16 +439,24 @@ void uv_process_fs_event_req(uv_loop_t* loop, uv_req_t* req, } /* - * If we couldn't get the long name - just use the name - * provided by ReadDirectoryChangesW. + * We could not resolve the long form explicitly. + * We therefore use the name given by ReadDirectoryChangesW. + * This may be the long form or the 8.3 short name in some cases. */ if (!long_filenamew) { filenamew = file_info->FileName; sizew = file_info->FileNameLength / sizeof(WCHAR); } } else { - /* Removed or renamed callbacks don't provide filename. */ - filenamew = NULL; + /* + * Removed or renamed events cannot be resolved to the long form. + * We therefore use the name given by ReadDirectoryChangesW. + * This may be the long form or the 8.3 short name in some cases. + */ + if (!long_filenamew) { + filenamew = file_info->FileName; + sizew = file_info->FileNameLength / sizeof(WCHAR); + } } } else { /* We already have the long name of the file, so just use it. */ diff --git a/test/test-fs-event.c b/test/test-fs-event.c index 725103b7..e02ff2fd 100644 --- a/test/test-fs-event.c +++ b/test/test-fs-event.c @@ -115,7 +115,11 @@ static void fs_event_cb_dir(uv_fs_event_t* handle, const char* filename, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_RENAME); + #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) + ASSERT(strcmp(filename, "file1") == 0); + #else ASSERT(filename == NULL || strcmp(filename, "file1") == 0); + #endif ASSERT(0 == uv_fs_event_stop(handle)); uv_close((uv_handle_t*)handle, close_cb); } @@ -178,8 +182,12 @@ static void fs_event_cb_dir_multi_file(uv_fs_event_t* handle, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_CHANGE || UV_RENAME); + #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) + ASSERT(strncmp(filename, file_prefix, sizeof(file_prefix) - 1) == 0); + #else ASSERT(filename == NULL || strncmp(filename, file_prefix, sizeof(file_prefix) - 1) == 0); + #endif if (fs_event_created + fs_event_removed == fs_event_file_count) { /* Once we've processed all create events, delete all files */ @@ -250,8 +258,16 @@ static void fs_event_cb_dir_multi_file_in_subdir(uv_fs_event_t* handle, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_CHANGE || UV_RENAME); + #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) + ASSERT(strncmp(filename, + file_prefix_in_subdir, + sizeof(file_prefix_in_subdir) - 1) == 0); + #else ASSERT(filename == NULL || - strncmp(filename, file_prefix_in_subdir, sizeof(file_prefix_in_subdir) - 1) == 0); + strncmp(filename, + file_prefix_in_subdir, + sizeof(file_prefix_in_subdir) - 1) == 0); + #endif if (fs_event_created + fs_event_removed == fs_event_file_count) { /* Once we've processed all create events, delete all files */ @@ -270,7 +286,11 @@ static void fs_event_cb_file(uv_fs_event_t* handle, const char* filename, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_CHANGE); + #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) + ASSERT(strcmp(filename, "file2") == 0); + #else ASSERT(filename == NULL || strcmp(filename, "file2") == 0); + #endif ASSERT(0 == uv_fs_event_stop(handle)); uv_close((uv_handle_t*)handle, close_cb); } @@ -293,7 +313,11 @@ static void fs_event_cb_file_current_dir(uv_fs_event_t* handle, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_CHANGE); + #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) + ASSERT(strcmp(filename, "watch_file") == 0); + #else ASSERT(filename == NULL || strcmp(filename, "watch_file") == 0); + #endif /* Regression test for SunOS: touch should generate just one event. */ {