From 24846c8562b621c684a250dcbe1e7c88da06cf83 Mon Sep 17 00:00:00 2001
From: Benjamin Gruenbaum <benjamingr@gmail.com>
Date: Fri, 6 Nov 2020 12:59:43 +0200
Subject: [PATCH] events: remove listener after timer is finished

---
 lib/timers/promises.js                   | 13 ++++++++++---
 test/parallel/test-timers-promisified.js | 13 ++++++++++++-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/lib/timers/promises.js b/lib/timers/promises.js
index ef1e6437d4f6ca..eb615d3f0cc60b 100644
--- a/lib/timers/promises.js
+++ b/lib/timers/promises.js
@@ -59,17 +59,24 @@ function setTimeout(after, value, options = {}) {
       lazyDOMException('The operation was aborted', 'AbortError'));
   }
   return new Promise((resolve, reject) => {
-    const timeout = new Timeout(resolve, after, args, false, true);
+    let signalHandler = null;
+    function cleanupSignalHandlerThenResolve(arg) {
+      signal.removeEventListener('abort', signalHandler, { once: true });
+      resolve(arg);
+    }
+    const onDone = signal ? cleanupSignalHandlerThenResolve : resolve;
+    const timeout = new Timeout(onDone, after, args, false, true);
     if (!ref) timeout.unref();
     insert(timeout, timeout._idleTimeout);
     if (signal) {
-      signal.addEventListener('abort', () => {
+      signalHandler = () => {
         if (!timeout._destroyed) {
           // eslint-disable-next-line no-undef
           clearTimeout(timeout);
           reject(lazyDOMException('The operation was aborted', 'AbortError'));
         }
-      }, { once: true });
+      };
+      signal.addEventListener('abort', signalHandler, { once: true });
     }
   });
 }
diff --git a/test/parallel/test-timers-promisified.js b/test/parallel/test-timers-promisified.js
index bd52d2166cf88a..4b0e410c43a2c7 100644
--- a/test/parallel/test-timers-promisified.js
+++ b/test/parallel/test-timers-promisified.js
@@ -1,4 +1,4 @@
-// Flags: --no-warnings
+// Flags: --no-warnings --expose-internals
 'use strict';
 const common = require('../common');
 const assert = require('assert');
@@ -6,6 +6,9 @@ const timers = require('timers');
 const { promisify } = require('util');
 const child_process = require('child_process');
 
+// TODO(benjamingr) - refactor to use getEventListeners when #35991 lands
+const { NodeEventTarget } = require('internal/event_target');
+
 const timerPromises = require('timers/promises');
 
 /* eslint-disable no-restricted-syntax */
@@ -91,6 +94,14 @@ process.on('multipleResolves', common.mustNotCall());
     ac.abort();
   });
 }
+{
+  // Check that timer adding signals does not leak handlers
+  const abortEmitter = new NodeEventTarget();
+  abortEmitter.aborted = false;
+  setTimeout(0, null, { signal: abortEmitter }).then(common.mustCall(() => {
+    assert.strictEqual(abortEmitter.listenerCount('abort'), 0);
+  }));
+}
 
 {
   Promise.all(