repl: keep reference count for process.on('newListener')

When investigating a memory leak in one of our applications,
we discovered that this listener holds on to a `REPLServer`
instance and all heap objects transitively kept alive by it
by capturing as part of its closure.

It's cleaner to declare the listener outside of the `REPLServer`
class and to actually clean it up properly when it is no longer
required or meaningful, which is easily achieved through
keeping a reference count.

PR-URL: #61895
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Author: addaleax

lib/repl.js

@@ -186,7 +186,35 @@ const domainSet = new SafeWeakSet();
 const kBufferedCommandSymbol = Symbol('bufferedCommand');
 const kLoadingSymbol = Symbol('loading');
 
-let addedNewListener = false;
+function processNewListener(event, listener) {
+  if (event === 'uncaughtException' &&
+      process.domain &&
+      listener.name !== 'domainUncaughtExceptionClear' &&
+      domainSet.has(process.domain)) {
+    // Throw an error so that the event will not be added and the current
+    // domain takes over. That way the user is notified about the error
+    // and the current code evaluation is stopped, just as any other code
+    // that contains an error.
+    throw new ERR_INVALID_REPL_INPUT(
+      'Listeners for `uncaughtException` cannot be used in the REPL');
+  }
+}
+
+let processNewListenerUseCount = 0;
+function addProcessNewListener() {
+  if (processNewListenerUseCount++ === 0) {
+    // Add this listener only once and use a WeakSet that contains the REPLs
+    // domains. Otherwise we'd have to add a single listener to each REPL
+    // instance and that could trigger the `MaxListenersExceededWarning`.
+    process.prependListener('newListener', processNewListener);
+  }
+}
+
+function removeProcessNewListener() {
+  if (--processNewListenerUseCount === 0) {
+    process.removeListener('newListener', processNewListener);
+  }
+}
 
 fixReplRequire(module);
 
@@ -327,24 +355,9 @@ function REPLServer(prompt,
     // It is possible to introspect the running REPL accessing this variable
     // from inside the REPL. This is useful for anyone working on the REPL.
     module.exports.repl = this;
-  } else if (!addedNewListener) {
-    // Add this listener only once and use a WeakSet that contains the REPLs
-    // domains. Otherwise we'd have to add a single listener to each REPL
-    // instance and that could trigger the `MaxListenersExceededWarning`.
-    process.prependListener('newListener', (event, listener) => {
-      if (event === 'uncaughtException' &&
-          process.domain &&
-          listener.name !== 'domainUncaughtExceptionClear' &&
-          domainSet.has(process.domain)) {
-        // Throw an error so that the event will not be added and the current
-        // domain takes over. That way the user is notified about the error
-        // and the current code evaluation is stopped, just as any other code
-        // that contains an error.
-        throw new ERR_INVALID_REPL_INPUT(
-          'Listeners for `uncaughtException` cannot be used in the REPL');
-      }
-    });
-    addedNewListener = true;
+  } else {
+    addProcessNewListener();
+    this.once('exit', removeProcessNewListener);
   }
 
   domainSet.add(this._domain);

test/parallel/test-repl-no-terminal-restore-process-listeners.js

@@ -0,0 +1,17 @@
+'use strict';
+const common = require('../common');
+const { startNewREPLServer } = require('../common/repl');
+const assert = require('assert');
+
+const originalProcessNewListenerCount = process.listenerCount('newListener');
+const { replServer } = startNewREPLServer();
+
+const listenerCountBeforeClose = process.listenerCount('newListener');
+replServer.close();
+replServer.once('exit', common.mustCall(() => {
+  setImmediate(common.mustCall(() => {
+    const listenerCountAfterClose = process.listenerCount('newListener');
+    assert.strictEqual(listenerCountAfterClose, listenerCountBeforeClose - 1);
+    assert.strictEqual(listenerCountAfterClose, originalProcessNewListenerCount);
+  }));
+}));