timers: cleanup abort listener on awaitable timers

Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com>
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #36006
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: jasnell

lib/internal/timers/promises.js

@@ -2,6 +2,7 @@
 
 const{
  Promise,
+ PromisePrototypeFinally,
  PromiseReject,
 }=primordials;
 
@@ -26,6 +27,13 @@ const lazyDOMException = hideStackFrames((message, name) => {
 returnnewDOMException(message,name);
 });
 
+functioncancelListenerHandler(clear,reject){
+if(!this._destroyed){
+clear(this);
+reject(lazyDOMException('The operation was aborted','AbortError'));
+}
+}
+
 functionsetTimeout(after,value,options={}){
 constargs=value!==undefined ? [value] : value;
 if(options==null||typeofoptions!=='object'){
@@ -55,20 +63,21 @@ function setTimeout(after, value, options = {}) {
 returnPromiseReject(
 lazyDOMException('The operation was aborted','AbortError'));
 }
-returnnewPromise((resolve,reject)=>{
+letoncancel;
+constret=newPromise((resolve,reject)=>{
 consttimeout=newTimeout(resolve,after,args,false,true);
 if(!ref)timeout.unref();
 insert(timeout,timeout._idleTimeout);
 if(signal){
-signal.addEventListener('abort',()=>{
-if(!timeout._destroyed){
-// eslint-disable-next-line no-undef
-clearTimeout(timeout);
-reject(lazyDOMException('The operation was aborted','AbortError'));
-}
-},{once: true});
+// eslint-disable-next-line no-undef
+oncancel=cancelListenerHandler.bind(timeout,clearTimeout,reject);
+signal.addEventListener('abort',oncancel);
 }
 });
+returnoncancel!==undefined ?
+PromisePrototypeFinally(
+ret,
+()=>signal.removeEventListener('abort',oncancel)) : ret;
 }
 
 functionsetImmediate(value,options={}){
@@ -99,19 +108,20 @@ function setImmediate(value, options = {}) {
 returnPromiseReject(
 lazyDOMException('The operation was aborted','AbortError'));
 }
-returnnewPromise((resolve,reject)=>{
+letoncancel;
+constret=newPromise((resolve,reject)=>{
 constimmediate=newImmediate(resolve,[value]);
 if(!ref)immediate.unref();
 if(signal){
-signal.addEventListener('abort',()=>{
-if(!immediate._destroyed){
-// eslint-disable-next-line no-undef
-clearImmediate(immediate);
-reject(lazyDOMException('The operation was aborted','AbortError'));
-}
-},{once: true});
+// eslint-disable-next-line no-undef
+oncancel=cancelListenerHandler.bind(immediate,clearImmediate,reject);
+signal.addEventListener('abort',oncancel);
 }
 });
+returnoncancel!==undefined ?
+PromisePrototypeFinally(
+ret,
+()=>signal.removeEventListener('abort',oncancel)) : ret;
 }
 
 module.exports={

test/parallel/test-timers-promisified.js

@@ -1,10 +1,13 @@
-// Flags: --no-warnings --experimental-abortcontroller
+// Flags: --no-warnings --expose-internals --experimental-abortcontroller
 'use strict';
 constcommon=require('../common');
 constassert=require('assert');
 consttimers=require('timers');
 const{ promisify }=require('util');
 
+// TODO(benjamingr) - refactor to use getEventListeners when #35991 lands
+const{ NodeEventTarget }=require('internal/event_target');
+
 /* eslint-disable no-restricted-syntax */
 
 constsetTimeout=promisify(timers.setTimeout);
@@ -85,6 +88,24 @@ process.on('multipleResolves', common.mustNotCall());
 });
 }
 
+{
+// Check that timer adding signals does not leak handlers
+constsignal=newNodeEventTarget();
+signal.aborted=false;
+setTimeout(0,null,{ signal }).finally(common.mustCall(()=>{
+assert.strictEqual(signal.listenerCount('abort'),0);
+}));
+}
+
+{
+// Check that timer adding signals does not leak handlers
+constsignal=newNodeEventTarget();
+signal.aborted=false;
+setImmediate(0,{ signal }).finally(common.mustCall(()=>{
+assert.strictEqual(signal.listenerCount('abort'),0);
+}));
+}
+
 {
 Promise.all(
 [1,'',false,Infinity].map((i)=>assert.rejects(setImmediate(10,i)),{