major rewrite of Linux recursive watch

Signed-off-by: Matteo Collina <hello@matteocollina.com>
nodejs-github-bot · Jan 25, 2024 · Jan 8, 2024 · Jan 8, 2024 · Jan 11, 2024 · Jan 11, 2024
commit faf947d69034a6d36bcdaf55110af2f683889d1b
diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js
@@ -40,6 +40,7 @@ class FSWatcher extends EventEmitter {
   #options = null;
   #closed = false;
   #files = new SafeMap();
+  #watchers = new SafeMap();
   #symbolicFiles = new SafeSet();
   #rootPath = pathResolve();
   #watchingFile = false;
@@ -79,11 +80,11 @@ class FSWatcher extends EventEmitter {
       return;
     }
 
-    const { unwatchFile } = lazyLoadFsSync();
     this.#closed = true;
 
     for (const file of this.#files.keys()) {
-      unwatchFile(file);
+      this.#watchers.get(file).close();
+      this.#watchers.delete(file);
     }
 
     this.#files.clear();
@@ -92,13 +93,13 @@ class FSWatcher extends EventEmitter {
   }
 
   #unwatchFiles(file) {
-    const { unwatchFile } = lazyLoadFsSync();
-
     this.#symbolicFiles.delete(file);
 
     for (const filename of this.#files.keys()) {
       if (StringPrototypeStartsWith(filename, file)) {
-        unwatchFile(filename);
+        this.#files.delete(filename);
+        this.#watchers.get(filename).close();
+        this.#watchers.delete(filename);
       }
     }
   }
@@ -125,10 +126,8 @@ class FSWatcher extends EventEmitter {
             this.#symbolicFiles.add(f);
           }
 
-          this.#files.set(f, file);
-          if (file.isFile()) {
-            this.#watchFile(f);
-          } else if (file.isDirectory() && !file.isSymbolicLink()) {
+          this.#watchFile(f);
+          if (file.isDirectory() && !file.isSymbolicLink()) {
             this.#watchFolder(f);
           }
         }
@@ -143,22 +142,30 @@ class FSWatcher extends EventEmitter {
       return;
     }
 
-    const { watchFile } = lazyLoadFsSync();
-    const existingStat = this.#files.get(file);
+    const { watch, statSync } = lazyLoadFsSync();
+
+    if (this.#files.has(file)) {
+      return;
+    }
+
+    {
+      const existingStat = statSync(file);
+      this.#files.set(file, existingStat);
+    }
 
