Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 77e4f19

Browse filesBrowse files
jasnelldanielleadams
authored andcommitted
timers: cleanup abort listener on awaitable timers
Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com> Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #36006 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 567f8d8 commit 77e4f19
Copy full SHA for 77e4f19

File tree

Expand file treeCollapse file tree

2 files changed

+48
-17
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+48
-17
lines changed
Open diff view settings
Collapse file

‎lib/timers/promises.js‎

Copy file name to clipboardExpand all lines: lib/timers/promises.js
+26-16Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
Promise,
5+
PromisePrototypeFinally,
56
PromiseReject,
67
} = primordials;
78

@@ -24,6 +25,13 @@ const lazyDOMException = hideStackFrames((message, name) => {
2425
return new DOMException(message, name);
2526
});
2627

28+
function cancelListenerHandler(clear, reject) {
29+
if (!this._destroyed) {
30+
clear(this);
31+
reject(lazyDOMException('The operation was aborted', 'AbortError'));
32+
}
33+
}
34+
2735
function setTimeout(after, value, options = {}) {
2836
const args = value !== undefined ? [value] : value;
2937
if (options == null || typeof options !== 'object') {
@@ -58,20 +66,21 @@ function setTimeout(after, value, options = {}) {
5866
return PromiseReject(
5967
lazyDOMException('The operation was aborted', 'AbortError'));
6068
}
61-
return new Promise((resolve, reject) => {
69+
let oncancel;
70+
const ret = new Promise((resolve, reject) => {
6271
const timeout = new Timeout(resolve, after, args, false, true);
6372
if (!ref) timeout.unref();
6473
insert(timeout, timeout._idleTimeout);
6574
if (signal) {
66-
signal.addEventListener('abort', () => {
67-
if (!timeout._destroyed) {
68-
// eslint-disable-next-line no-undef
69-
clearTimeout(timeout);
70-
reject(lazyDOMException('The operation was aborted', 'AbortError'));
71-
}
72-
}, { once: true });
75+
// eslint-disable-next-line no-undef
76+
oncancel = cancelListenerHandler.bind(timeout, clearTimeout, reject);
77+
signal.addEventListener('abort', oncancel);
7378
}
7479
});
80+
return oncancel !== undefined ?
81+
PromisePrototypeFinally(
82+
ret,
83+
() => signal.removeEventListener('abort', oncancel)) : ret;
7584
}
7685

7786
function setImmediate(value, options = {}) {
@@ -107,19 +116,20 @@ function setImmediate(value, options = {}) {
107116
return PromiseReject(
108117
lazyDOMException('The operation was aborted', 'AbortError'));
109118
}
110-
return new Promise((resolve, reject) => {
119+
let oncancel;
120+
const ret = new Promise((resolve, reject) => {
111121
const immediate = new Immediate(resolve, [value]);
112122
if (!ref) immediate.unref();
113123
if (signal) {
114-
signal.addEventListener('abort', () => {
115-
if (!immediate._destroyed) {
116-
// eslint-disable-next-line no-undef
117-
clearImmediate(immediate);
118-
reject(lazyDOMException('The operation was aborted', 'AbortError'));
119-
}
120-
}, { once: true });
124+
// eslint-disable-next-line no-undef
125+
oncancel = cancelListenerHandler.bind(immediate, clearImmediate, reject);
126+
signal.addEventListener('abort', oncancel);
121127
}
122128
});
129+
return oncancel !== undefined ?
130+
PromisePrototypeFinally(
131+
ret,
132+
() => signal.removeEventListener('abort', oncancel)) : ret;
123133
}
124134

125135
module.exports = {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-timers-promisified.js
+22-1Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
1-
// Flags: --no-warnings
1+
// Flags: --no-warnings --expose-internals
22
'use strict';
33
const common = require('../common');
44
const assert = require('assert');
55
const timers = require('timers');
66
const { promisify } = require('util');
77
const child_process = require('child_process');
88

9+
// TODO(benjamingr) - refactor to use getEventListeners when #35991 lands
10+
const { NodeEventTarget } = require('internal/event_target');
11+
912
const timerPromises = require('timers/promises');
1013

1114
/* eslint-disable no-restricted-syntax */
@@ -92,6 +95,24 @@ process.on('multipleResolves', common.mustNotCall());
9295
});
9396
}
9497

98+
{
99+
// Check that timer adding signals does not leak handlers
100+
const signal = new NodeEventTarget();
101+
signal.aborted = false;
102+
setTimeout(0, null, { signal }).finally(common.mustCall(() => {
103+
assert.strictEqual(signal.listenerCount('abort'), 0);
104+
}));
105+
}
106+
107+
{
108+
// Check that timer adding signals does not leak handlers
109+
const signal = new NodeEventTarget();
110+
signal.aborted = false;
111+
setImmediate(0, { signal }).finally(common.mustCall(() => {
112+
assert.strictEqual(signal.listenerCount('abort'), 0);
113+
}));
114+
}
115+
95116
{
96117
Promise.all(
97118
[1, '', false, Infinity].map((i) => assert.rejects(setImmediate(10, i)), {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.