From beee1f201009d769afd542cda53adfc5830f8443 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 14 Jun 2024 15:41:53 +0200 Subject: [PATCH 1/3] fs: do not crash if the watched file is removed while setting up watch Signed-off-by: Matteo Collina --- lib/internal/fs/recursive_watch.js | 17 +++++++--- ...s-watch-recursive-linux-parallel-remove.js | 32 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-fs-watch-recursive-linux-parallel-remove.js diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index d312e54013cfb9..606f32ea3293c2 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -83,7 +83,7 @@ class FSWatcher extends EventEmitter { this.#closed = true; for (const file of this.#files.keys()) { - this.#watchers.get(file).close(); + this.#watchers.get(file)?.close(); this.#watchers.delete(file); } @@ -98,7 +98,7 @@ class FSWatcher extends EventEmitter { for (const filename of this.#files.keys()) { if (StringPrototypeStartsWith(filename, file)) { this.#files.delete(filename); - this.#watchers.get(filename).close(); + this.#watchers.get(filename)?.close(); this.#watchers.delete(filename); } } @@ -126,9 +126,16 @@ class FSWatcher extends EventEmitter { this.#symbolicFiles.add(f); } - this.#watchFile(f); - if (file.isDirectory() && !file.isSymbolicLink()) { - this.#watchFolder(f); + try { + this.#watchFile(f); + if (file.isDirectory() && !file.isSymbolicLink()) { + this.#watchFolder(f); + } + } catch (err) { + // Ignore ENOENT + if (err.code !== 'ENOENT') { + throw err + } } } } diff --git a/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js b/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js new file mode 100644 index 00000000000000..ef045dc14dd157 --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js @@ -0,0 +1,32 @@ +'use strict'; + +const common = require('../common'); + +if (!common.isLinux) + common.skip('This test can run only on Linux'); + +const assert = require('node:assert'); +const path = require('node:path'); +const fs = require('node:fs'); +const { spawn } = require('node:child_process'); + +const tmpdir = require('../common/tmpdir'); +const testDir = tmpdir.path; +tmpdir.refresh(); + +const watcher = fs.watch(testDir, { recursive: true }); +watcher.on('change', function(event, filename) { + // This console.log makes the error happen + // do not remove + console.log(filename, event) + // assert.strictEqual(event, 'rename'); +}); + +const testFile = path.join(testDir, 'a') +const child = spawn(process.argv[0], ['-e', `const fs = require('node:fs'); for (let i = 0; i < 10000; i++) { const fd = fs.openSync('${testFile}', 'w'); fs.writeSync(fd, Buffer.from('hello')); fs.rmSync('${testFile}') }`], { + stdio: 'inherit' +}) + +child.on('exit', function () { + watcher.close() +}) From 2b0f06927a15d16ad68c34603c54308b4aa6be71 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 14 Jun 2024 15:50:44 +0200 Subject: [PATCH 2/3] fixup Signed-off-by: Matteo Collina --- lib/internal/fs/recursive_watch.js | 2 +- ...-fs-watch-recursive-linux-parallel-remove.js | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index 606f32ea3293c2..38c0505797c3ad 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -134,7 +134,7 @@ class FSWatcher extends EventEmitter { } catch (err) { // Ignore ENOENT if (err.code !== 'ENOENT') { - throw err + throw err; } } } diff --git a/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js b/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js index ef045dc14dd157..a2c2856ed90cc9 100644 --- a/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js +++ b/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js @@ -5,7 +5,9 @@ const common = require('../common'); if (!common.isLinux) common.skip('This test can run only on Linux'); -const assert = require('node:assert'); +// Test the watcher do not crash if the file "disappears" while +// watch is being set up. + const path = require('node:path'); const fs = require('node:fs'); const { spawn } = require('node:child_process'); @@ -18,15 +20,14 @@ const watcher = fs.watch(testDir, { recursive: true }); watcher.on('change', function(event, filename) { // This console.log makes the error happen // do not remove - console.log(filename, event) - // assert.strictEqual(event, 'rename'); + console.log(filename, event); }); -const testFile = path.join(testDir, 'a') +const testFile = path.join(testDir, 'a'); const child = spawn(process.argv[0], ['-e', `const fs = require('node:fs'); for (let i = 0; i < 10000; i++) { const fd = fs.openSync('${testFile}', 'w'); fs.writeSync(fd, Buffer.from('hello')); fs.rmSync('${testFile}') }`], { stdio: 'inherit' -}) +}); -child.on('exit', function () { - watcher.close() -}) +child.on('exit', function() { + watcher.close(); +}); From 97c7652629e3baeaaa2392e11a97c7c11c445329 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 15 Jun 2024 07:38:45 +0200 Subject: [PATCH 3/3] Update test-fs-watch-recursive-linux-parallel-remove.js Co-authored-by: Luigi Pinca --- test/parallel/test-fs-watch-recursive-linux-parallel-remove.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js b/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js index a2c2856ed90cc9..145b3314f24b59 100644 --- a/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js +++ b/test/parallel/test-fs-watch-recursive-linux-parallel-remove.js @@ -5,7 +5,7 @@ const common = require('../common'); if (!common.isLinux) common.skip('This test can run only on Linux'); -// Test the watcher do not crash if the file "disappears" while +// Test that the watcher do not crash if the file "disappears" while // watch is being set up. const path = require('node:path');