timers: fix processing of nested same delay timers

lib/timers.js

@@ -87,13 +87,35 @@ function listOnTimeout() {
 
   var first;
   while (first = L.peek(list)) {
-    // If the previous iteration caused a timer to be added,
-    // update the value of "now" so that timing computations are
-    // done correctly. See test/simple/test-timers-blocking-callback.js
-    // for more information.
-    if (now < first._monotonicStartTime) {
-      now = Timer.now();
-      debug('now: %d', now);
+    // This handles the case of a timer that was created within a timers
+    // callback with the same timeout value. For instance, when processing the
+    // timer that would call `bar` in such code:
+    //
+    //   setTimeout(function foo() { setTimeout(function bar() {}, 0) }, 0);
+    //
+    // or
+    //
+    //   setTimeout(function foo() { setTimeout(function bar() {}, 500) }, 500);
+    //
+    // We want to make sure that newly added timer fires in the next turn of the
+    // event loop at the earliest. So even if it's already expired now,
+    // reschedule it to fire later.
+    //
+    // At that point, it's not necessary to process any other timer in that
+    // list, because any remaining timer has been added within a callback of a
+    // timer that has already been processed, and thus needs to be processed at
+    // the earliest not in the current tick, but when the rescheduled timer will
+    // expire.
+    //
+    // See: https://github.com/joyent/node/issues/25607
+    if (now <= first._monotonicStartTime) {
+      var timeRemaining = msecs - (Timer.now() - first._monotonicStartTime);
+      if (timeRemaining < 0) {
+        timeRemaining = 0;
+      }
+      debug(msecs + ' list wait because timer was added from another timer');
+      list.start(timeRemaining, 0);
+      return;
     }
 
     var diff = now - first._monotonicStartTime;

test/common.js

@@ -22,6 +22,7 @@
 var path = require('path');
 var fs = require('fs');
 var assert = require('assert');
+var Timer = process.binding('timer_wrap').Timer;
 
 exports.testDir = path.dirname(__filename);
 exports.fixturesDir = path.join(exports.testDir, 'fixtures');
@@ -220,9 +221,9 @@ exports.hasMultiLocalhost = function hasMultiLocalhost() {
 };
 
 exports.busyLoop = function busyLoop(time) {
-  var startTime = new Date().getTime();
+  var startTime = Timer.now();
   var stopTime =  startTime + time;
-  while (new Date().getTime() < stopTime) {
+  while (Timer.now() < stopTime) {
     ;
   }
 };

test/simple/test-timers-nested.js

@@ -0,0 +1,58 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+// Make sure we test 0ms timers, since they would had always wanted to run on
+// the current tick, and greater than 0ms timers, for scenarios where the
+// outer timer takes longer to complete than the delay of the nested timer.
+// Since the process of recreating this is identical regardless of the timer
+// delay, these scenarios are in one test.
+var scenarios = [0, 100];
+
+scenarios.forEach(function (delay) {
+  var nestedCalled = false;
+
+  setTimeout(function A() {
+    // Create the nested timer with the same delay as the outer timer so that it
+    // gets added to the current list of timers being processed by
+    // listOnTimeout.
+    setTimeout(function B() {
+      nestedCalled = true;
+    }, delay);
+
+    // Busy loop for the same timeout used for the nested timer to ensure that
+    // we are in fact expiring the nested timer.
+    common.busyLoop(delay);
+
+    // The purpose of running this assert in nextTick is to make sure it runs
+    // after A but before the next iteration of the libuv event loop.
+    process.nextTick(function() {
+      assert.ok(!nestedCalled);
+    });
+
+    // Ensure that the nested callback is indeed called prior to process exit.
+    process.on('exit', function onExit() {
+      assert.ok(nestedCalled);
+    });
+  }, delay);
+});