timers: setInterval interval includes cb duration

setInterval callback should be scheduled on the interval

Fixes: nodejs#7346

PR-URL: nodejs#14815
Fixes: nodejs#7346
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>

lib/timers.js

@@ -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;
 
@@ -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;
 
@@ -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);
+    }
   }
 }
 

test/sequential/test-timers-setInterval-excludes-callback-duration.js

@@ -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);