From 1385e1bc63665b8c226e9f7bc8c46a9263195efc Mon Sep 17 00:00:00 2001
From: zhangzifa <tonyzzf@163.com>
Date: Mon, 14 Aug 2017 09:59:15 +0800
Subject: [PATCH] timers: setInterval interval includes cb duration

setInterval callback should be scheduled on the interval

Fixes: https://github.com/nodejs/node/issues/7346

PR-URL: https://github.com/nodejs/node/pull/14815
Fixes: https://github.com/nodejs/node/issues/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>
---
 lib/timers.js                                 | 23 +++++++++++++++----
 ...-setInterval-excludes-callback-duration.js | 18 +++++++++++++++
 2 files changed, 36 insertions(+), 5 deletions(-)
 create mode 100644 test/sequential/test-timers-setInterval-excludes-callback-duration.js

diff --git a/lib/timers.js b/lib/timers.js
index 643731b105eb79..8a74bcf2de7b11 100644
--- a/lib/timers.js
+++ b/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);
+    }
   }
 }
 
diff --git a/test/sequential/test-timers-setInterval-excludes-callback-duration.js b/test/sequential/test-timers-setInterval-excludes-callback-duration.js
new file mode 100644
index 00000000000000..662a5055b70df9
--- /dev/null
+++ b/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);