-    watchFile(file, {
+    const watcher = watch(file, {
       persistent: this.#options.persistent,
-    }, (currentStats, previousStats) => {
-      if (existingStat && !existingStat.isDirectory() &&
-        currentStats.nlink !== 0 && existingStat.mtimeMs === currentStats.mtimeMs) {
-        return;
-      }
+    }, (eventType, filename) => {
+      const existingStat = this.#files.get(file);
+      const currentStats = statSync(file);
 
       this.#files.set(file, currentStats);
 
-      if (currentStats.birthtimeMs === 0 && previousStats.birthtimeMs !== 0) {
+      if (currentStats.birthtimeMs === 0 && existingStat.birthtimeMs !== 0) {
         // The file is now deleted
         this.#files.delete(file);
+        this.#watchers.delete(file);
+        watcher.close();
         this.emit('change', 'rename', pathRelative(this.#rootPath, file));
         this.#unwatchFiles(file);
       } else if (file === this.#rootPath && this.#watchingFile) {
@@ -175,6 +182,7 @@ class FSWatcher extends EventEmitter {
         this.emit('change', 'change', pathRelative(this.#rootPath, file));
       }
     });
+    this.#watchers.set(file, watcher);
   }
 
   [kFSWatchStart](filename) {
@@ -187,15 +195,9 @@ class FSWatcher extends EventEmitter {
       this.#closed = false;
       this.#watchingFile = file.isFile();
 
+      this.#watchFile(filename);
       if (file.isDirectory()) {
-        this.#files.set(filename, file);
-
         this.#watchFolder(filename);
-        for (const f of this.#files.keys()) {
-          this.#watchFile(f);
-        }
-      } else {
-        this.#watchFile(filename);
       }
     } catch (error) {
       if (error.code === 'ENOENT') {
@@ -230,7 +232,10 @@ class FSWatcher extends EventEmitter {
           resolve({ __proto__: null, value: { eventType, filename } });
         });
       } : (resolve, reject) => {
-        const onAbort = () => reject(new AbortError(undefined, { cause: signal.reason }));
+        const onAbort = () => {
+          this.close();
+          reject(new AbortError(undefined, { cause: signal.reason }));
+        };
         if (signal.aborted) return onAbort();
         kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
         signal.addEventListener('abort', onAbort, { __proto__: null, once: true, [kResistStopPropagation]: true });
@@ -243,6 +248,10 @@ class FSWatcher extends EventEmitter {
       next: () => (this.#closed ?
         { __proto__: null, done: true } :
         new Promise(promiseExecutor)),
+      return: () => {
+        this.close();
+        return { __proto__: null, done: true };
+      },
       [SymbolAsyncIterator]() { return this; },
     };
   }

diff --git a/test/parallel/test-fs-recursive-sync-write.js b/test/parallel/test-fs-recursive-sync-write.js
@@ -33,7 +33,3 @@ const watcher = watch(tmpDir, { recursive: true }, common.mustCall((eventType, _
 }));
 
 writeFileSync(filename, 'foobar2');
-
-process.once('exit', function() {
-  assert(watcherClosed, 'watcher Object was not closed');
-});
diff --git a/test/parallel/test-fs-watch-recursive-add-file-to-existing-subfolder.js b/test/parallel/test-fs-watch-recursive-add-file-to-existing-subfolder.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const common = require('../common');
-const { setTimeout } = require('timers/promises');
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');

diff --git a/test/parallel/test-fs-watch-recursive-add-file-to-new-folder.js b/test/parallel/test-fs-watch-recursive-add-file-to-new-folder.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const common = require('../common');
-const { setTimeout } = require('timers/promises');
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');

diff --git a/test/parallel/test-fs-watch-recursive-add-file-with-url.js b/test/parallel/test-fs-watch-recursive-add-file-with-url.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const common = require('../common');
-const { setTimeout } = require('timers/promises');
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');

diff --git a/test/parallel/test-fs-watch-recursive-add-file.js b/test/parallel/test-fs-watch-recursive-add-file.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const common = require('../common');
-const { setTimeout } = require('timers/promises');
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');

diff --git a/test/parallel/test-fs-watch-recursive-add-folder.js b/test/parallel/test-fs-watch-recursive-add-folder.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const common = require('../common');
-const { setTimeout } = require('timers/promises');
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');

diff --git a/test/parallel/test-fs-watch-recursive-symlink.js b/test/parallel/test-fs-watch-recursive-symlink.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const common = require('../common');
-const { setTimeout } = require('timers/promises');
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');

diff --git a/test/parallel/test-fs-watch-recursive-update-file.js b/test/parallel/test-fs-watch-recursive-update-file.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const common = require('../common');
-const { setTimeout } = require('timers/promises');
 
 if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');
@@ -31,14 +30,12 @@ const testFile = path.join(testDirectory, 'file-1.txt');
 fs.writeFileSync(testFile, 'hello');
 
 const watcher = fs.watch(testDirectory, { recursive: true });
-let watcherClosed = false;
 watcher.on('change', common.mustCallAtLeast(function(event, filename) {
   // Libuv inconsistenly emits a rename event for the file we are watching
   assert.ok(event === 'change' || event === 'rename');
 
   if (filename === path.basename(testFile)) {
     watcher.close();
-    watcherClosed = true;
   }
 }));