Skip to content

Commit

Permalink
timers: fix multipleResolves in promisified timeouts/immediates
Browse files Browse the repository at this point in the history
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: nodejs#33949
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
lundibundi authored and targos committed Apr 30, 2021
1 parent c86e131 commit 290ce9f
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
12 changes: 8 additions & 4 deletions lib/timers.js
Expand Up @@ -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 });
}
});
Expand Down Expand Up @@ -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 });
}
});
Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-timers-promisified.js
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)), {
Expand Down

0 comments on commit 290ce9f

Please sign in to comment.