Skip to content

Commit 668c4de

Browse files
committed
fs: handle recursive watch setup races
Arm directory watchers before reading entries to close the setup window. Discard stale watcher state when paths disappear so recreated paths can be watched again. On AIX, treat ENODEV as missing only when stat confirms the path is gone. Preserve root throwIfNoEntry behavior and other watch errors. Fixes: #65698 Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex
1 parent 24c7c30 commit 668c4de

3 files changed

Lines changed: 332 additions & 22 deletions

File tree

lib/internal/fs/recursive_watch.js

Lines changed: 69 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,24 @@ function lazyLoadFsSync() {
3737

3838
let kResistStopPropagation;
3939

40+
const kIsAIX = process.platform === 'aix';
41+
42+
function isPathMissingForWatch(error, file) {
43+
if (error.code === 'ENOENT') {
44+
return true;
45+
}
46+
// AIX reports ENODEV when the path disappears before fs.watch() starts.
47+
// Check that it is actually gone so other ENODEV errors still propagate.
48+
if (!kIsAIX || error.code !== 'ENODEV') {
49+
return false;
50+
}
51+
try {
52+
return lazyLoadFsSync().statSync(file, { throwIfNoEntry: false }) === undefined;
53+
} catch {
54+
return false;
55+
}
56+
}
57+
4058
// Inotify reports changes to a directory's entries, with their names, on the
4159
// directory's own watch, so one watcher per directory is enough on Linux.
4260
// kqueue and event ports only report that the directory itself changed, so
@@ -116,56 +134,81 @@ class FSWatcher extends EventEmitter {
116134
}
117135

118136
#forget(file) {
137+
const watcher = this.#watchers.get(file);
138+
if (watcher !== undefined) {
139+
watcher.close();
140+
this.#watchers.delete(file);
141+
}
142+
this.#entries.delete(file);
143+
this.#symbolicLinks.delete(file);
144+
119145
const childPrefix = file + pathSep;
120146
for (const entry of this.#entries) {
121-
if (entry === file || StringPrototypeStartsWith(entry, childPrefix)) {
147+
if (StringPrototypeStartsWith(entry, childPrefix)) {
122148
this.#entries.delete(entry);
123149
this.#symbolicLinks.delete(entry);
124-
const watcher = this.#watchers.get(entry);
125-
if (watcher !== undefined) {
126-
watcher.close();
150+
const childWatcher = this.#watchers.get(entry);
151+
if (childWatcher !== undefined) {
152+
childWatcher.close();
127153
this.#watchers.delete(entry);
128154
}
129155
}
130156
}
131157
}
132158

133-
// An entry that vanished between being listed and being watched is left to
134-
// the directory's own watcher to report.
159+
// Report an entry that vanished between being listed and being watched so
160+
// the caller can discard its stale bookkeeping.
135161
#watch(file, onChange) {
136-
if (this.#closed || this.#watchers.has(file)) {
137-
return;
162+
if (this.#closed) {
163+
return false;
164+
}
165+
if (this.#watchers.has(file)) {
166+
return true;
138167
}
139168
const { watch } = lazyLoadFsSync();
140169
let watcher;
141170
try {
142171
watcher = watch(file, { persistent: this.#options.persistent }, onChange);
143172
} catch (err) {
144-
if (err.code === 'ENOENT') {
145-
return;
173+
if (isPathMissingForWatch(err, file) &&
174+
(file !== this.#rootPath || !this.#options.throwIfNoEntry)) {
175+
return false;
146176
}
147177
throw err;
148178
}
149179
this.#watchers.set(file, watcher);
180+
return true;
150181
}
151182

152-
// Registers the entries of `folder` that are not known yet (emitting
153-
// 'rename' for them unless this is the initial scan) and arms one watcher
154-
// for the directory; #addEntry() descends into subdirectories.
183+
// Arms one watcher for `folder`, then registers entries that are not known
184+
// yet (emitting 'rename' for them unless this is the initial scan);
185+
// #addEntry() descends into subdirectories.
155186
#scanFolder(folder, initial) {
187+
if (!this.#watch(folder, (eventType, filename) => this.#onFolderEvent(folder, filename))) {
188+
this.#forget(folder);
189+
return;
190+
}
191+
156192
const { readdirSync } = lazyLoadFsSync();
157193
let entries;
158194
try {
159195
entries = readdirSync(folder, { withFileTypes: true });
160196
} catch (error) {
161-
if (error.code !== 'ENOENT') {
197+
if (error.code === 'ENOENT' || error.code === 'ENOTDIR') {
198+
if (folder === this.#rootPath && initial &&
199+
(error.code !== 'ENOENT' || this.#options.throwIfNoEntry)) {
200+
throw error;
201+
}
202+
if (!initial && !this.#closed) {
203+
this.#emit('rename', folder);
204+
}
205+
this.#forget(folder);
206+
} else {
162207
this.emit('error', error);
163208
}
164209
return;
165210
}
166211

167-
this.#watch(folder, (eventType, filename) => this.#onFolderEvent(folder, filename));
168-
169212
for (const entry of entries) {
170213
if (this.#closed) {
171214
break;
@@ -187,11 +230,15 @@ class FSWatcher extends EventEmitter {
187230
// The link target is watched so that changes behind the link surface
188231
// as a 'rename' of the link, as they always have on this code path.
189232
this.#symbolicLinks.add(file);
190-
this.#watch(file, () => this.#emit('rename', file));
233+
if (!this.#watch(file, () => this.#emit('rename', file))) {
234+
this.#forget(file);
235+
}
191236
} else if (entry.isDirectory()) {
192237
this.#scanFolder(file, initial);
193238
} else if (!kDirectoryWatchReportsEntries) {
194-
this.#watch(file, () => this.#onEntryEvent(file));
239+
if (!this.#watch(file, () => this.#onEntryEvent(file))) {
240+
this.#forget(file);
241+
}
195242
}
196243
}
197244

@@ -253,7 +300,7 @@ class FSWatcher extends EventEmitter {
253300
#watchRootFile(file) {
254301
const { statSync } = lazyLoadFsSync();
255302
this.#entries.add(file);
256-
this.#watch(file, () => {
303+
if (!this.#watch(file, () => {
257304
if (this.#closed) {
258305
return;
259306
}
@@ -263,7 +310,9 @@ class FSWatcher extends EventEmitter {
263310
} else {
264311
this.emit('change', 'change', pathBasename(file));
265312
}
266-
});
313+
})) {
314+
this.#forget(file);
315+
}
267316
}
268317

269318
[kFSWatchStart](filename) {

test/parallel/parallel.status

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,6 @@ test-fs-watch-ignore-regexp: SKIP
105105
test-runner-coverage: PASS, FLAKY
106106
# https://github.com/nodejs/node/issues/54346
107107
test-esm-loader-hooks-inspect-wait: PASS, FLAKY
108-
# https://github.com/nodejs/node/issues/65697
109-
fs-watch-recursive-delete-race: SKIP
110108

111109
[$system==ibmi]
112110
# https://github.com/nodejs/node/pull/30819

0 commit comments

Comments
 (0)