mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: fs.watch() behavior change in node >= 10.16.0 (#20408)

Browse source 
This reverts the patch from https://github.com/electron/node/pull/100
which never got merged due to reasons outlined in https://github.com/libuv/libuv/pull/2313

* Adds new patches that backports https://github.com/libuv/libuv/pull/2459
  and https://github.com/libuv/libuv/pull/2460

Based on https://github.com/nodejs/node/issues/29460
This commit is contained in:
Robo 2019-10-07 13:04:15 -07:00 committed by GitHub
parent 0c87471c12
commit c85648a8a1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 280 additions and 214 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
patches/node/.patches
View file

@ -26,7 +26,6 @@ src_disable_node_use_v8_platform_in_node_options.patch
build_modify_js2c_py_to_allow_injection_of_original-fs_and_custom_embedder_js.patch
refactor_allow_embedder_overriding_of_internal_fs_calls.patch
chore_add_ability_to_prevent_warn_non_context-aware_native_modules.patch
fsevents_fix_file_event_reporting.patch
chore_allow_the_node_entrypoint_to_be_a_builtin_module.patch
inherit_electron_crashpad_pipe_name_in_child_process.patch
fixme_revert_crypto_add_support_for_rsa-pss_keys.patch
@ -39,4 +38,6 @@ chore_handle_default_configuration_not_being_set_in_the_electron_env.patch
revert_crypto_add_outputlength_option_to_crypto_createhash.patch
add_openssl_is_boringssl_guard_to_oaep_hash_check.patch
fix_microtasks.patch
fsevents-stop-using-fsevents-to-watch-files.patch
fsevents-regression-in-watching.patch
fix_enable_worker_threads.patch

158
patches/node/fsevents-regression-in-watching.patch Normal file
View file

@ -0,0 +1,158 @@
From ae12376dbb56fa080b699f00840c7b9c5230a85f Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash@gmail.com>
Date: Sat, 7 Sep 2019 20:45:39 -0400
Subject: [PATCH] fsevents: regression in watching /
This case got lost by accident in
https://github.com/libuv/libuv/pull/2082,
preventing the realpath `/` from ever matching.
Fixes: https://github.com/nodejs/node/issues/28917
PR-URL: https://github.com/libuv/libuv/pull/2460
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
diff --git a/deps/uv/src/unix/fsevents.c b/deps/uv/src/unix/fsevents.c
index ddacda31..deeaa63d 100644
--- a/deps/uv/src/unix/fsevents.c
+++ b/deps/uv/src/unix/fsevents.c
@@ -263,10 +263,12 @@ static void uv__fsevents_event_cb(ConstFSEventStreamRef streamRef,
if (len < handle->realpath_len)
continue;
+ /* Make sure that realpath actually named a directory,
+ * (unless watching root, which alone keeps a trailing slash on the realpath)
+ * or that we matched the whole string */
if (handle->realpath_len != len &&
+ handle->realpath_len > 1 &&
path[handle->realpath_len] != '/')
- /* Make sure that realpath actually named a directory,
- * or that we matched the whole string */
continue;
if (memcmp(path, handle->realpath, handle->realpath_len) != 0)
diff --git a/deps/uv/test/test-fs-event.c b/deps/uv/test/test-fs-event.c
index 4b8bb1ef..7725c3af 100644
--- a/deps/uv/test/test-fs-event.c
+++ b/deps/uv/test/test-fs-event.c
@@ -47,6 +47,7 @@ static const char file_prefix[] = "fsevent-";
static const int fs_event_file_count = 16;
#if defined(__APPLE__) || defined(_WIN32)
static const char file_prefix_in_subdir[] = "subdir";
+static int fs_multievent_cb_called;
#endif
static uv_timer_t timer;
static int timer_cb_called;
@@ -280,7 +281,7 @@ static void fs_event_cb_dir_multi_file_in_subdir(uv_fs_event_t* handle,
if (filename && strcmp(filename, file_prefix_in_subdir) == 0)
return;
#endif
- fs_event_cb_called++;
+ fs_multievent_cb_called++;
ASSERT(handle == &fs_event);
ASSERT(status == 0);
ASSERT(events == UV_CHANGE || events == UV_RENAME);
@@ -298,7 +299,7 @@ static void fs_event_cb_dir_multi_file_in_subdir(uv_fs_event_t* handle,
if (fs_event_created + fs_event_removed == fs_event_file_count) {
/* Once we've processed all create events, delete all files */
ASSERT(0 == uv_timer_start(&timer, fs_event_unlink_files_in_subdir, 1, 0));
- } else if (fs_event_cb_called == 2 * fs_event_file_count) {
+ } else if (fs_multievent_cb_called == 2 * fs_event_file_count) {
/* Once we've processed all create and delete events, stop watching */
uv_close((uv_handle_t*) &timer, close_cb);
uv_close((uv_handle_t*) handle, close_cb);
@@ -393,6 +394,21 @@ static void timer_cb_watch_twice(uv_timer_t* handle) {
uv_close((uv_handle_t*) handle, NULL);
}
+static void fs_event_cb_close(uv_fs_event_t* handle,
+ const char* filename,
+ int events,
+ int status) {
+ ASSERT(status == 0);
+
+ ASSERT(fs_event_cb_called < 3);
+ ++fs_event_cb_called;
+
+ if (fs_event_cb_called == 3) {
+ uv_close((uv_handle_t*) handle, close_cb);
+ }
+}
+
+
TEST_IMPL(fs_event_watch_dir) {
#if defined(NO_FS_EVENTS)
RETURN_SKIP(NO_FS_EVENTS);
@@ -434,10 +450,12 @@ TEST_IMPL(fs_event_watch_dir) {
return 0;
}
+
TEST_IMPL(fs_event_watch_dir_recursive) {
#if defined(__APPLE__) || defined(_WIN32)
uv_loop_t* loop;
int r;
+ uv_fs_event_t fs_event_root;
/* Setup */
loop = uv_default_loop();
@@ -451,17 +469,37 @@ TEST_IMPL(fs_event_watch_dir_recursive) {
r = uv_fs_event_init(loop, &fs_event);
ASSERT(r == 0);
- r = uv_fs_event_start(&fs_event, fs_event_cb_dir_multi_file_in_subdir, "watch_dir", UV_FS_EVENT_RECURSIVE);
+ r = uv_fs_event_start(&fs_event,
+ fs_event_cb_dir_multi_file_in_subdir,
+ "watch_dir",
+ UV_FS_EVENT_RECURSIVE);
ASSERT(r == 0);
r = uv_timer_init(loop, &timer);
ASSERT(r == 0);
r = uv_timer_start(&timer, fs_event_create_files_in_subdir, 100, 0);
ASSERT(r == 0);
+#ifndef _WIN32
+ /* Also try to watch the root directory.
+ * This will be noisier, so we're just checking for any couple events to happen. */
+ r = uv_fs_event_init(loop, &fs_event_root);
+ ASSERT(r == 0);
+ r = uv_fs_event_start(&fs_event_root,
+ fs_event_cb_close,
+ "/",
+ UV_FS_EVENT_RECURSIVE);
+ ASSERT(r == 0);
+#else
+ fs_event_cb_called += 3;
+ close_cb_called += 1;
+ (void)fs_event_root;
+#endif
+
uv_run(loop, UV_RUN_DEFAULT);
- ASSERT(fs_event_cb_called == fs_event_created + fs_event_removed);
- ASSERT(close_cb_called == 2);
+ ASSERT(fs_multievent_cb_called == fs_event_created + fs_event_removed);
+ ASSERT(fs_event_cb_called == 3);
+ ASSERT(close_cb_called == 3);
/* Cleanup */
fs_event_unlink_files_in_subdir(NULL);
@@ -870,18 +908,6 @@ TEST_IMPL(fs_event_close_with_pending_event) {
return 0;
}
-static void fs_event_cb_close(uv_fs_event_t* handle, const char* filename,
- int events, int status) {
- ASSERT(status == 0);
-
- ASSERT(fs_event_cb_called < 3);
- ++fs_event_cb_called;
-
- if (fs_event_cb_called == 3) {
- uv_close((uv_handle_t*) handle, close_cb);
- }
-}
-
TEST_IMPL(fs_event_close_in_callback) {
#if defined(NO_FS_EVENTS)
RETURN_SKIP(NO_FS_EVENTS);

120
patches/node/fsevents-stop-using-fsevents-to-watch-files.patch Normal file
View file

@ -0,0 +1,120 @@
From 97b85e8b75b8f3df774b6e008dbaa143daa412b7 Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash@gmail.com>
Date: Sat, 7 Sep 2019 14:55:40 -0400
Subject: [PATCH] fsevents: stop using fsevents to watch files
Goes back to just using it to watch folders,
but keeps the other logic changes around.
Refs: https://github.com/libuv/libuv/pull/387
Refs: https://github.com/libuv/libuv/pull/2082
Refs: https://github.com/libuv/libuv/pull/1572
Refs: https://github.com/nodejs/node/issues/29460
Fixes: https://github.com/libuv/libuv/issues/2488
Closes: https://github.com/libuv/libuv/pull/2452
PR-URL: https://github.com/libuv/libuv/pull/2459
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
diff --git a/deps/uv/src/unix/kqueue.c b/deps/uv/src/unix/kqueue.c
index c04e7a48..ad09f403 100644
--- a/deps/uv/src/unix/kqueue.c
+++ b/deps/uv/src/unix/kqueue.c
@@ -454,10 +454,26 @@ int uv_fs_event_start(uv_fs_event_t* handle,
const char* path,
unsigned int flags) {
int fd;
+#if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+ struct stat statbuf;
+#endif
if (uv__is_active(handle))
return UV_EINVAL;
+ handle->cb = cb;
+ handle->path = uv__strdup(path);
+ if (handle->path == NULL)
+ return UV_ENOMEM;
+
+ /* TODO open asynchronously - but how do we report back errors? */
+ fd = open(handle->path, O_RDONLY);
+ if (fd == -1) {
+ uv__free(handle->path);
+ handle->path = NULL;
+ return UV__ERR(errno);
+ }
+
#if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
/* Nullify field to perform checks later */
handle->cf_cb = NULL;
@@ -465,14 +481,17 @@ int uv_fs_event_start(uv_fs_event_t* handle,
handle->realpath_len = 0;
handle->cf_flags = flags;
+ if (fstat(fd, &statbuf))
+ goto fallback;
+ /* FSEvents works only with directories */
+ if (!(statbuf.st_mode & S_IFDIR))
+ goto fallback;
+
if (!uv__has_forked_with_cfrunloop) {
int r;
- /* The fallback fd is not used */
+ /* The fallback fd is no longer needed */
+ uv__close_nocheckstdio(fd);
handle->event_watcher.fd = -1;
- handle->path = uv__strdup(path);
- if (handle->path == NULL)
- return UV_ENOMEM;
- handle->cb = cb;
r = uv__fsevents_init(handle);
if (r == 0) {
uv__handle_start(handle);
@@ -482,20 +501,9 @@ int uv_fs_event_start(uv_fs_event_t* handle,
}
return r;
}
+fallback:
#endif /* #if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
- /* TODO open asynchronously - but how do we report back errors? */
- fd = open(path, O_RDONLY);
- if (fd == -1)
- return UV__ERR(errno);
-
- handle->path = uv__strdup(path);
- if (handle->path == NULL) {
- uv__close_nocheckstdio(fd);
- return UV_ENOMEM;
- }
-
- handle->cb = cb;
uv__handle_start(handle);
uv__io_init(&handle->event_watcher, uv__fs_event, fd);
uv__io_start(handle->loop, &handle->event_watcher, POLLIN);
@@ -514,7 +522,7 @@ int uv_fs_event_stop(uv_fs_event_t* handle) {
uv__handle_stop(handle);
#if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
- if (!uv__has_forked_with_cfrunloop)
+ if (!uv__has_forked_with_cfrunloop && handle->cf_cb != NULL)
r = uv__fsevents_close(handle);
#endif
diff --git a/deps/uv/test/test-fs-event.c b/deps/uv/test/test-fs-event.c
index ea34bd63..4b8bb1ef 100644
--- a/deps/uv/test/test-fs-event.c
+++ b/deps/uv/test/test-fs-event.c
@@ -656,6 +656,12 @@ TEST_IMPL(fs_event_watch_file_current_dir) {
/* Setup */
remove("watch_file");
create_file("watch_file");
+#if defined(__APPLE__) && !defined(MAC_OS_X_VERSION_10_12)
+ /* Empirically, kevent seems to (sometimes) report the preceeding
+ * create_file events prior to macOS 10.11.6 in the subsequent fs_event_start
+ * So let the system settle before running the test. */
+ uv_sleep(1100);
+#endif
r = uv_fs_event_init(loop, &fs_event);
ASSERT(r == 0);

213
patches/node/fsevents_fix_file_event_reporting.patch
View file

@ -1,213 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Mon, 27 May 2019 14:31:53 +0200
Subject: fsevents: fix file event reporting
Commit 2d2af38 ("fsevents: really watch files with fsevents on macos
10.7+") from last November introduced a regression where events that
were previously reported as UV_RENAME were now reported as UV_CHANGE.
This commit rectifies that.
Fixes: https://github.com/nodejs/node/issues/27869
diff --git a/deps/uv/src/unix/fsevents.c b/deps/uv/src/unix/fsevents.c
index ddacda31fef87eee131fc2ee2ff46cc88be429d9..911023a6562319af5e9e3412ddbfd2ca14f77b05 100644
--- a/deps/uv/src/unix/fsevents.c
+++ b/deps/uv/src/unix/fsevents.c
@@ -49,19 +49,9 @@ void uv__fsevents_loop_delete(uv_loop_t* loop) {
#include <CoreFoundation/CFRunLoop.h>
#include <CoreServices/CoreServices.h>
-/* These are macros to avoid "initializer element is not constant" errors
+/* Macro to avoid "initializer element is not constant" errors
* with old versions of gcc.
*/
-#define kFSEventsModified (kFSEventStreamEventFlagItemFinderInfoMod | \
- kFSEventStreamEventFlagItemModified | \
- kFSEventStreamEventFlagItemInodeMetaMod | \
- kFSEventStreamEventFlagItemChangeOwner | \
- kFSEventStreamEventFlagItemXattrMod)
-
-#define kFSEventsRenamed (kFSEventStreamEventFlagItemCreated | \
- kFSEventStreamEventFlagItemRemoved | \
- kFSEventStreamEventFlagItemRenamed)
-
#define kFSEventsSystem (kFSEventStreamEventFlagUserDropped | \
kFSEventStreamEventFlagKernelDropped | \
kFSEventStreamEventFlagEventIdsWrapped | \
@@ -289,8 +279,6 @@ static void uv__fsevents_event_cb(ConstFSEventStreamRef streamRef,
path--;
len++;
}
- /* Created and Removed seem to be always set, but don't make sense */
- flags &= ~kFSEventsRenamed;
} else {
/* Skip forward slash */
path++;
@@ -311,12 +299,12 @@ static void uv__fsevents_event_cb(ConstFSEventStreamRef streamRef,
memset(event, 0, sizeof(*event));
memcpy(event->path, path, len + 1);
- event->events = UV_RENAME;
+ event->events = UV_CHANGE;
- if (0 == (flags & kFSEventsRenamed)) {
- if (0 != (flags & kFSEventsModified) ||
- 0 == (flags & kFSEventStreamEventFlagItemIsDir))
- event->events = UV_CHANGE;
+ if ((flags & kFSEventStreamEventFlagItemIsDir) ||
+ (flags & kFSEventStreamEventFlagItemRemoved) ||
+ (flags & kFSEventStreamEventFlagItemRenamed)) {
+ event->events = UV_RENAME;
}
QUEUE_INSERT_TAIL(&head, &event->member);
diff --git a/deps/uv/test/test-fs-event.c b/deps/uv/test/test-fs-event.c
index ea34bd63a70625c3e2c60d5a1bbb087c5f0bbb2e..38d722a27ef1a78717726272d54578c734280242 100644
--- a/deps/uv/test/test-fs-event.c
+++ b/deps/uv/test/test-fs-event.c
@@ -62,6 +62,15 @@ static char fs_event_filename[1024];
static int timer_cb_touch_called;
static int timer_cb_exact_called;
+static void expect_filename(const char* path, const char* expected) {
+#if defined(__APPLE__) || defined(_WIN32) || defined(__linux__)
+ ASSERT(0 == strcmp(path, expected));
+#else
+ if (path != NULL)
+ ASSERT(0 == strcmp(path, expected));
+#endif
+}
+
static void fs_event_fail(uv_fs_event_t* handle,
const char* filename,
int events,
@@ -130,11 +139,7 @@ static void fs_event_cb_dir(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, "file1") == 0);
- #else
- ASSERT(filename == NULL || strcmp(filename, "file1") == 0);
- #endif
+ expect_filename(filename, "file1");
ASSERT(0 == uv_fs_event_stop(handle));
uv_close((uv_handle_t*)handle, close_cb);
}
@@ -312,11 +317,7 @@ 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
+ expect_filename(filename, "file2");
ASSERT(0 == uv_fs_event_stop(handle));
uv_close((uv_handle_t*)handle, close_cb);
}
@@ -339,11 +340,7 @@ 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
+ expect_filename(filename, "watch_file");
/* Regression test for SunOS: touch should generate just one event. */
{
@@ -619,6 +616,69 @@ TEST_IMPL(fs_event_watch_file_exact_path) {
return 0;
}
+static void file_remove_cb(uv_fs_event_t* handle,
+ const char* path,
+ int events,
+ int status) {
+ fs_event_cb_called++;
+
+ expect_filename(path, "file1");
+ /* TODO(bnoordhuis) Harmonize the behavior across platforms. Right now
+ * this test merely ensures the status quo doesn't regress.
+ */
+#if defined(_AIX) || defined(__linux__)
+ ASSERT(UV_CHANGE == events);
+#else
+ ASSERT(UV_RENAME == events);
+#endif
+ ASSERT(0 == status);
+
+ uv_close((uv_handle_t*) handle, NULL);
+}
+
+static void file_remove_next(uv_timer_t* handle) {
+ uv_close((uv_handle_t*) handle, NULL);
+ remove("watch_dir/file1");
+}
+
+static void file_remove_start(uv_timer_t* handle) {
+ uv_fs_event_t* watcher;
+ uv_loop_t* loop;
+
+ loop = handle->loop;
+ watcher = handle->data;
+
+ ASSERT(0 == uv_fs_event_init(loop, watcher));
+ ASSERT(0 == uv_fs_event_start(watcher, file_remove_cb, "watch_dir/file1", 0));
+ ASSERT(0 == uv_timer_start(handle, file_remove_next, 50, 0));
+}
+
+TEST_IMPL(fs_event_watch_file_remove) {
+ uv_fs_event_t watcher;
+ uv_timer_t timer;
+ uv_loop_t* loop;
+
+#if defined(__MVS__)
+ RETURN_SKIP("test does not work on this OS");
+#endif
+
+ remove("watch_dir/file1");
+ remove("watch_dir/");
+ create_dir("watch_dir");
+ create_file("watch_dir/file1");
+
+ loop = uv_default_loop();
+ timer.data = &watcher;
+
+ ASSERT(0 == uv_timer_init(loop, &timer));
+ ASSERT(0 == uv_timer_start(&timer, file_remove_start, 500, 0));
+ ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT));
+ ASSERT(1 == fs_event_cb_called);
+
+ MAKE_VALGRIND_HAPPY();
+ return 0;
+}
+
TEST_IMPL(fs_event_watch_file_twice) {
#if defined(NO_FS_EVENTS)
RETURN_SKIP(NO_FS_EVENTS);
diff --git a/deps/uv/test/test-list.h b/deps/uv/test/test-list.h
index a48f6f3806806af6bf844558d08fa9679e268112..f9020fc6c1ec8d076c470fa5042d062b74226ae4 100644
--- a/deps/uv/test/test-list.h
+++ b/deps/uv/test/test-list.h
@@ -335,6 +335,7 @@ TEST_DECLARE (fs_event_watch_dir_short_path)
#endif
TEST_DECLARE (fs_event_watch_file)
TEST_DECLARE (fs_event_watch_file_exact_path)
+TEST_DECLARE (fs_event_watch_file_remove)
TEST_DECLARE (fs_event_watch_file_twice)
TEST_DECLARE (fs_event_watch_file_current_dir)
#ifdef _WIN32
@@ -924,6 +925,7 @@ TASK_LIST_START
#endif
TEST_ENTRY (fs_event_watch_file)
TEST_ENTRY (fs_event_watch_file_exact_path)
+ TEST_ENTRY (fs_event_watch_file_remove)
TEST_ENTRY (fs_event_watch_file_twice)
TEST_ENTRY (fs_event_watch_file_current_dir)
#ifdef _WIN32