Skip to content

Commit

Permalink
timers: setInterval interval includes cb duration
Browse files Browse the repository at this point in the history
setInterval callback should be scheduled on the interval

Fixes: nodejs#7346

PR-URL: nodejs#14815
Fixes: nodejs#7346
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
zhangzifa authored and BridgeAR committed Jan 12, 2018
1 parent e8c491a commit 1385e1b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
23 changes: 18 additions & 5 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,15 @@ exports._unrefActive = function(item) {
// Appends a timer onto the end of an existing timers list, or creates a new
// TimerWrap backed list if one does not already exist for the specified timeout
// duration.
function insert(item, unrefed) {
function insert(item, unrefed, start) {
const msecs = item._idleTimeout;
if (msecs < 0 || msecs === undefined) return;

item._idleStart = TimerWrap.now();
if (typeof start === 'number') {
item._idleStart = start;
} else {
item._idleStart = TimerWrap.now();
}

const lists = unrefed === true ? unrefedLists : refedLists;

Expand Down Expand Up @@ -446,16 +450,17 @@ function ontimeout(timer) {
var args = timer._timerArgs;
if (typeof timer._onTimeout !== 'function')
return promiseResolve(timer._onTimeout, args[0]);
const start = TimerWrap.now();
if (!args)
timer._onTimeout();
else
Reflect.apply(timer._onTimeout, timer, args);
if (timer._repeat)
rearm(timer);
rearm(timer, start);
}


function rearm(timer) {
function rearm(timer, start) {
// // Do not re-arm unenroll'd or closed timers.
if (timer._idleTimeout === -1) return;

Expand All @@ -464,7 +469,15 @@ function rearm(timer) {
timer._handle.start(timer._repeat);
} else {
timer._idleTimeout = timer._repeat;
active(timer);

const duration = TimerWrap.now() - start;
if (duration >= timer._repeat) {
// If callback duration >= timer._repeat,
// add 1 ms to avoid blocking eventloop
insert(timer, false, start + duration - timer._repeat + 1);
} else {
insert(timer, false, start);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';
const common = require('../common');
const Timer = process.binding('timer_wrap').Timer;
const assert = require('assert');

let cntr = 0;
let first, second;
const t = setInterval(() => {
common.busyLoop(50);
cntr++;
if (cntr === 1) {
first = Timer.now();
} else if (cntr === 2) {
second = Timer.now();
assert(Math.abs(second - first - 100) < 10);
clearInterval(t);
}
}, 100);

0 comments on commit 1385e1b

Please sign in to comment.