fs: Add unwatchFile to handle reference counting

nodejs · rickyes · Apr 28, 2020 · Apr 29, 2020 · May 1, 2020 · Apr 29, 2020
commit 5cb73d42d3ee71afd2483d10051370e05610ca19
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -596,12 +596,47 @@ called previously.
 <!-- YAML
 added: REPLACEME
 -->
+
 When called, the active `FSWatcher` object will not require the Node.js
 event loop to remain active. If there is no other activity keeping the
 event loop running, the process may exit before the `FSWatcher` object's
 callback is invoked. Calling `watcher.unref()` multiple times will have
 no effect.
 
+## Class: `fs.StatWatcher`
+<!-- YAML
+added: REPLACEME
+-->
+
+* Extends {EventEmitter}
+
+A successful call to `fs.watchFile()` method will return a new `fs.StatWatcher`
+object.
+
+### `watcher.ref()`
+<!-- YAML
+added: REPLACEME
+-->
+
+When called, requests that the Node.js event loop *not* exit so long as the
+`StatWatcher` is active. Calling `watcher.ref()` multiple times will have
+no effect.
+
+By default, all `StatWatcher` objects are "ref'ed", making it normally
+unnecessary to call `watcher.ref()` unless `watcher.unref()` had been
+called previously.
+
+### `watcher.unref()`
+<!-- YAML
+added: REPLACEME
+-->
+
+When called, the active `StatWatcher` object will not require the Node.js
+event loop to remain active. If there is no other activity keeping the
+event loop running, the process may exit before the `StatWatcher` object's
+callback is invoked. Calling `watcher.unref()` multiple times will have
+no effect.
+
 ## Class: `fs.ReadStream`
 <!-- YAML
 added: v0.1.93
@@ -3981,6 +4016,7 @@ changes:
 * `listener` {Function}
   * `current` {fs.Stats}
   * `previous` {fs.Stats}
+* Returns: {fs.StatWatcher}
 
 Watch for changes on `filename`. The callback `listener` will be called each
 time the file is accessed.

diff --git a/lib/fs.js b/lib/fs.js
@@ -1490,8 +1490,7 @@ function watchFile(filename, options, listener) {
                                        options.persistent, options.interval);
     statWatchers.set(filename, stat);
   } else {
-    stat[watchers.KFSStatWatcherRefCount]++;
-    stat[watchers.KFSStatWatcherMaxRefCount]++;
+    stat[watchers.kFSStatWatcherAddOrCleanRef]('add');
   }
 
   stat.addListener('change', listener);
