fs: do not crash if the watched file is removed while setting up watch

mcollina · targos · commit 568377f7f06d · 2024-06-20T14:45:16.000+02:00
Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #53452 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
diff --git 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
@@ -0,0 +1,33 @@
+'use strict';
+
+const common = require('../common');
+
+if (!common.isLinux)
+  common.skip('This test can run only on Linux');
+
+// Test that 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');
+
+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);
+});
+
+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();
+});

Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ class FSWatcher extends EventEmitter {`
`83`	`83`	`this.#closed = true;`
`84`	`84`
`85`	`85`	`for (const file of this.#files.keys()) {`
`86`		`- this.#watchers.get(file).close();`
	`86`	`+ this.#watchers.get(file)?.close();`
`87`	`87`	`this.#watchers.delete(file);`
`88`	`88`	`}`
`89`	`89`
`@@ -98,7 +98,7 @@ class FSWatcher extends EventEmitter {`
`98`	`98`	`for (const filename of this.#files.keys()) {`
`99`	`99`	`if (StringPrototypeStartsWith(filename, file)) {`
`100`	`100`	`this.#files.delete(filename);`
`101`		`- this.#watchers.get(filename).close();`
	`101`	`+ this.#watchers.get(filename)?.close();`
`102`	`102`	`this.#watchers.delete(filename);`
`103`	`103`	`}`
`104`	`104`	`}`
`@@ -126,9 +126,16 @@ class FSWatcher extends EventEmitter {`
`126`	`126`	`this.#symbolicFiles.add(f);`
`127`	`127`	`}`
`128`	`128`
`129`		`- this.#watchFile(f);`
`130`		`- if (file.isDirectory() && !file.isSymbolicLink()) {`
`131`		`- this.#watchFolder(f);`
	`129`	`+ try {`
	`130`	`+ this.#watchFile(f);`
	`131`	`+ if (file.isDirectory() && !file.isSymbolicLink()) {`
	`132`	`+ this.#watchFolder(f);`
	`133`	`+ }`
	`134`	`+ } catch (err) {`
	`135`	`+ // Ignore ENOENT`
	`136`	`+ if (err.code !== 'ENOENT') {`
	`137`	`+ throw err;`
	`138`	`+ }`
`132`	`139`	`}`
`133`	`140`	`}`
`134`	`141`	`}`