Skip to content

Commit d28af81

Browse files
committed
test_runner: cancel abort listener when promisified timer settles
1 parent 942bc29 commit d28af81

File tree

2 files changed

+40
-32
lines changed

2 files changed

+40
-32
lines changed

lib/internal/test_runner/mock/mock_timers.js

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const {
1313
ObjectDefineProperty,
1414
ObjectGetOwnPropertyDescriptor,
1515
ObjectGetOwnPropertyDescriptors,
16-
Promise,
16+
PromiseWithResolvers,
1717
Symbol,
1818
SymbolDispose,
1919
globalThis,
@@ -42,7 +42,6 @@ const nodeTimers = require('timers');
4242
const nodeTimersPromises = require('timers/promises');
4343
const EventEmitter = require('events');
4444

45-
let kResistStopPropagation;
4645
// Internal reference to the MockTimers class inside MockDate
4746
let kMock;
4847
// Initial epoch to which #now should be set to
@@ -464,38 +463,31 @@ class MockTimers {
464463
);
465464
}
466465

467-
#promisifyTimer({ timerFn, clearFn, ms, result, options }) {
468-
return new Promise((resolve, reject) => {
469-
if (options?.signal) {
470-
try {
471-
validateAbortSignal(options.signal, 'options.signal');
472-
} catch (err) {
473-
return reject(err);
474-
}
475-
476-
if (options.signal.aborted) {
477-
return reject(abortIt(options.signal));
478-
}
479-
}
466+
async #promisifyTimer({ timerFn, clearFn, ms, result, options }) {
467+
const { promise, resolve, reject } = PromiseWithResolvers();
468+
469+
let abortListener;
470+
if (options?.signal) {
471+
validateAbortSignal(options.signal, 'options.signal');
480472

481-
const onabort = () => {
482-
clearFn(timer);
483-
return reject(abortIt(options.signal));
484-
};
485-
486-
const timer = timerFn(() => {
487-
return resolve(result);
488-
}, ms);
489-
490-
if (options?.signal) {
491-
kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
492-
options.signal.addEventListener('abort', onabort, {
493-
__proto__: null,
494-
once: true,
495-
[kResistStopPropagation]: true,
496-
});
473+
if (options.signal.aborted) {
474+
throw abortIt(options.signal);
497475
}
498-
});
476+
477+
abortListener = addAbortListener(options.signal, () => {
478+
reject(abortIt(options.signal));
479+
});
480+
}
481+
482+
const timer = timerFn(resolve, ms);
483+
484+
try {
485+
await promise;
486+
return result;
487+
} finally {
488+
abortListener?.[SymbolDispose]();
489+
clearFn(timer);
490+
}
499491
}
500492

501493
#setImmediatePromisified(result, options) {

test/parallel/test-runner-mock-timers.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,22 @@ describe('Mock Timers Test Suite', () => {
518518
});
519519
});
520520

521+
it('should clear the abort listener when the timer resolves', async (t) => {
522+
t.mock.timers.enable({ apis: ['setTimeout'] });
523+
const expectedResult = 'result';
524+
const controller = new AbortController();
525+
const p = nodeTimersPromises.setTimeout(500, expectedResult, {
526+
ref: true,
527+
signal: controller.signal,
528+
});
529+
530+
assert(hasAbortListener(controller.signal));
531+
532+
t.mock.timers.tick(500);
533+
await p;
534+
assert(!hasAbortListener(controller.signal));
535+
});
536+
521537
it('should reject given an an invalid signal instance', async (t) => {
522538
t.mock.timers.enable({ apis: ['setTimeout'] });
523539
const expectedResult = 'result';

0 commit comments

Comments
 (0)