test: fix freebsd10-64 CI failures

Remove unneeded timers from some tests and move others from parallel
testing to sequential testing.

This is to resolve test failures on freebsd10-64 on CI. The failures
are all due to timers firing later than expected. Timers firing later
than they are set for can happen on resource-constrained hosts and is
not a bug.

In general, it may be wise to put tests that depend on timing into
sequential testing rather than parallel testing, as the timing can
be affected by other simultaneously-running test processes.

Fixes: https://github.com/nodejs/node/issues/8041
Fixes: https://github.com/nodejs/node/issues/9227
PR-URL: https://github.com/nodejs/node/pull/9317
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Johan Bergstrom <bugs@bergstroem.nu>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>

1 files changed, 80 insertions, 0 deletions

diff --git a/test/sequential/test-timers-blocking-callback.js b/test/sequential/test-timers-blocking-callback.js
new file mode 100644
index 0000000000..e4e85cddc6
--- /dev/null
+++ b/test/sequential/test-timers-blocking-callback.js
@@ -0,0 +1,80 @@
+'use strict';
+
+/*
+ * This is a regression test for https://github.com/joyent/node/issues/15447
+ * and https://github.com/joyent/node/issues/9333.
+ *
+ * When a timer is added in another timer's callback, its underlying timer
+ * handle was started with a timeout that was actually incorrect.
+ *
+ * The reason was that the value that represents the current time was not
+ * updated between the time the original callback was called and the time
+ * the added timer was processed by timers.listOnTimeout. That led the
+ * logic in timers.listOnTimeout to do an incorrect computation that made
+ * the added timer fire with a timeout of scheduledTimeout +
+ * timeSpentInCallback.
+ *
+ * This test makes sure that a timer added by another timer's callback
+ * fires with the expected timeout.
+ *
+ * It makes sure that it works when the timers list for a given timeout is
+ * empty (see testAddingTimerToEmptyTimersList) and when the timers list
+ * is not empty (see testAddingTimerToNonEmptyTimersList).
+ */
+
+const common = require('../common');
+const assert = require('assert');
+const Timer = process.binding('timer_wrap').Timer;
+
+const TIMEOUT = 100;
+
+var nbBlockingCallbackCalls = 0;
+var latestDelay = 0;
+var timeCallbackScheduled = 0;
+
+function initTest() {
+  nbBlockingCallbackCalls = 0;
+  latestDelay = 0;
+  timeCallbackScheduled = 0;
+}
+
+function blockingCallback(callback) {
+  ++nbBlockingCallbackCalls;
+
+  if (nbBlockingCallbackCalls > 1) {
+    latestDelay = Timer.now() - timeCallbackScheduled;
+    // Even if timers can fire later than when they've been scheduled
+    // to fire, they shouldn't generally be more than 100% late in this case.
+    // But they are guaranteed to be at least 100ms late given the bug in
+    // https://github.com/nodejs/node-v0.x-archive/issues/15447 and
+    // https://github.com/nodejs/node-v0.x-archive/issues/9333..
+    assert(latestDelay < TIMEOUT * 2);
+    if (callback)
+      return callback();
+  } else {
+    // block by busy-looping to trigger the issue
+    common.busyLoop(TIMEOUT);
+
+    timeCallbackScheduled = Timer.now();
+    setTimeout(blockingCallback.bind(null, callback), TIMEOUT);
+  }
+}
+
+const testAddingTimerToEmptyTimersList = common.mustCall(function(callback) {
+  initTest();
+  // Call setTimeout just once to make sure the timers list is
+  // empty when blockingCallback is called.
+  setTimeout(blockingCallback.bind(null, callback), TIMEOUT);
+});
+
+const testAddingTimerToNonEmptyTimersList = common.mustCall(function() {
+  initTest();
+  // Call setTimeout twice with the same timeout to make
+  // sure the timers list is not empty when blockingCallback is called.
+  setTimeout(blockingCallback, TIMEOUT);
+  setTimeout(blockingCallback, TIMEOUT);
+});
+
+// Run the test for the empty timers list case, and then for the non-empty
+// timers list one
+testAddingTimerToEmptyTimersList(testAddingTimerToNonEmptyTimersList);