diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index c01f1807cc8f..85d84f9dd8f3 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -37,6 +37,24 @@ function lazyLoadFsSync() { let kResistStopPropagation; +const kIsAIX = process.platform === 'aix'; + +function isPathMissingForWatch(error, file) { + if (error.code === 'ENOENT') { + return true; + } + // AIX reports ENODEV when the path disappears before fs.watch() starts. + // Check that it is actually gone so other ENODEV errors still propagate. + if (!kIsAIX || error.code !== 'ENODEV') { + return false; + } + try { + return lazyLoadFsSync().statSync(file, { throwIfNoEntry: false }) === undefined; + } catch { + return false; + } +} + // Inotify reports changes to a directory's entries, with their names, on the // directory's own watch, so one watcher per directory is enough on Linux. // kqueue and event ports only report that the directory itself changed, so @@ -116,56 +134,81 @@ class FSWatcher extends EventEmitter { } #forget(file) { + const watcher = this.#watchers.get(file); + if (watcher !== undefined) { + watcher.close(); + this.#watchers.delete(file); + } + this.#entries.delete(file); + this.#symbolicLinks.delete(file); + const childPrefix = file + pathSep; for (const entry of this.#entries) { - if (entry === file || StringPrototypeStartsWith(entry, childPrefix)) { + if (StringPrototypeStartsWith(entry, childPrefix)) { this.#entries.delete(entry); this.#symbolicLinks.delete(entry); - const watcher = this.#watchers.get(entry); - if (watcher !== undefined) { - watcher.close(); + const childWatcher = this.#watchers.get(entry); + if (childWatcher !== undefined) { + childWatcher.close(); this.#watchers.delete(entry); } } } } - // An entry that vanished between being listed and being watched is left to - // the directory's own watcher to report. + // Report an entry that vanished between being listed and being watched so + // the caller can discard its stale bookkeeping. #watch(file, onChange) { - if (this.#closed || this.#watchers.has(file)) { - return; + if (this.#closed) { + return false; + } + if (this.#watchers.has(file)) { + return true; } const { watch } = lazyLoadFsSync(); let watcher; try { watcher = watch(file, { persistent: this.#options.persistent }, onChange); } catch (err) { - if (err.code === 'ENOENT') { - return; + if (isPathMissingForWatch(err, file) && + (file !== this.#rootPath || !this.#options.throwIfNoEntry)) { + return false; } throw err; } this.#watchers.set(file, watcher); + return true; } - // Registers the entries of `folder` that are not known yet (emitting - // 'rename' for them unless this is the initial scan) and arms one watcher - // for the directory; #addEntry() descends into subdirectories. + // Arms one watcher for `folder`, then registers entries that are not known + // yet (emitting 'rename' for them unless this is the initial scan); + // #addEntry() descends into subdirectories. #scanFolder(folder, initial) { + if (!this.#watch(folder, (eventType, filename) => this.#onFolderEvent(folder, filename))) { + this.#forget(folder); + return; + } + const { readdirSync } = lazyLoadFsSync(); let entries; try { entries = readdirSync(folder, { withFileTypes: true }); } catch (error) { - if (error.code !== 'ENOENT') { + if (error.code === 'ENOENT' || error.code === 'ENOTDIR') { + if (folder === this.#rootPath && initial && + (error.code !== 'ENOENT' || this.#options.throwIfNoEntry)) { + throw error; + } + if (!initial && !this.#closed) { + this.#emit('rename', folder); + } + this.#forget(folder); + } else { this.emit('error', error); } return; } - this.#watch(folder, (eventType, filename) => this.#onFolderEvent(folder, filename)); - for (const entry of entries) { if (this.#closed) { break; @@ -187,11 +230,15 @@ class FSWatcher extends EventEmitter { // The link target is watched so that changes behind the link surface // as a 'rename' of the link, as they always have on this code path. this.#symbolicLinks.add(file); - this.#watch(file, () => this.#emit('rename', file)); + if (!this.#watch(file, () => this.#emit('rename', file))) { + this.#forget(file); + } } else if (entry.isDirectory()) { this.#scanFolder(file, initial); } else if (!kDirectoryWatchReportsEntries) { - this.#watch(file, () => this.#onEntryEvent(file)); + if (!this.#watch(file, () => this.#onEntryEvent(file))) { + this.#forget(file); + } } } @@ -253,7 +300,7 @@ class FSWatcher extends EventEmitter { #watchRootFile(file) { const { statSync } = lazyLoadFsSync(); this.#entries.add(file); - this.#watch(file, () => { + if (!this.#watch(file, () => { if (this.#closed) { return; } @@ -263,7 +310,9 @@ class FSWatcher extends EventEmitter { } else { this.emit('change', 'change', pathBasename(file)); } - }); + })) { + this.#forget(file); + } } [kFSWatchStart](filename) { diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 5f45d1ecdccd..bffe32296911 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -105,8 +105,6 @@ test-fs-watch-ignore-regexp: SKIP test-runner-coverage: PASS, FLAKY # https://github.com/nodejs/node/issues/54346 test-esm-loader-hooks-inspect-wait: PASS, FLAKY -# https://github.com/nodejs/node/issues/65697 -test-fs-watch-recursive-delete-race: SKIP [$system==ibmi] # https://github.com/nodejs/node/pull/30819 diff --git a/test/parallel/test-fs-watch-recursive-aix-preserve-watch-error.js b/test/parallel/test-fs-watch-recursive-aix-preserve-watch-error.js new file mode 100644 index 000000000000..2573f582a312 --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-aix-preserve-watch-error.js @@ -0,0 +1,53 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +if (!common.isAIX) + common.skip('AIX-specific ENODEV handling'); + +const assert = require('assert'); +const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); +const { kFSWatchStart } = require('internal/fs/watchers'); +const { FSWatcher } = require('internal/fs/recursive_watch'); + +tmpdir.refresh(); + +const directory = tmpdir.resolve('preserve-watch-error'); +const originalStatSync = fs.statSync; +const originalWatch = fs.watch; +const expected = new Error('watch failed'); +const verificationError = new Error('stat failed'); +const watcher = new FSWatcher({ recursive: true, throwIfNoEntry: false }); +const failVerification = common.mustCall(() => { + throw verificationError; +}); + +expected.code = 'ENODEV'; +verificationError.code = 'EIO'; +fs.mkdirSync(directory); +fs.watch = common.mustCall((filename) => { + assert.strictEqual(filename, directory); + throw expected; +}); +fs.statSync = (filename, options) => { + if (filename === directory && options?.throwIfNoEntry === false) { + return failVerification(); + } + return Reflect.apply(originalStatSync, fs, [filename, options]); +}; + +try { + assert.throws( + () => watcher[kFSWatchStart](directory), + (error) => { + assert.strictEqual(error, expected); + return true; + }, + ); +} finally { + watcher.close(); + fs.statSync = originalStatSync; + fs.watch = originalWatch; +} diff --git a/test/parallel/test-fs-watch-recursive-retry-directory-disappears-before-read.js b/test/parallel/test-fs-watch-recursive-retry-directory-disappears-before-read.js new file mode 100644 index 000000000000..b91dbe625aeb --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-retry-directory-disappears-before-read.js @@ -0,0 +1,54 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); +const { kFSWatchStart } = require('internal/fs/watchers'); +const { FSWatcher } = require('internal/fs/recursive_watch'); + +function createWatcher() { + return { close() {} }; +} + +tmpdir.refresh(); + +const directory = tmpdir.resolve('retry-directory-disappears-before-read'); +const child = path.join(directory, 'child'); +const originalWatch = fs.watch; +const watcher = new FSWatcher({ recursive: true }); +let childWatchCalls = 0; +let directoryListener; +let staleWatcherCloseCalls = 0; + +fs.mkdirSync(child, { recursive: true }); +fs.watch = common.mustCall((filename, options, listener) => { + if (filename === directory) { + directoryListener = listener; + return createWatcher(); + } + + assert.strictEqual(filename, child); + childWatchCalls++; + if (childWatchCalls === 1) { + fs.rmSync(child, { recursive: true }); + return { close: () => staleWatcherCloseCalls++ }; + } + return createWatcher(); +}, 3); + +try { + watcher[kFSWatchStart](directory); + assert.strictEqual(childWatchCalls, 1); + assert.strictEqual(staleWatcherCloseCalls, 1); + + fs.mkdirSync(child); + directoryListener('rename', null); + + assert.strictEqual(childWatchCalls, 2); +} finally { + watcher.close(); + fs.watch = originalWatch; +} diff --git a/test/parallel/test-fs-watch-recursive-retry-directory-disappears-before-watch.js b/test/parallel/test-fs-watch-recursive-retry-directory-disappears-before-watch.js new file mode 100644 index 000000000000..8f570180a855 --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-retry-directory-disappears-before-watch.js @@ -0,0 +1,66 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +if (common.isIBMi) + common.skip('IBMi does not support `fs.watch()`'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); +const { kFSWatchStart } = require('internal/fs/watchers'); +const { FSWatcher } = require('internal/fs/recursive_watch'); + +function createWatcher() { + return { close() {} }; +} + +tmpdir.refresh(); + +const directory = tmpdir.resolve('retry-directory-disappears-before-watch'); +const child = path.join(directory, 'child'); +const originalWatch = fs.watch; +const changes = []; +const watcher = new FSWatcher({ recursive: true }); +let childWatchCalls = 0; +let directoryListener; +let watchError; + +fs.mkdirSync(child, { recursive: true }); +watcher.on('change', (eventType, filename) => changes.push([eventType, filename])); +fs.watch = common.mustCall((filename, options, listener) => { + if (filename === directory) { + directoryListener = listener; + return createWatcher(); + } + + assert.strictEqual(filename, child); + childWatchCalls++; + if (childWatchCalls === 1) { + fs.rmSync(child, { recursive: true }); + try { + return Reflect.apply(originalWatch, fs, [filename, options, listener]); + } catch (error) { + watchError = error; + throw error; + } + } + return createWatcher(); +}, 3); + +try { + watcher[kFSWatchStart](directory); + assert.strictEqual(childWatchCalls, 1); + assert.match(watchError?.code, /^(ENOENT|ENODEV)$/); + + fs.mkdirSync(child); + directoryListener('rename', null); + + assert.strictEqual(childWatchCalls, 2); + assert.deepStrictEqual(changes, [['rename', 'child']]); +} finally { + watcher.close(); + fs.watch = originalWatch; +} diff --git a/test/parallel/test-fs-watch-recursive-root-disappears-before-read.js b/test/parallel/test-fs-watch-recursive-root-disappears-before-read.js new file mode 100644 index 000000000000..7692c86a3abe --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-root-disappears-before-read.js @@ -0,0 +1,37 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); +const { kFSWatchStart } = require('internal/fs/watchers'); +const { FSWatcher } = require('internal/fs/recursive_watch'); + +tmpdir.refresh(); + +const directory = tmpdir.resolve('root-disappears-before-read'); +const originalWatch = fs.watch; +const watcher = new FSWatcher({ recursive: true }); +let closeCalls = 0; + +fs.mkdirSync(directory); +fs.watch = common.mustCall((filename) => { + assert.strictEqual(filename, directory); + fs.rmSync(directory, { recursive: true }); + return { close: () => closeCalls++ }; +}); + +try { + assert.throws( + () => watcher[kFSWatchStart](directory), + { + code: 'ENOENT', + filename: directory, + }, + ); + assert.strictEqual(closeCalls, 1); +} finally { + watcher.close(); + fs.watch = originalWatch; +} diff --git a/test/parallel/test-fs-watch-recursive-root-disappears-before-watch-no-throw.js b/test/parallel/test-fs-watch-recursive-root-disappears-before-watch-no-throw.js new file mode 100644 index 000000000000..c43f3e275184 --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-root-disappears-before-watch-no-throw.js @@ -0,0 +1,40 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +if (common.isIBMi) + common.skip('IBMi does not support `fs.watch()`'); + +const assert = require('assert'); +const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); +const { kFSWatchStart } = require('internal/fs/watchers'); +const { FSWatcher } = require('internal/fs/recursive_watch'); + +tmpdir.refresh(); + +const directory = tmpdir.resolve('root-disappears-before-watch-no-throw'); +const originalWatch = fs.watch; +const watcher = new FSWatcher({ recursive: true, throwIfNoEntry: false }); +let watchError; + +fs.mkdirSync(directory); +fs.watch = common.mustCall((filename, ...args) => { + assert.strictEqual(filename, directory); + fs.rmSync(directory, { recursive: true }); + try { + return Reflect.apply(originalWatch, fs, [filename, ...args]); + } catch (error) { + watchError = error; + throw error; + } +}); + +try { + watcher[kFSWatchStart](directory); + assert.match(watchError?.code, /^(ENOENT|ENODEV)$/); +} finally { + watcher.close(); + fs.watch = originalWatch; +} diff --git a/test/parallel/test-fs-watch-recursive-root-disappears-before-watch.js b/test/parallel/test-fs-watch-recursive-root-disappears-before-watch.js new file mode 100644 index 000000000000..917489004650 --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-root-disappears-before-watch.js @@ -0,0 +1,46 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +if (common.isIBMi) + common.skip('IBMi does not support `fs.watch()`'); + +const assert = require('assert'); +const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); +const { kFSWatchStart } = require('internal/fs/watchers'); +const { FSWatcher } = require('internal/fs/recursive_watch'); + +tmpdir.refresh(); + +const directory = tmpdir.resolve('root-disappears-before-watch'); +const originalWatch = fs.watch; +const watcher = new FSWatcher({ recursive: true }); +let watchError; + +fs.mkdirSync(directory); +fs.watch = common.mustCall((filename, ...args) => { + assert.strictEqual(filename, directory); + fs.rmSync(directory, { recursive: true }); + try { + return Reflect.apply(originalWatch, fs, [filename, ...args]); + } catch (error) { + watchError = error; + throw error; + } +}); + +try { + assert.throws( + () => watcher[kFSWatchStart](directory), + (error) => { + assert.strictEqual(error, watchError); + assert.match(error.code, /^(ENOENT|ENODEV)$/); + return true; + }, + ); +} finally { + watcher.close(); + fs.watch = originalWatch; +} diff --git a/test/parallel/test-fs-watch-recursive-watch-before-read.js b/test/parallel/test-fs-watch-recursive-watch-before-read.js new file mode 100644 index 000000000000..4faeb5b8efbe --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-watch-before-read.js @@ -0,0 +1,42 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); +const { kFSWatchStart } = require('internal/fs/watchers'); +const { FSWatcher } = require('internal/fs/recursive_watch'); + +function createWatcher() { + return { close() {} }; +} + +tmpdir.refresh(); + +const directory = tmpdir.resolve('watch-before-read'); +const originalReaddirSync = fs.readdirSync; +const originalWatch = fs.watch; +const operations = []; +const watcher = new FSWatcher({ recursive: true }); + +fs.mkdirSync(directory); +fs.watch = common.mustCall((filename) => { + assert.strictEqual(filename, directory); + operations.push('watch'); + return createWatcher(); +}); +fs.readdirSync = common.mustCall((filename, ...args) => { + assert.strictEqual(filename, directory); + operations.push('read'); + return Reflect.apply(originalReaddirSync, fs, [filename, ...args]); +}); + +try { + watcher[kFSWatchStart](directory); + assert.deepStrictEqual(operations, ['watch', 'read']); +} finally { + watcher.close(); + fs.readdirSync = originalReaddirSync; + fs.watch = originalWatch; +}