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
Backport-PR-URL: #63194
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Author: addaleax

lib/repl.js

@@ -186,7 +186,35 @@ const domainSet = new SafeWeakSet();
 constkBufferedCommandSymbol=Symbol('bufferedCommand');
 constkLoadingSymbol=Symbol('loading');
 
-letaddedNewListener=false;
+functionprocessNewListener(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.
+thrownewERR_INVALID_REPL_INPUT(
+'Listeners for `uncaughtException` cannot be used in the REPL');
+}
+}
+
+letprocessNewListenerUseCount=0;
+functionaddProcessNewListener(){
+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);
+}
+}
+
+functionremoveProcessNewListener(){
+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;
-}elseif(!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.
-thrownewERR_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';
+constcommon=require('../common');
+const{ startNewREPLServer }=require('../common/repl');
+constassert=require('assert');
+
+constoriginalProcessNewListenerCount=process.listenerCount('newListener');
+const{ replServer }=startNewREPLServer();
+
+constlistenerCountBeforeClose=process.listenerCount('newListener');
+replServer.close();
+replServer.once('exit',common.mustCall(()=>{
+setImmediate(common.mustCall(()=>{
+constlistenerCountAfterClose=process.listenerCount('newListener');
+assert.strictEqual(listenerCountAfterClose,listenerCountBeforeClose-1);
+assert.strictEqual(listenerCountAfterClose,originalProcessNewListenerCount);
+}));
+}));