timers: fix multipleResolves in promisified timeouts/immediates

After successful timer finish the abort event callback would still reject (already resolved promise) upon calling abortController.abort(). Signed-off-by: Denys Otrishko <[email protected]> PR-URL: #33949 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Jun 21, 2020 · 64d22c3 · 64d22c3
1 parent 51a2df4
commit 64d22c3
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 4 deletions.
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)), {