Skip to content

Commit e501930

Browse files
deepak1556deepak1556
authored
fix: fs.watch() behavior change in node >= 10.16.0 (electron#20429)
This reverts the patch from electron/node#100 which never got merged due to reasons outlined in libuv/libuv#2313 * Adds new patches that backports libuv/libuv#2459 and libuv/libuv#2460 Based on nodejs/node#29460
1 parent 28eb7b0 commit e501930

File tree

4 files changed

+280
-214
lines changed

4 files changed

+280
-214
lines changed

‎patches/node/.patches

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ feat_add_original-fs_module.patch
3232
build_allow_embedders_to_override_the_node_module_version_define.patch
3333
refactor_allow_embedder_overriding_of_internal_fs_calls.patch
3434
chore_add_ability_to_prevent_warn_non_context-aware_native_modules.patch
35-
fsevents_fix_file_event_reporting.patch
3635
src_only_run_preloadmodules_if_the_preload_array_is_not_empty.patch
3736
src_read_break_node_first_line_from_the_inspect_options.patch
3837
chore_allow_the_node_entrypoint_to_be_a_builtin_module.patch
@@ -55,3 +54,5 @@ chore_read_nobrowserglobals_from_global_not_process.patch
5554
chore_use_v8_inspector_js_protocol_to_find_pdl_file.patch
5655
chore_split_createenvironment_into_createenvironment_and.patch
5756
fix_set_uptime_offset_in_correct_init_method.patch
57+
fsevents-stop-using-fsevents-to-watch-files.patch
58+
fsevents-regression-in-watching.patch

‎patches/node/fsevents-regression-in-watching.patch

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
From ae12376dbb56fa080b699f00840c7b9c5230a85f Mon Sep 17 00:00:00 2001
2+
From: Jameson Nash <vtjnash@gmail.com>
3+
Date: Sat, 7 Sep 2019 20:45:39 -0400
4+
Subject: [PATCH] fsevents: regression in watching /
5+
6+
This case got lost by accident in
7+
https://github.com/libuv/libuv/pull/2082,
8+
preventing the realpath `/` from ever matching.
9+
10+
Fixes: https://github.com/nodejs/node/issues/28917
11+
PR-URL: https://github.com/libuv/libuv/pull/2460
12+
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
13+
Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
14+
15+
diff --git a/deps/uv/src/unix/fsevents.c b/deps/uv/src/unix/fsevents.c
16+
index ddacda31..deeaa63d 100644
17+
--- a/deps/uv/src/unix/fsevents.c
18+
+++ b/deps/uv/src/unix/fsevents.c
19+
@@ -263,10 +263,12 @@ static void uv__fsevents_event_cb(ConstFSEventStreamRef streamRef,
20+
if (len < handle->realpath_len)
21+
continue;
22+
23+
+ /* Make sure that realpath actually named a directory,
24+
+ * (unless watching root, which alone keeps a trailing slash on the realpath)
25+
+ * or that we matched the whole string */
26+
if (handle->realpath_len != len &&
27+
+ handle->realpath_len > 1 &&
28+
path[handle->realpath_len] != '/')
29+
- /* Make sure that realpath actually named a directory,
30+
- * or that we matched the whole string */
31+
continue;
32+
33+
if (memcmp(path, handle->realpath, handle->realpath_len) != 0)
34+
diff --git a/deps/uv/test/test-fs-event.c b/deps/uv/test/test-fs-event.c
35+
index 4b8bb1ef..7725c3af 100644
36+
--- a/deps/uv/test/test-fs-event.c
37+
+++ b/deps/uv/test/test-fs-event.c
38+
@@ -47,6 +47,7 @@ static const char file_prefix[] = "fsevent-";
39+
static const int fs_event_file_count = 16;
40+
#if defined(__APPLE__) || defined(_WIN32)
41+
static const char file_prefix_in_subdir[] = "subdir";
42+
+static int fs_multievent_cb_called;
43+
#endif
44+
static uv_timer_t timer;
45+
static int timer_cb_called;
46+
@@ -280,7 +281,7 @@ static void fs_event_cb_dir_multi_file_in_subdir(uv_fs_event_t* handle,
47+
if (filename && strcmp(filename, file_prefix_in_subdir) == 0)
48+
return;
49+
#endif
50+
- fs_event_cb_called++;
51+
+ fs_multievent_cb_called++;
52+
ASSERT(handle == &fs_event);
53+
ASSERT(status == 0);
54+
ASSERT(events == UV_CHANGE || events == UV_RENAME);
55+
@@ -298,7 +299,7 @@ static void fs_event_cb_dir_multi_file_in_subdir(uv_fs_event_t* handle,
56+
if (fs_event_created + fs_event_removed == fs_event_file_count) {
57+
/* Once we've processed all create events, delete all files */
58+
ASSERT(0 == uv_timer_start(&timer, fs_event_unlink_files_in_subdir, 1, 0));
59+
- } else if (fs_event_cb_called == 2 * fs_event_file_count) {
60+
+ } else if (fs_multievent_cb_called == 2 * fs_event_file_count) {
61+
/* Once we've processed all create and delete events, stop watching */
62+
uv_close((uv_handle_t*) &timer, close_cb);
63+
uv_close((uv_handle_t*) handle, close_cb);
64+
@@ -393,6 +394,21 @@ static void timer_cb_watch_twice(uv_timer_t* handle) {
65+
uv_close((uv_handle_t*) handle, NULL);
66+
}
67+
68+
+static void fs_event_cb_close(uv_fs_event_t* handle,
69+
+ const char* filename,
70+
+ int events,
71+
+ int status) {
72+
+ ASSERT(status == 0);
73+
+
74+
+ ASSERT(fs_event_cb_called < 3);
75+
+ ++fs_event_cb_called;
76+
+
77+
+ if (fs_event_cb_called == 3) {
78+
+ uv_close((uv_handle_t*) handle, close_cb);
79+
+ }
80+
+}
81+
+
82+
+
83+
TEST_IMPL(fs_event_watch_dir) {
84+
#if defined(NO_FS_EVENTS)
85+
RETURN_SKIP(NO_FS_EVENTS);
86+
@@ -434,10 +450,12 @@ TEST_IMPL(fs_event_watch_dir) {
87+
return 0;
88+
}
89+
90+
+
91+
TEST_IMPL(fs_event_watch_dir_recursive) {
92+
#if defined(__APPLE__) || defined(_WIN32)
93+
uv_loop_t* loop;
94+
int r;
95+
+ uv_fs_event_t fs_event_root;
96+
97+
/* Setup */
98+
loop = uv_default_loop();
99+
@@ -451,17 +469,37 @@ TEST_IMPL(fs_event_watch_dir_recursive) {
100+
101+
r = uv_fs_event_init(loop, &fs_event);
102+
ASSERT(r == 0);
103+
- r = uv_fs_event_start(&fs_event, fs_event_cb_dir_multi_file_in_subdir, "watch_dir", UV_FS_EVENT_RECURSIVE);
104+
+ r = uv_fs_event_start(&fs_event,
105+
+ fs_event_cb_dir_multi_file_in_subdir,
106+
+ "watch_dir",
107+
+ UV_FS_EVENT_RECURSIVE);
108+
ASSERT(r == 0);
109+
r = uv_timer_init(loop, &timer);
110+
ASSERT(r == 0);
111+
r = uv_timer_start(&timer, fs_event_create_files_in_subdir, 100, 0);
112+
ASSERT(r == 0);
113+
114+
+#ifndef _WIN32
115+
+ /* Also try to watch the root directory.
116+
+ * This will be noisier, so we're just checking for any couple events to happen. */
117+
+ r = uv_fs_event_init(loop, &fs_event_root);
118+
+ ASSERT(r == 0);
119+
+ r = uv_fs_event_start(&fs_event_root,
120+
+ fs_event_cb_close,
121+
+ "/",
122+
+ UV_FS_EVENT_RECURSIVE);
123+
+ ASSERT(r == 0);
124+
+#else
125+
+ fs_event_cb_called += 3;
126+
+ close_cb_called += 1;
127+
+ (void)fs_event_root;
128+
+#endif
129+
+
130+
uv_run(loop, UV_RUN_DEFAULT);
131+
132+
- ASSERT(fs_event_cb_called == fs_event_created + fs_event_removed);
133+
- ASSERT(close_cb_called == 2);
134+
+ ASSERT(fs_multievent_cb_called == fs_event_created + fs_event_removed);
135+
+ ASSERT(fs_event_cb_called == 3);
136+
+ ASSERT(close_cb_called == 3);
137+
138+
/* Cleanup */
139+
fs_event_unlink_files_in_subdir(NULL);
140+
@@ -870,18 +908,6 @@ TEST_IMPL(fs_event_close_with_pending_event) {
141+
return 0;
142+
}
143+
144+
-static void fs_event_cb_close(uv_fs_event_t* handle, const char* filename,
145+
- int events, int status) {
146+
- ASSERT(status == 0);
147+
-
148+
- ASSERT(fs_event_cb_called < 3);
149+
- ++fs_event_cb_called;
150+
-
151+
- if (fs_event_cb_called == 3) {
152+
- uv_close((uv_handle_t*) handle, close_cb);
153+
- }
154+
-}
155+
-
156+
TEST_IMPL(fs_event_close_in_callback) {
157+
#if defined(NO_FS_EVENTS)
158+
RETURN_SKIP(NO_FS_EVENTS);

‎patches/node/fsevents-stop-using-fsevents-to-watch-files.patch

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
From 97b85e8b75b8f3df774b6e008dbaa143daa412b7 Mon Sep 17 00:00:00 2001
2+
From: Jameson Nash <vtjnash@gmail.com>
3+
Date: Sat, 7 Sep 2019 14:55:40 -0400
4+
Subject: [PATCH] fsevents: stop using fsevents to watch files
5+
6+
Goes back to just using it to watch folders,
7+
but keeps the other logic changes around.
8+
9+
Refs: https://github.com/libuv/libuv/pull/387
10+
Refs: https://github.com/libuv/libuv/pull/2082
11+
Refs: https://github.com/libuv/libuv/pull/1572
12+
Refs: https://github.com/nodejs/node/issues/29460
13+
Fixes: https://github.com/libuv/libuv/issues/2488
14+
Closes: https://github.com/libuv/libuv/pull/2452
15+
PR-URL: https://github.com/libuv/libuv/pull/2459
16+
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
17+
Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
18+
19+
diff --git a/deps/uv/src/unix/kqueue.c b/deps/uv/src/unix/kqueue.c
20+
index c04e7a48..ad09f403 100644
21+
--- a/deps/uv/src/unix/kqueue.c
22+
+++ b/deps/uv/src/unix/kqueue.c
23+
@@ -454,10 +454,26 @@ int uv_fs_event_start(uv_fs_event_t* handle,
24+
const char* path,
25+
unsigned int flags) {
26+
int fd;
27+
+#if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
28+
+ struct stat statbuf;
29+
+#endif
30+
31+
if (uv__is_active(handle))
32+
return UV_EINVAL;
33+
34+
+ handle->cb = cb;
35+
+ handle->path = uv__strdup(path);
36+
+ if (handle->path == NULL)
37+
+ return UV_ENOMEM;
38+
+
39+
+ /* TODO open asynchronously - but how do we report back errors? */
40+
+ fd = open(handle->path, O_RDONLY);
41+
+ if (fd == -1) {
42+
+ uv__free(handle->path);
43+
+ handle->path = NULL;
44+
+ return UV__ERR(errno);
45+
+ }
46+
+
47+
#if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
48+
/* Nullify field to perform checks later */
49+
handle->cf_cb = NULL;
50+
@@ -465,14 +481,17 @@ int uv_fs_event_start(uv_fs_event_t* handle,
51+
handle->realpath_len = 0;
52+
handle->cf_flags = flags;
53+
54+
+ if (fstat(fd, &statbuf))
55+
+ goto fallback;
56+
+ /* FSEvents works only with directories */
57+
+ if (!(statbuf.st_mode & S_IFDIR))
58+
+ goto fallback;
59+
+
60+
if (!uv__has_forked_with_cfrunloop) {
61+
int r;
62+
- /* The fallback fd is not used */
63+
+ /* The fallback fd is no longer needed */
64+
+ uv__close_nocheckstdio(fd);
65+
handle->event_watcher.fd = -1;
66+
- handle->path = uv__strdup(path);
67+
- if (handle->path == NULL)
68+
- return UV_ENOMEM;
69+
- handle->cb = cb;
70+
r = uv__fsevents_init(handle);
71+
if (r == 0) {
72+
uv__handle_start(handle);
73+
@@ -482,20 +501,9 @@ int uv_fs_event_start(uv_fs_event_t* handle,
74+
}
75+
return r;
76+
}
77+
+fallback:
78+
#endif /* #if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
79+
80+
- /* TODO open asynchronously - but how do we report back errors? */
81+
- fd = open(path, O_RDONLY);
82+
- if (fd == -1)
83+
- return UV__ERR(errno);
84+
-
85+
- handle->path = uv__strdup(path);
86+
- if (handle->path == NULL) {
87+
- uv__close_nocheckstdio(fd);
88+
- return UV_ENOMEM;
89+
- }
90+
-
91+
- handle->cb = cb;
92+
uv__handle_start(handle);
93+
uv__io_init(&handle->event_watcher, uv__fs_event, fd);
94+
uv__io_start(handle->loop, &handle->event_watcher, POLLIN);
95+
@@ -514,7 +522,7 @@ int uv_fs_event_stop(uv_fs_event_t* handle) {
96+
uv__handle_stop(handle);
97+
98+
#if defined(__APPLE__) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
99+
- if (!uv__has_forked_with_cfrunloop)
100+
+ if (!uv__has_forked_with_cfrunloop && handle->cf_cb != NULL)
101+
r = uv__fsevents_close(handle);
102+
#endif
103+
104+
diff --git a/deps/uv/test/test-fs-event.c b/deps/uv/test/test-fs-event.c
105+
index ea34bd63..4b8bb1ef 100644
106+
--- a/deps/uv/test/test-fs-event.c
107+
+++ b/deps/uv/test/test-fs-event.c
108+
@@ -656,6 +656,12 @@ TEST_IMPL(fs_event_watch_file_current_dir) {
109+
/* Setup */
110+
remove("watch_file");
111+
create_file("watch_file");
112+
+#if defined(__APPLE__) && !defined(MAC_OS_X_VERSION_10_12)
113+
+ /* Empirically, kevent seems to (sometimes) report the preceeding
114+
+ * create_file events prior to macOS 10.11.6 in the subsequent fs_event_start
115+
+ * So let the system settle before running the test. */
116+
+ uv_sleep(1100);
117+
+#endif
118+
119+
r = uv_fs_event_init(loop, &fs_event);
120+
ASSERT(r == 0);

0 commit comments

Comments
 (0)