timers: fix multipleResolves in promisified timeouts/immediates

lundibundi · ronag · commit 64d22c320c67 · 2020-06-21T17:26:36.000+02:00
After successful timer finish the abort event callback would still reject (already resolved promise) upon calling abortController.abort(). Signed-off-by: Denys Otrishko <shishugi@gmail.com> PR-URL: #33949 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/lib/timers.js b/lib/timers.js
@@ -190,8 +190,10 @@ setTimeout[customPromisify] = function(after, value, options = {}) {
     insert(timeout, timeout._idleTimeout);
     if (signal) {
       signal.addEventListener('abort', () => {
-        clearTimeout(timeout);
-        reject(lazyDOMException('AbortError'));
+        if (!timeout._destroyed) {
+          clearTimeout(timeout);
+          reject(lazyDOMException('AbortError'));
+        }
       }, { once: true });
     }
   });
@@ -340,8 +342,10 @@ setImmediate[customPromisify] = function(value, options = {}) {
     const immediate = new Immediate(resolve, [value]);
     if (signal) {
       signal.addEventListener('abort', () => {
-        clearImmediate(immediate);
-        reject(lazyDOMException('AbortError'));
+        if (!immediate._destroyed) {
+          clearImmediate(immediate);
+          reject(lazyDOMException('AbortError'));
+        }
       }, { once: true });
     }
   });
diff --git a/test/parallel/test-timers-promisified.js b/test/parallel/test-timers-promisified.js
@@ -10,6 +10,8 @@ const { promisify } = require('util');
 const setTimeout = promisify(timers.setTimeout);
 const setImmediate = promisify(timers.setImmediate);
 
+process.on('multipleResolves', common.mustNotCall());
+
 {
   const promise = setTimeout(1);
   promise.then(common.mustCall((value) => {
@@ -66,6 +68,23 @@ const setImmediate = promisify(timers.setImmediate);
   assert.rejects(setImmediate(10, { signal }), /AbortError/);
 }
 
+{
+  // Check that aborting after resolve will not reject.
+  const ac = new AbortController();
+  const signal = ac.signal;
+  setTimeout(10, undefined, { signal }).then(() => {
+    ac.abort();
+  });
+}
+{
+  // Check that aborting after resolve will not reject.
+  const ac = new AbortController();
+  const signal = ac.signal;
+  setImmediate(10, { signal }).then(() => {
+    ac.abort();
+  });
+}
+
 {
   Promise.all(
     [1, '', false, Infinity].map((i) => assert.rejects(setImmediate(10, i)), {
