Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timers: truncate decimal values #24819

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc/api/timers.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ added: v0.0.1
Schedules repeated execution of `callback` every `delay` milliseconds.

When `delay` is larger than `2147483647` or less than `1`, the `delay` will be
set to `1`.
set to `1`. Non-integer delays are truncated to an integer.

If `callback` is not a function, a [`TypeError`][] will be thrown.

Expand All @@ -209,7 +209,7 @@ nor of their ordering. The callback will be called as close as possible to the
time specified.

When `delay` is larger than `2147483647` or less than `1`, the `delay`
will be set to `1`.
will be set to `1`. Non-integer delays are truncated to an integer.

If `callback` is not a function, a [`TypeError`][] will be thrown.

Expand Down
9 changes: 7 additions & 2 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,13 @@ exports._unrefActive = function(item) {
// Appends a timer onto the end of an existing timers list, or creates a new
// list if one does not already exist for the specified timeout duration.
function insert(item, refed, start) {
const msecs = item._idleTimeout;
let msecs = item._idleTimeout;
if (msecs < 0 || msecs === undefined)
return;

// Truncate so that accuracy of sub-milisecond timers is not assumed.
msecs = Math.trunc(msecs);

item._idleStart = start;

// Use an existing list if there is one, otherwise we need to make a new one.
Expand Down Expand Up @@ -378,7 +381,9 @@ function unenroll(item) {
// clearTimeout that makes it clear that the list should not be deleted.
// That function could then be used by http and other similar modules.
if (item[kRefed]) {
const list = lists[item._idleTimeout];
// Compliment truncation during insert().
const msecs = Math.trunc(item._idleTimeout);
const list = lists[msecs];
if (list !== undefined && L.isEmpty(list)) {
debug('unenroll: list empty');
queue.removeAt(list.priorityQueuePosition);
Expand Down
35 changes: 35 additions & 0 deletions test/parallel/test-timers-non-integer-delay.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

'use strict';
const common = require('../common');
const assert = require('assert');

/*
* This test makes sure that non-integer timer delays do not make the process
Expand All @@ -47,3 +48,37 @@ const interval = setInterval(common.mustCall(() => {
process.exit(0);
}
}, N), TIMEOUT_DELAY);

// Test non-integer delay ordering
{
const ordering = [];

setTimeout(common.mustCall(() => {
ordering.push(1);
}), 1);

setTimeout(common.mustCall(() => {
ordering.push(2);
}), 1.8);

setTimeout(common.mustCall(() => {
ordering.push(3);
}), 1.1);

setTimeout(common.mustCall(() => {
ordering.push(4);
}), 1);

setTimeout(common.mustCall(() => {
const expected = [1, 2, 3, 4];

assert.deepStrictEqual(
ordering,
expected,
`Non-integer delay ordering should be ${expected}, but got ${ordering}`
);

// 2 should always be last of these delays due to ordering guarentees by
// the implementation.
}), 2);
}