fs: fix nonNativeWatcher leak of StatWatchers

PR-URL: nodejs#45501 Reviewed-By: Yagiz Nizipli <[email protected]>
ErickWendel · Nov 30, 2022 · cc12b01 · cc12b01
1 parent fc02232
commit cc12b01
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 6 deletions.
diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js
@@ -154,14 +154,11 @@ class FSWatcher extends EventEmitter {
             this.#symbolicFiles.add(f);
           }
 
+          this.#files.set(f, file);
           if (file.isFile()) {
             this.#watchFile(f);
-          } else {
-            this.#files.set(f, file);
-
-            if (file.isDirectory() && !file.isSymbolicLink()) {
-              await this.#watchFolder(f);
-            }
+          } else if (file.isDirectory() && !file.isSymbolicLink()) {
+            await this.#watchFolder(f);
           }
         }
       }

diff --git a/test/parallel/test-fs-watch-recursive.js b/test/parallel/test-fs-watch-recursive.js
@@ -213,6 +213,33 @@ tmpdir.refresh();
   });
 })().then(common.mustCall());
 
+
+(async () => {
+  // Assert recursive watch does not leak handles
+  const rootDirectory = fs.mkdtempSync(testDir + path.sep);
+  const testDirectory = path.join(rootDirectory, 'test-7');
+  const filePath = path.join(testDirectory, 'only-file.txt');
+  fs.mkdirSync(testDirectory);
+
+  let watcherClosed = false;
+  const watcher = fs.watch(testDirectory, { recursive: true });
+  watcher.on('change', common.mustCallAtLeast(async (event, filename) => {
+    await setTimeout(common.platformTimeout(100));
+    if (filename === path.basename(filePath)) {
+      watcher.close();
+      watcherClosed = true;
+    }
+    await setTimeout(common.platformTimeout(100));
+    assert(!process._getActiveHandles().some((handle) => handle.constructor.name === 'StatWatcher'));
+  }));
+
+  process.on('exit', function() {
+    assert(watcherClosed, 'watcher Object was not closed');
+  });
+  await setTimeout(common.platformTimeout(100));
+  fs.writeFileSync(filePath, 'content');
+})().then(common.mustCall());
+
 (async () => {
   // Handle non-boolean values for options.recursive