From c6b436c05f737b8cbe390f7efd7d638fadeffeed Mon Sep 17 00:00:00 2001 From: Fabian Cook Date: Wed, 28 Oct 2020 14:11:43 +1300 Subject: [PATCH 1/3] timers: introduce setInterval async iterator Utilises Symbol.asyncIterator to provide an iterator compatible with `for await` ``` const { setInterval } = require('timers/promises'); const ac = new AbortController() const signal = ac.signal setTimeout(() => ac.abort(), 500) for await (const value of setInterval(interval, null, { ref: false, signal })) { } ``` --- doc/api/timers.md | 13 +++ lib/timers.js | 9 ++ lib/timers/promises.js | 105 +++++++++++++++++++ test/parallel/test-timers-promisified.js | 125 ++++++++++++++++++++++- 4 files changed, 250 insertions(+), 2 deletions(-) diff --git a/doc/api/timers.md b/doc/api/timers.md index 108c102db51383..f1b69c754bd197 100644 --- a/doc/api/timers.md +++ b/doc/api/timers.md @@ -363,6 +363,19 @@ added: v15.0.0 * `signal` {AbortSignal} An optional `AbortSignal` that can be used to cancel the scheduled `Immediate`. +### `timersPromises.setInterval([delay[, value[, options]]])` + +* `delay` {number} The number of milliseconds to wait between iterations. + **Default**: `1`. +* `value` {any} A value with which the iterator returns. +* `options` {Object} + * `ref` {boolean} Set to `false` to indicate that the scheduled `Timeout` + between iterations should not require the Node.js event loop to + remain active. + **Default**: `true`. + * `signal` {AbortSignal} An optional `AbortSignal` that can be used to + cancel the scheduled `Timeout` between operations. + [Event Loop]: https://nodejs.org/en/docs/guides/event-loop-timers-and-nexttick/#setimmediate-vs-settimeout [`AbortController`]: globals.md#globals_class_abortcontroller [`TypeError`]: errors.md#errors_class_typeerror diff --git a/lib/timers.js b/lib/timers.js index 6cb64b98b5cdc0..50b84b84b13e20 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -215,6 +215,15 @@ function setInterval(callback, repeat, arg1, arg2, arg3) { return timeout; } +ObjectDefineProperty(setInterval, customPromisify, { + enumerable: true, + get() { + if (!timersPromises) + timersPromises = require('timers/promises'); + return timersPromises.setInterval; + } +}); + function clearInterval(timer) { // clearTimeout and clearInterval can be used to clear timers created from // both setTimeout and setInterval, as specified by HTML Living Standard: diff --git a/lib/timers/promises.js b/lib/timers/promises.js index 76713bcf603342..569325d64158e0 100644 --- a/lib/timers/promises.js +++ b/lib/timers/promises.js @@ -1,9 +1,11 @@ 'use strict'; const { + Symbol, FunctionPrototypeBind, Promise, PromisePrototypeFinally, + PromiseResolve, PromiseReject, } = primordials; @@ -125,7 +127,110 @@ function setImmediate(value, options = {}) { () => signal.removeEventListener('abort', oncancel)) : ret; } +function setInterval(after, value, options = {}) { + const args = value !== undefined ? [value] : value; + if (options == null || typeof options !== 'object') { + throw new ERR_INVALID_ARG_TYPE( + 'options', + 'Object', + options); + } + const { signal, ref = true } = options; + if (signal !== undefined && + (signal === null || + typeof signal !== 'object' || + !('aborted' in signal))) { + throw new ERR_INVALID_ARG_TYPE( + 'options.signal', + 'AbortSignal', + signal); + } + if (typeof ref !== 'boolean') { + throw new ERR_INVALID_ARG_TYPE( + 'options.ref', + 'boolean', + ref); + } + return { + [Symbol.asyncIterator]() { + let timeout, + deferred; + let active = true; + const iterator = { + next() { + if (!active) { + return PromiseResolve({ + done: true, + value: undefined + }); + } + // TODO(@jasnell): If a decision is made that this cannot + // be backported to 12.x, then this can be converted to + // use optional chaining to simplify the check. + if (signal && signal.aborted) { + return PromiseReject( + lazyDOMException('The operation was aborted', 'AbortError') + ); + } + const promise = new Promise( + (resolve, reject) => { + deferred = { resolve, reject }; + } + ); + timeout = new Timeout((value) => { + if (deferred) { + deferred.resolve({ + done: false, + value + }); + deferred = undefined; + } + }, after, args, false, true); + if (!ref) timeout.unref(); + insert(timeout, timeout._idleTimeout); + return promise; + }, + return() { + active = false; + if (timeout) { + // eslint-disable-next-line no-undef + clearTimeout(timeout); + } + if (deferred) { + deferred.resolve({ + done: true, + value: undefined + }); + deferred = undefined; + } + if (signal) { + signal.removeEventListener('abort', onAbort); + } + return PromiseResolve({ + done: true, + value: undefined + }); + } + }; + if (signal) { + signal.addEventListener('abort', onAbort, { once: true }); + } + return iterator; + + function onAbort() { + if (deferred) { + deferred.reject( + lazyDOMException('The operation was aborted', 'AbortError') + ); + deferred = undefined; + } + } + } + }; +} + module.exports = { setTimeout, setImmediate, + setInterval, }; diff --git a/test/parallel/test-timers-promisified.js b/test/parallel/test-timers-promisified.js index be73984b4fa602..4758bc12b1a158 100644 --- a/test/parallel/test-timers-promisified.js +++ b/test/parallel/test-timers-promisified.js @@ -15,10 +15,12 @@ const timerPromises = require('timers/promises'); const setTimeout = promisify(timers.setTimeout); const setImmediate = promisify(timers.setImmediate); +const setInterval = promisify(timers.setInterval); const exec = promisify(child_process.exec); assert.strictEqual(setTimeout, timerPromises.setTimeout); assert.strictEqual(setImmediate, timerPromises.setImmediate); +assert.strictEqual(setInterval, timerPromises.setInterval); process.on('multipleResolves', common.mustNotCall()); @@ -50,6 +52,50 @@ process.on('multipleResolves', common.mustNotCall()); })); } +{ + const iterable = setInterval(1, 'foobar'); + const iterator = iterable[Symbol.asyncIterator](); + const promise = iterator.next(); + promise + .then(common.mustCall((result) => { + assert.ok(!result.done); + assert.strictEqual(result.value, 'foobar'); + return iterator.return(); + })) + .then(common.mustCall()); +} + +{ + const iterable = setInterval(1); + const iterator = iterable[Symbol.asyncIterator](); + const promise = iterator.next(); + promise + .then(common.mustCall((result) => { + assert.ok(!result.done); + assert.strictEqual(result.value, undefined); + return iterator.return(); + })) + .then(common.mustCall()); +} + +{ + const iterable = setInterval(1, 'foobar'); + const iterator = iterable[Symbol.asyncIterator](); + const promise = iterator.next(); + promise + .then(common.mustCall((result) => { + assert.ok(!result.done); + assert.strictEqual(result.value, 'foobar'); + return iterator.next(); + })) + .then(common.mustCall((result) => { + assert.ok(!result.done); + assert.strictEqual(result.value, 'foobar'); + return iterator.return(); + })) + .then(common.mustCall()); +} + { const ac = new AbortController(); const signal = ac.signal; @@ -78,6 +124,33 @@ process.on('multipleResolves', common.mustNotCall()); assert.rejects(setImmediate(10, { signal }), /AbortError/); } +{ + const ac = new AbortController(); + const signal = ac.signal; + ac.abort(); // Abort in advance + + const iterable = setInterval(1, undefined, { signal }); + const iterator = iterable[Symbol.asyncIterator](); + + assert.rejects(iterator.next(), /AbortError/); +} + +{ + const ac = new AbortController(); + const signal = ac.signal; + + const iterable = setInterval(100, undefined, { signal }); + const iterator = iterable[Symbol.asyncIterator](); + + // This promise should take 100 seconds to resolve, so now aborting it should + // mean we abort early + const promise = iterator.next(); + + ac.abort(); // Abort in after we have a next promise + + assert.rejects(promise, /AbortError/); +} + { // Check that aborting after resolve will not reject. const ac = new AbortController(); @@ -148,6 +221,10 @@ process.on('multipleResolves', common.mustNotCall()); (ref) => assert.rejects(setTimeout(10, null, { ref })), { code: 'ERR_INVALID_ARG_TYPE' })).then(common.mustCall()); + + [1, '', Infinity, null, {}].forEach((ref) => { + assert.throws(() => setInterval(10, undefined, { ref })); + }); } { @@ -160,8 +237,52 @@ process.on('multipleResolves', common.mustNotCall()); { exec(`${process.execPath} -pe "const assert = require('assert');` + - 'require(\'timers/promises\').setImmediate(null, { ref: false }).' + - 'then(assert.fail)"').then(common.mustCall(({ stderr }) => { + 'require(\'timers/promises\').setImmediate(null, { ref: false }).' + + 'then(assert.fail)"').then(common.mustCall(({ stderr }) => { + assert.strictEqual(stderr, ''); + })); +} + +{ + exec(`${process.execPath} -pe "const assert = require('assert');` + + 'const interval = require(\'timers/promises\')' + + '.setInterval(1000, null, { ref: false });' + + 'interval[Symbol.asyncIterator]().next()' + + '.then(assert.fail)"').then(common.mustCall(({ stderr }) => { assert.strictEqual(stderr, ''); })); } + +{ + const ac = new AbortController(); + const input = 'foobar'; + const signal = ac.signal; + + const mainInterval = 5; + const loopInterval = mainInterval * 1.5; + + const interval = setInterval(mainInterval, input, { signal }); + + async function runInterval(fn) { + const times = []; + for await (const value of interval) { + const index = times.length; + times[index] = [Date.now()]; + assert.strictEqual(value, input); + await fn(); + times[index] = [...times[index], Date.now()]; + } + } + + const noopLoop = runInterval(() => {}); + const timeoutLoop = runInterval(() => setTimeout(loopInterval)); + + // Let it loop 5 times, then abort before the next + setTimeout(Math.floor(loopInterval * 5.5)).then(common.mustCall(() => { + ac.abort(); + })); + + assert.rejects(noopLoop, /AbortError/); + assert.rejects(timeoutLoop, /AbortError/); + +} From e5496f2022b2f9c7e45b59514fe1497f2b051762 Mon Sep 17 00:00:00 2001 From: Fabian Cook Date: Thu, 29 Oct 2020 20:45:29 +1300 Subject: [PATCH 2/3] fixup! timers: introduce setInterval async iterator Remove custom promisify from setInterval --- lib/timers.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 50b84b84b13e20..6cb64b98b5cdc0 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -215,15 +215,6 @@ function setInterval(callback, repeat, arg1, arg2, arg3) { return timeout; } -ObjectDefineProperty(setInterval, customPromisify, { - enumerable: true, - get() { - if (!timersPromises) - timersPromises = require('timers/promises'); - return timersPromises.setInterval; - } -}); - function clearInterval(timer) { // clearTimeout and clearInterval can be used to clear timers created from // both setTimeout and setInterval, as specified by HTML Living Standard: From b71c61f983f599288cb02bd72f12c160a1c98c40 Mon Sep 17 00:00:00 2001 From: Fabian Cook Date: Fri, 18 Dec 2020 01:12:24 +1300 Subject: [PATCH 3/3] fixup! timers: introduce setInterval async iterator --- This is still WIP and I will replace this commit with more progress, currently waiting for the branch to build locally.. --- --- lib/timers/promises.js | 156 ++++++++++++++++------- test/parallel/test-timers-promisified.js | 4 +- 2 files changed, 110 insertions(+), 50 deletions(-) diff --git a/lib/timers/promises.js b/lib/timers/promises.js index 569325d64158e0..190cf06e2b4dc6 100644 --- a/lib/timers/promises.js +++ b/lib/timers/promises.js @@ -17,7 +17,7 @@ const { const { AbortError, - codes: { ERR_INVALID_ARG_TYPE } + codes: { ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE } } = require('internal/errors'); function cancelListenerHandler(clear, reject) { @@ -135,7 +135,17 @@ function setInterval(after, value, options = {}) { 'Object', options); } - const { signal, ref = true } = options; + const { + signal, + ref = true, + // Defers start of timers until the first iteration + wait = false, + // This has each invocation of iterator.next set up a new timeout + timeout: asTimeout = false, + // Skips intervals that are missed + skip = false + // Clears entire queue of callbacks when skip = true and the callbacks well missed the timeout + } = options; if (signal !== undefined && (signal === null || typeof signal !== 'object' || @@ -153,78 +163,128 @@ function setInterval(after, value, options = {}) { } return { [Symbol.asyncIterator]() { + // asTimeout can't skip as they always have intervals between each iteration + const resolveEarlyEnabled = !asTimeout && skip; let timeout, - deferred; - let active = true; + callbacks = [], + active = true, + missed = 0; + + setIntervalCycle(); + const iterator = { - next() { + async next() { if (!active) { - return PromiseResolve({ + return { done: true, value: undefined - }); + }; } - // TODO(@jasnell): If a decision is made that this cannot - // be backported to 12.x, then this can be converted to - // use optional chaining to simplify the check. + // TODO(@jasnell): If a decision is made that this cannot be backported + // to 12.x, then this can be converted to use optional chaining to + // simplify the check. if (signal && signal.aborted) { - return PromiseReject( - lazyDOMException('The operation was aborted', 'AbortError') - ); + return PromiseReject(new AbortError()); } - const promise = new Promise( + return new Promise( (resolve, reject) => { - deferred = { resolve, reject }; + callbacks.push({ resolve, reject }); + if (missed > 0) { + resolveNext(); + } + setIntervalCycle(); } ); - timeout = new Timeout((value) => { - if (deferred) { - deferred.resolve({ - done: false, - value - }); - deferred = undefined; - } - }, after, args, false, true); - if (!ref) timeout.unref(); - insert(timeout, timeout._idleTimeout); - return promise; }, - return() { + async return() { active = false; - if (timeout) { - // eslint-disable-next-line no-undef - clearTimeout(timeout); - } - if (deferred) { - deferred.resolve({ - done: true, - value: undefined - }); - deferred = undefined; - } + clear(); + resolveAll({ + done: true, + value: undefined + }); if (signal) { - signal.removeEventListener('abort', onAbort); + signal.removeEventListener('abort', oncancel); } - return PromiseResolve({ + return { done: true, value: undefined - }); + }; } }; if (signal) { - signal.addEventListener('abort', onAbort, { once: true }); + signal.addEventListener('abort', oncancel, { once: true }); } return iterator; - function onAbort() { + function setIntervalCycle() { + if (!active) { + return; + } + if (timeout) { + return; + } + // Wait and asTimeout both imply a callback is required before setting up a timeout + if (!callbacks.length && (wait || asTimeout)) { + return; + } + missed = 0; + const currentTimeout = timeout = new Timeout(() => { + if (asTimeout && currentTimeout === timeout) { + // No need to clear here as we set to not repeat for asTimeout + timeout = undefined; + } + resolveNext(); + }, after, undefined, !asTimeout, true); + if (!ref) timeout.unref(); + insert(timeout, timeout._idleTimeout); + } + + function resolveNext() { + if (!callbacks.length) { + if (resolveEarlyEnabled) { + missed += 1; + } + return; + } + const deferred = callbacks.shift(); if (deferred) { - deferred.reject( - lazyDOMException('The operation was aborted', 'AbortError') - ); - deferred = undefined; + const { resolve } = deferred; + resolve({ + done: false, + value + }); + missed -= 1; + } + if (missed > 0 && callbacks.length) { + // Loop till we have completed each missed interval that we have a callback for + resolveNext(); + } + } + + function resolveAll(value) { + callbacks.forEach(({ resolve }) => resolve(value)); + callbacks = []; + } + + function rejectAll(error) { + callbacks.forEach(({ reject }) => reject(error)); + callbacks = []; + } + + function clear() { + if (timeout) { + // eslint-disable-next-line no-undef + clearTimeout(timeout); + timeout = undefined; } } + + function oncancel() { + clear(); + rejectAll(new AbortError()); + } + } }; } diff --git a/test/parallel/test-timers-promisified.js b/test/parallel/test-timers-promisified.js index 4758bc12b1a158..a552289620f0f8 100644 --- a/test/parallel/test-timers-promisified.js +++ b/test/parallel/test-timers-promisified.js @@ -261,7 +261,7 @@ process.on('multipleResolves', common.mustNotCall()); const mainInterval = 5; const loopInterval = mainInterval * 1.5; - const interval = setInterval(mainInterval, input, { signal }); + const interval = setInterval(mainInterval, input, { signal, timeout: true }); async function runInterval(fn) { const times = []; @@ -278,7 +278,7 @@ process.on('multipleResolves', common.mustNotCall()); const timeoutLoop = runInterval(() => setTimeout(loopInterval)); // Let it loop 5 times, then abort before the next - setTimeout(Math.floor(loopInterval * 5.5)).then(common.mustCall(() => { + setTimeout(Math.floor(loopInterval * 5.5), undefined, { timeout: true }).then(common.mustCall(() => { ac.abort(); }));