fs: handle recursive watch setup races · nodejs/node@1dfb047 · GitHub
Skip to content

Commit 1dfb047

Browse files
panvaaduh95
authored andcommitted
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: #65697 Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex PR-URL: #65699 Fixes: #65697 Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent bc431c8 commit 1dfb047

9 files changed

Lines changed: 407 additions & 22 deletions

‎lib/internal/fs/recursive_watch.js‎

Lines changed: 69 additions & 20 deletions

‎test/parallel/parallel.status‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ test-fs-watch-ignore-regexp: SKIP
114114
[$system==aix]
115115
# https://github.com/nodejs/node/issues/54346
116116
test-esm-loader-hooks-inspect-wait: PASS, FLAKY
117-
# https://github.com/nodejs/node/issues/65697
118-
test-fs-watch-recursive-delete-race: SKIP
119117

120118
[$system==ibmi]
121119
# https://github.com/nodejs/node/pull/30819
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
const common = require('../common');
5+
6+
if (!common.isAIX)
7+
common.skip('AIX-specific ENODEV handling');
8+
9+
const assert = require('assert');
10+
const fs = require('fs');
11+
const tmpdir = require('../common/tmpdir');
12+
const { kFSWatchStart } = require('internal/fs/watchers');
13+
const { FSWatcher } = require('internal/fs/recursive_watch');
14+
15+
tmpdir.refresh();
16+
17+
const directory = tmpdir.resolve('preserve-watch-error');
18+
const originalStatSync = fs.statSync;
19+
const originalWatch = fs.watch;
20+
const expected = new Error('watch failed');
21+
const verificationError = new Error('stat failed');
22+
const watcher = new FSWatcher({ recursive: true, throwIfNoEntry: false });
23+
const failVerification = common.mustCall(() => {
24+
throw verificationError;
25+
});
26+
27+
expected.code = 'ENODEV';
28+
verificationError.code = 'EIO';
29+
fs.mkdirSync(directory);
30+
fs.watch = common.mustCall((filename) => {
31+
assert.strictEqual(filename, directory);
32+
throw expected;
33+
});
34+
fs.statSync = (filename, options) => {
35+
if (filename === directory && options?.throwIfNoEntry === false) {
36+
return failVerification();
37+
}
38+
return Reflect.apply(originalStatSync, fs, [filename, options]);
39+
};
40+
41+
try {
42+
assert.throws(
43+
() => watcher[kFSWatchStart](directory),
44+
(error) => {
45+
assert.strictEqual(error, expected);
46+
return true;
47+
},
48+
);
49+
} finally {
50+
watcher.close();
51+
fs.statSync = originalStatSync;
52+
fs.watch = originalWatch;
53+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('assert');
6+
const fs = require('fs');
7+
const path = require('path');
8+
const tmpdir = require('../common/tmpdir');
9+
const { kFSWatchStart } = require('internal/fs/watchers');
10+
const { FSWatcher } = require('internal/fs/recursive_watch');
11+
12+
function createWatcher() {
13+
return { close() {} };
14+
}
15+
16+
tmpdir.refresh();
17+
18+
const directory = tmpdir.resolve('retry-directory-disappears-before-read');
19+
const child = path.join(directory, 'child');
20+
const originalWatch = fs.watch;
21+
const watcher = new FSWatcher({ recursive: true });
22+
let childWatchCalls = 0;
23+
let directoryListener;
24+
let staleWatcherCloseCalls = 0;
25+
26+
fs.mkdirSync(child, { recursive: true });
27+
fs.watch = common.mustCall((filename, options, listener) => {
28+
if (filename === directory) {
29+
directoryListener = listener;
30+
return createWatcher();
31+
}
32+
33+
assert.strictEqual(filename, child);
34+
childWatchCalls++;
35+
if (childWatchCalls === 1) {
36+
fs.rmSync(child, { recursive: true });
37+
return { close: () => staleWatcherCloseCalls++ };
38+
}
39+
return createWatcher();
40+
}, 3);
41+
42+
try {
43+
watcher[kFSWatchStart](directory);
44+
assert.strictEqual(childWatchCalls, 1);
45+
assert.strictEqual(staleWatcherCloseCalls, 1);
46+
47+
fs.mkdirSync(child);
48+
directoryListener('rename', null);
49+
50+
assert.strictEqual(childWatchCalls, 2);
51+
} finally {
52+
watcher.close();
53+
fs.watch = originalWatch;
54+
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
const common = require('../common');
5+
6+
if (common.isIBMi)
7+
common.skip('IBMi does not support `fs.watch()`');
8+
9+
const assert = require('assert');
10+
const fs = require('fs');
11+
const path = require('path');
12+
const tmpdir = require('../common/tmpdir');
13+
const { kFSWatchStart } = require('internal/fs/watchers');
14+
const { FSWatcher } = require('internal/fs/recursive_watch');
15+
16+
function createWatcher() {
17+
return { close() {} };
18+
}
19+
20+
tmpdir.refresh();
21+
22+
const directory = tmpdir.resolve('retry-directory-disappears-before-watch');
23+
const child = path.join(directory, 'child');
24+
const originalWatch = fs.watch;
25+
const changes = [];
26+
const watcher = new FSWatcher({ recursive: true });
27+
let childWatchCalls = 0;
28+
let directoryListener;
29+
let watchError;
30+
31+
fs.mkdirSync(child, { recursive: true });
32+
watcher.on('change', (eventType, filename) => changes.push([eventType, filename]));
33+
fs.watch = common.mustCall((filename, options, listener) => {
34+
if (filename === directory) {
35+
directoryListener = listener;
36+
return createWatcher();
37+
}
38+
39+
assert.strictEqual(filename, child);
40+
childWatchCalls++;
41+
if (childWatchCalls === 1) {
42+
fs.rmSync(child, { recursive: true });
43+
try {
44+
return Reflect.apply(originalWatch, fs, [filename, options, listener]);
45+
} catch (error) {
46+
watchError = error;
47+
throw error;
48+
}
49+
}
50+
return createWatcher();
51+
}, 3);
52+
53+
try {
54+
watcher[kFSWatchStart](directory);
55+
assert.strictEqual(childWatchCalls, 1);
56+
assert.match(watchError?.code, /^(ENOENT|ENODEV)$/);
57+
58+
fs.mkdirSync(child);
59+
directoryListener('rename', null);
60+
61+
assert.strictEqual(childWatchCalls, 2);
62+
assert.deepStrictEqual(changes, [['rename', 'child']]);
63+
} finally {
64+
watcher.close();
65+
fs.watch = originalWatch;
66+
}
Lines changed: 37 additions & 0 deletions

0 commit comments

Comments
 (0)