From ca46c3b5dc7d349274c720345a48c5e862b66969 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Fri, 19 Jun 2020 21:31:21 +0300 Subject: [PATCH] timers: allow timers to be used as primitives This allows timers to be matched to numeric Ids and therefore used as keys of an Object, passed and stored without storing the Timer instance. clearTimeout/clearInterval is modified to support numeric/string Ids. Co-authored-by: Bradley Farias Co-authored-by: Anatoli Papirovski Refs: https://github.com/nodejs/node/pull/21152 PR-URL: https://github.com/nodejs/node/pull/34017 Reviewed-By: James M Snell Reviewed-By: Bradley Farias Reviewed-By: Jeremiah Senkpiel Reviewed-By: Anna Henningsen Reviewed-By: Trivikram Kamat Reviewed-By: Yongsheng Zhang Reviewed-By: Benjamin Gruenbaum --- doc/api/timers.md | 16 +++++++++++++ lib/internal/timers.js | 4 ++++ lib/timers.js | 28 ++++++++++++++++++++++ test/parallel/test-timers-to-primitive.js | 29 +++++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 test/parallel/test-timers-to-primitive.js diff --git a/doc/api/timers.md b/doc/api/timers.md index e66cade8950619..4a465ca3cf064d 100644 --- a/doc/api/timers.md +++ b/doc/api/timers.md @@ -123,6 +123,21 @@ Calling `timeout.unref()` creates an internal timer that will wake the Node.js event loop. Creating too many of these can adversely impact performance of the Node.js application. +### `timeout[Symbol.toPrimitive]()` + + +* Returns: {integer} number that can be used to reference this `timeout` + +Coerce a `Timeout` to a primitive, a primitive will be generated that +can be used to clear the `Timeout`. +The generated number can only be used in the same thread where timeout +was created. Therefore to use it cross [`worker_threads`][] it has +to first be passed to a correct thread. +This allows enhanced compatibility with browser's `setTimeout()`, and +`setInterval()` implementations. + ## Scheduling timers A timer in Node.js is an internal construct that calls a given function after @@ -346,3 +361,4 @@ const timersPromises = require('timers/promises'); [`setInterval()`]: timers.html#timers_setinterval_callback_delay_args [`setTimeout()`]: timers.html#timers_settimeout_callback_delay_args [`util.promisify()`]: util.html#util_util_promisify_original +[`worker_threads`]: worker_threads.html diff --git a/lib/internal/timers.js b/lib/internal/timers.js index fc1a23839629ac..d48fd9a893389d 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -104,6 +104,8 @@ const { const async_id_symbol = Symbol('asyncId'); const trigger_async_id_symbol = Symbol('triggerId'); +const kHasPrimitive = Symbol('kHasPrimitive'); + const { ERR_INVALID_CALLBACK, ERR_OUT_OF_RANGE @@ -185,6 +187,7 @@ function Timeout(callback, after, args, isRepeat, isRefed) { if (isRefed) incRefCount(); this[kRefed] = isRefed; + this[kHasPrimitive] = false; initAsyncResource(this, 'Timeout'); } @@ -639,6 +642,7 @@ module.exports = { Timeout, Immediate, kRefed, + kHasPrimitive, initAsyncResource, setUnrefTimeout, getTimerDuration, diff --git a/lib/timers.js b/lib/timers.js index 878f360ab0c04e..f7106c90cbff10 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -22,8 +22,10 @@ 'use strict'; const { + ObjectCreate, MathTrunc, Object, + SymbolToPrimitive } = primordials; const { @@ -41,6 +43,7 @@ const { kRefCount }, kRefed, + kHasPrimitive, getTimerDuration, timerListMap, timerListQueue, @@ -66,6 +69,11 @@ const { emitDestroy } = require('internal/async_hooks'); +// This stores all the known timer async ids to allow users to clearTimeout and +// clearInterval using those ids, to match the spec and the rest of the web +// platform. +const knownTimersById = ObjectCreate(null); + // Remove a timer. Cancels the timeout and resets the relevant timer properties. function unenroll(item) { if (item._destroyed) @@ -73,6 +81,9 @@ function unenroll(item) { item._destroyed = true; + if (item[kHasPrimitive]) + delete knownTimersById[item[async_id_symbol]]; + // Fewer checks may be possible, but these cover everything. if (destroyHooksExist() && item[async_id_symbol] !== undefined) emitDestroy(item[async_id_symbol]); @@ -163,6 +174,14 @@ function clearTimeout(timer) { if (timer && timer._onTimeout) { timer._onTimeout = null; unenroll(timer); + return; + } + if (typeof timer === 'number' || typeof timer === 'string') { + const timerInstance = knownTimersById[timer]; + if (timerInstance !== undefined) { + timerInstance._onTimeout = null; + unenroll(timerInstance); + } } } @@ -208,6 +227,15 @@ Timeout.prototype.close = function() { return this; }; +Timeout.prototype[SymbolToPrimitive] = function() { + const id = this[async_id_symbol]; + if (!this[kHasPrimitive]) { + this[kHasPrimitive] = true; + knownTimersById[id] = this; + } + return id; +}; + function setImmediate(callback, arg1, arg2, arg3) { validateCallback(callback); diff --git a/test/parallel/test-timers-to-primitive.js b/test/parallel/test-timers-to-primitive.js new file mode 100644 index 00000000000000..65f11b91483040 --- /dev/null +++ b/test/parallel/test-timers-to-primitive.js @@ -0,0 +1,29 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +[ + setTimeout(common.mustNotCall(), 1), + setInterval(common.mustNotCall(), 1), +].forEach((timeout) => { + assert.strictEqual(Number.isNaN(+timeout), false); + assert.strictEqual(+timeout, timeout[Symbol.toPrimitive]()); + assert.strictEqual(`${timeout}`, timeout[Symbol.toPrimitive]().toString()); + assert.deepStrictEqual(Object.keys({ [timeout]: timeout }), [`${timeout}`]); + clearTimeout(+timeout); +}); + +{ + // Check that clearTimeout works with number id. + const timeout = setTimeout(common.mustNotCall(), 1); + const id = +timeout; + clearTimeout(id); +} + +{ + // Check that clearTimeout works with string id. + const timeout = setTimeout(common.mustNotCall(), 1); + const id = `${timeout}`; + clearTimeout(id); +}