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, 17 insertions, 4 deletions

diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js
index 685d5be5e4..1007a5a136 100644
--- a/lib/internal/fs/watchers.js
+++ b/lib/internal/fs/watchers.js
@@ -100,7 +100,11 @@ function FSWatcher() {
     // after the handle is closed, and to fire both UV_RENAME and UV_CHANGE
     // if they are set by libuv at the same time.
     if (status < 0) {
-      this._handle.close();
+      if (this._handle !== null) {
+        // We don't use this.close() here to avoid firing the close event.
+        this._handle.close();
+        this._handle = null;  // make the handle garbage collectable
+      }
       const error = errors.uvException({
         errno: status,
         syscall: 'watch',
@@ -120,13 +124,17 @@ util.inherits(FSWatcher, EventEmitter);
 // 1. Throw an Error if it's the first time .start() is called
 // 2. Return silently if .start() has already been called
 //    on a valid filename and the wrap has been initialized
+// 3. Return silently if the watcher has already been closed
 // This method is a noop if the watcher has already been started.
 FSWatcher.prototype.start = function(filename,
                                      persistent,
                                      recursive,
                                      encoding) {
+  if (this._handle === null) {  // closed
+    return;
+  }
   assert(this._handle instanceof FSEvent, 'handle must be a FSEvent');
-  if (this._handle.initialized) {
+  if (this._handle.initialized) {  // already started
     return;
   }
 
@@ -148,13 +156,18 @@ FSWatcher.prototype.start = function(filename,
   }
 };
 
-// This method is a noop if the watcher has not been started.
+// This method is a noop if the watcher has not been started or
+// has already been closed.
 FSWatcher.prototype.close = function() {
+  if (this._handle === null) {  // closed
+    return;
+  }
   assert(this._handle instanceof FSEvent, 'handle must be a FSEvent');
-  if (!this._handle.initialized) {
+  if (!this._handle.initialized) {  // not started
     return;
   }
   this._handle.close();
+  this._handle = null;  // make the handle garbage collectable
   process.nextTick(emitCloseNT, this);
 };