@@ -1506,9 +1505,13 @@ function unwatchFile(filename, listener) {
   if (stat === undefined) return;
 
   if (typeof listener === 'function') {
+    const beforeListenerCount = stat.listenerCount('change');
     stat.removeListener('change', listener);
+    if (stat.listenerCount('change') < beforeListenerCount)
+      stat[watchers.kFSStatWatcherAddOrCleanRef]('clean');
   } else {
     stat.removeAllListeners('change');
+    stat[watchers.kFSStatWatcherAddOrCleanRef]('cleanAll');
   }
 
   if (stat.listenerCount('change') === 0) {

diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js
@@ -33,6 +33,7 @@ const kFSWatchStart = Symbol('kFSWatchStart');
 const kFSStatWatcherStart = Symbol('kFSStatWatcherStart');
 const KFSStatWatcherRefCount = Symbol('KFSStatWatcherRefCount');
 const KFSStatWatcherMaxRefCount = Symbol('KFSStatWatcherMaxRefCount');
+const kFSStatWatcherAddOrCleanRef = Symbol('kFSStatWatcherAddOrCleanRef');
 
 function emitStop(self) {
   self.emit('stop');
@@ -121,19 +122,39 @@ StatWatcher.prototype.stop = function() {
   this._handle = null;
 };
 
+// Clean up or add ref counters.
+StatWatcher.prototype[kFSStatWatcherAddOrCleanRef] = function(operate) {
+  if (operate === 'add') {
+    // Add a Ref
+    this[KFSStatWatcherRefCount]++;
+    this[KFSStatWatcherMaxRefCount]++;
+  } else if (operate === 'clean') {
+    // Clean up a single
+    this[KFSStatWatcherMaxRefCount]--;
+    this.unref();
+  } else if (operate === 'cleanAll') {
+    // Clean up all
+    this[KFSStatWatcherMaxRefCount] = 0;
+    this[KFSStatWatcherRefCount] = 0;
+    this._handle && this._handle.unref();
+  }
+};
+
 StatWatcher.prototype.ref = function() {
   // Avoid refCount calling ref multiple times causing unref to have no effect.
   if (this[KFSStatWatcherRefCount] === this[KFSStatWatcherMaxRefCount])
-    return;
+    return this;
   if (this._handle && this[KFSStatWatcherRefCount]++ === 0)
     this._handle.ref();
+  return this;
 };
 
 StatWatcher.prototype.unref = function() {
   // Avoid refCount calling unref multiple times causing ref to have no effect.
-  if (this[KFSStatWatcherRefCount] === 0) return;
+  if (this[KFSStatWatcherRefCount] === 0) return this;
   if (this._handle && --this[KFSStatWatcherRefCount] === 0)
     this._handle.unref();
+  return this;
 };
 
 
@@ -229,10 +250,12 @@ FSWatcher.prototype.close = function() {
 
 FSWatcher.prototype.ref = function() {
   if (this._handle) this._handle.ref();
+  return this;
 };
 
 FSWatcher.prototype.unref = function() {
   if (this._handle) this._handle.unref();
+  return this;
 };
 
 function emitCloseNT(self) {
@@ -251,6 +274,5 @@ module.exports = {
   StatWatcher,
   kFSWatchStart,
   kFSStatWatcherStart,
-  KFSStatWatcherRefCount,
-  KFSStatWatcherMaxRefCount,
+  kFSStatWatcherAddOrCleanRef,
 };
diff --git a/test/parallel/test-fs-watch-ref-unref.js b/test/parallel/test-fs-watch-ref-unref.js
@@ -6,30 +6,15 @@ if (common.isIBMi)
   common.skip('IBMi does not support `fs.watch()`');
 
 const fs = require('fs');
-const assert = require('assert');
-
-let refCount = 0;
 
 const watcher = fs.watch(__filename, common.mustNotCall());
 
-function unref() {
-  watcher.unref();
-  refCount--;
-  assert.strictEqual(refCount, -1);
-}
-
-function ref() {
-  watcher.ref();
-  refCount++;
-  assert.strictEqual(refCount, 0);
-}
-
-unref();
+watcher.unref();
 
 setTimeout(
   common.mustCall(() => {
-    ref();
-    unref();
+    watcher.ref();
+    watcher.unref();
   }),
   common.platformTimeout(100)
 );
diff --git a/test/parallel/test-fs-watchfile-ref-unref.js b/test/parallel/test-fs-watchfile-ref-unref.js
@@ -5,32 +5,31 @@ const common = require('../common');
 const fs = require('fs');
 const assert = require('assert');
 
-let refCount = 0;
+const uncalledListener = common.mustNotCall();
+const uncalledListener2 = common.mustNotCall();
+const watcher = fs.watchFile(__filename, uncalledListener);
 
-const watcher = fs.watchFile(__filename, common.mustNotCall());
+watcher.unref();
+watcher.unref();
+watcher.ref();
+watcher.unref();
+watcher.ref();
+watcher.ref();
+watcher.unref();
 
-function unref() {
-  watcher.unref();
-  refCount--;
-  assert.strictEqual(refCount, -1);
-}
+fs.unwatchFile(__filename, uncalledListener);
 
-function ref() {
-  watcher.ref();
-  refCount++;
-  assert.strictEqual(refCount, 0);
-}
-
-unref();
+// Watch the file with two different listeners.
+fs.watchFile(__filename, uncalledListener);
+const watcher2 = fs.watchFile(__filename, uncalledListener2);
 
 setTimeout(
   common.mustCall(() => {
-    ref();
-    unref();
-    watcher.unref();
-    watcher.ref();
-    watcher.ref();
-    watcher.unref();
+    fs.unwatchFile(__filename, common.mustNotCall());
+    assert.strictEqual(watcher2.listenerCount('change'), 2);
+    fs.unwatchFile(__filename, uncalledListener);
+    assert.strictEqual(watcher2.listenerCount('change'), 1);
+    watcher2.unref();
   }),
   common.platformTimeout(100)
 );
diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js
@@ -84,6 +84,7 @@ const customTypesMap = {
   'fs.FSWatcher': 'fs.html#fs_class_fs_fswatcher',
   'fs.ReadStream': 'fs.html#fs_class_fs_readstream',
   'fs.Stats': 'fs.html#fs_class_fs_stats',
+  'fs.StatWatcher': 'fs.html#fs_class_fs_statwatcher',
   'fs.WriteStream': 'fs.html#fs_class_fs_writestream',
 
   'http.Agent': 'http.html#http_class_http_agent',