fs: do not crash when using a closed fs event watcher

Before this commit, when the user calls methods on a closed or
errored fs event watcher, they could hit a crash since the
FSEventWrap in C++ land may have already been destroyed with
the internal pointer set to nullptr. This commit makes sure
that the user cannot hit crashes like that, instead the
methods calling on a closed watcher will be noops.

Also explicitly documents that the watchers should not be used
in `close` and `error` event handlers.

PR-URL: https://github.com/nodejs/node/pull/20985
Fixes: https://github.com/nodejs/node/issues/20738
Fixes: https://github.com/nodejs/node/issues/20297
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

1 files changed, 38 insertions, 0 deletions

diff --git a/test/parallel/test-fs-watch-close-when-destroyed.js b/test/parallel/test-fs-watch-close-when-destroyed.js
new file mode 100644
index 0000000000..fb5239bc44
--- /dev/null
+++ b/test/parallel/test-fs-watch-close-when-destroyed.js
@@ -0,0 +1,38 @@
+'use strict';
+
+// This tests that closing a watcher when the underlying handle is
+// already destroyed will result in a noop instead of a crash.
+
+const common = require('../common');
+const tmpdir = require('../common/tmpdir');
+const fs = require('fs');
+const path = require('path');
+
+tmpdir.refresh();
+const root = path.join(tmpdir.path, 'watched-directory');
+fs.mkdirSync(root);
+
+const watcher = fs.watch(root, { persistent: false, recursive: false });
+
+// The following listeners may or may not be invoked.
+
+watcher.addListener('error', () => {
+  setTimeout(
+    () => { watcher.close(); },  // Should not crash if it's invoked
+    common.platformTimeout(10)
+  );
+});
+
+watcher.addListener('change', () => {
+  setTimeout(
+    () => { watcher.close(); },
+    common.platformTimeout(10)
+  );
+});
+
+fs.rmdirSync(root);
+// Wait for the listener to hit
+setTimeout(
+  common.mustCall(() => {}),
+  common.platformTimeout(100)
+);