timers: fix multipleResolves in promisified timeouts/immediates · nodejs/node@59a8425 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 632
• Pull requests 754
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 59a8425

Browse filesBrowse the repository at this point in the historyBrowse files

lundibundi

authored and

targos

committed

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 <shishugi@gmail.com> PR-URL: #33949 Backport-PR-URL: #38386 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

1 parent 84b2863 commit 59a8425
Copy full SHA for 59a8425

2 files changed

+27-4Lines changed: 27 additions & 4 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• lib
  • timers.js
• test/parallel
  • test-timers-promisified.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎lib/timers.js‎

Copy file name to clipboardExpand all lines: lib/timers.js

+8-4Lines changed: 8 additions & 4 deletions

Original file line number	Diff line number	Diff line change
@@ -201,8 +201,10 @@ setTimeout[customPromisify] = function(after, value, options = {}) {
201	201	insert(timeout, timeout._idleTimeout);
202	202	if (signal) {
203	203	signal.addEventListener('abort', () => {
204		- clearTimeout(timeout);
205		- reject(lazyDOMException('AbortError'));
	204	+ if (!timeout._destroyed) {
	205	+ clearTimeout(timeout);
	206	+ reject(lazyDOMException('AbortError'));
	207	+ }
206	208	}, { once: true });
207	209	}
208	210	});
@@ -368,8 +370,10 @@ setImmediate[customPromisify] = function(value, options = {}) {
368	370	const immediate = new Immediate(resolve, [value]);
369	371	if (signal) {
370	372	signal.addEventListener('abort', () => {
371		- clearImmediate(immediate);
372		- reject(lazyDOMException('AbortError'));
	373	+ if (!immediate._destroyed) {
	374	+ clearImmediate(immediate);
	375	+ reject(lazyDOMException('AbortError'));
	376	+ }
373	377	}, { once: true });
374	378	}
375	379	});

Collapse file

‎test/parallel/test-timers-promisified.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-timers-promisified.js

+19Lines changed: 19 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -10,6 +10,8 @@ const { promisify } = require('util');
10	10	const setTimeout = promisify(timers.setTimeout);
11	11	const setImmediate = promisify(timers.setImmediate);
12	12
	13	+ process.on('multipleResolves', common.mustNotCall());
	14	+
13	15	{
14	16	const promise = setTimeout(1);
15	17	promise.then(common.mustCall((value) => {
@@ -66,6 +68,23 @@ const setImmediate = promisify(timers.setImmediate);
66	68	assert.rejects(setImmediate(10, { signal }), /AbortError/);
67	69	}
68	70
	71	+ {
	72	+ // Check that aborting after resolve will not reject.
	73	+ const ac = new AbortController();
	74	+ const signal = ac.signal;
	75	+ setTimeout(10, undefined, { signal }).then(() => {
	76	+ ac.abort();
	77	+ });
	78	+ }
	79	+ {
	80	+ // Check that aborting after resolve will not reject.
	81	+ const ac = new AbortController();
	82	+ const signal = ac.signal;
	83	+ setImmediate(10, { signal }).then(() => {
	84	+ ac.abort();
	85	+ });
	86	+ }
	87	+
69	88	{
70	89	Promise.all(
71	90	[1, '', false, Infinity].map((i) => assert.rejects(setImmediate(10, i)), {

0 commit comments

Comments

0 (0)