From 290ce9f463b0569b37655660a8f97a6a9fcd9001 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Thu, 18 Jun 2020 22:58:42 +0300 Subject: [PATCH] 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 PR-URL: https://github.com/nodejs/node/pull/33949 Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Reviewed-By: Anna Henningsen --- lib/timers.js | 12 ++++++++---- test/parallel/test-timers-promisified.js | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 3dbdeb433e4430..60e8bc5c0d073d 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -201,8 +201,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 }); } }); @@ -368,8 +370,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 index 6fbf6157603f2c..286a7fd8965d36 100644 --- 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)), {