| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 9c274e3 commit 49a6ea1
4 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -160,7 +160,7 @@ function listOnTimeout() { | |||
| 160 | 160 | debug('timeout callback %d', msecs); | |
| 161 | 161 | ||
| 162 | 162 | var now = TimerWrap.now(); | |
| 163 | - debug('now: %s', now); | ||
| 163 | + debug('now: %d', now); | ||
| 164 | 164 | ||
| 165 | 165 | var diff, timer; | |
| 166 | 166 | while (timer = L.peek(list)) { | |
@@ -169,7 +169,11 @@ function listOnTimeout() { | |||
| 169 | 169 | // Check if this loop iteration is too early for the next timer. | |
| 170 | 170 | // This happens if there are more timers scheduled for later in the list. | |
| 171 | 171 | if (diff < msecs) { | |
| 172 | - this.start(msecs - diff, 0); | ||
| 172 | + var timeRemaining = msecs - (TimerWrap.now() - timer._idleStart); | ||
| 173 | + if (timeRemaining < 0) { | ||
| 174 | + timeRemaining = 0; | ||
| 175 | + } | ||
| 176 | + this.start(timeRemaining, 0); | ||
| 173 | 177 | debug('%d list wait because diff is %d', msecs, diff); | |
| 174 | 178 | return; | |
| 175 | 179 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -7,6 +7,7 @@ var os = require('os'); | |||
| 7 | 7 | var child_process = require('child_process'); | |
| 8 | 8 | const stream = require('stream'); | |
| 9 | 9 | const util = require('util'); | |
| 10 | + const Timer = process.binding('timer_wrap').Timer; | ||
| 10 | 11 | ||
| 11 | 12 | const testRoot = path.resolve(process.env.NODE_TEST_DIR || | |
| 12 | 13 | path.dirname(__filename)); | |
@@ -484,3 +485,9 @@ exports.nodeProcessAborted = function nodeProcessAborted(exitCode, signal) { | |||
| 484 | 485 | return expectedExitCodes.indexOf(exitCode) > -1; | |
| 485 | 486 | } | |
| 486 | 487 | }; | |
| 488 | + | ||
| 489 | + exports.busyLoop = function busyLoop(time) { | ||
| 490 | + var startTime = Timer.now(); | ||
| 491 | + var stopTime = startTime + time; | ||
| 492 | + while (Timer.now() < stopTime) {} | ||
| 493 | + }; | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,81 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + /* | ||
| 4 | + * This is a regression test for https://github.com/joyent/node/issues/15447 | ||
| 5 | + * and https://github.com/joyent/node/issues/9333. | ||
| 6 | + * | ||
| 7 | + * When a timer is added in another timer's callback, its underlying timer | ||
| 8 | + * handle was started with a timeout that was actually incorrect. | ||
| 9 | + * | ||
| 10 | + * The reason was that the value that represents the current time was not | ||
| 11 | + * updated between the time the original callback was called and the time | ||
| 12 | + * the added timer was processed by timers.listOnTimeout. That lead the | ||
| 13 | + * logic in timers.listOnTimeout to do an incorrect computation that made | ||
| 14 | + * the added timer fire with a timeout of scheduledTimeout + | ||
| 15 | + * timeSpentInCallback. | ||
| 16 | + * | ||
| 17 | + * This test makes sure that a timer added by another timer's callback | ||
| 18 | + * fire with the expected timeout. | ||
| 19 | + * | ||
| 20 | + * It makes sure that it works when the timers list for a given timeout is | ||
| 21 | + * empty (see testAddingTimerToEmptyTimersList) and when the timers list | ||
| 22 | + * is not empty (see testAddingTimerToNonEmptyTimersList). | ||
| 23 | + */ | ||
| 24 | + | ||
| 25 | + const assert = require('assert'); | ||
| 26 | + const common = require('../common'); | ||
| 27 | + const Timer = process.binding('timer_wrap').Timer; | ||
| 28 | + | ||
| 29 | + const TIMEOUT = 100; | ||
| 30 | + | ||
| 31 | + var nbBlockingCallbackCalls = 0; | ||
| 32 | + var latestDelay = 0; | ||
| 33 | + var timeCallbackScheduled = 0; | ||
| 34 | + | ||
| 35 | + function initTest() { | ||
| 36 | + nbBlockingCallbackCalls = 0; | ||
| 37 | + latestDelay = 0; | ||
| 38 | + timeCallbackScheduled = 0; | ||
| 39 | + } | ||
| 40 | + | ||
| 41 | + function blockingCallback(callback) { | ||
| 42 | + ++nbBlockingCallbackCalls; | ||
| 43 | + | ||
| 44 | + if (nbBlockingCallbackCalls > 1) { | ||
| 45 | + latestDelay = Timer.now() - timeCallbackScheduled; | ||
| 46 | + // Even if timers can fire later than when they've been scheduled | ||
| 47 | + // to fire, they should more than 50% later with a timeout of | ||
| 48 | + // 100ms. Firing later than that would mean that we hit the regression | ||
| 49 | + // highlighted in | ||
| 50 | + // https://github.com/nodejs/node-v0.x-archive/issues/15447 and | ||
| 51 | + // https://github.com/nodejs/node-v0.x-archive/issues/9333.. | ||
| 52 | + assert(latestDelay < TIMEOUT * 1.5); | ||
| 53 | + if (callback) | ||
| 54 | + return callback(); | ||
| 55 | + } else { | ||
| 56 | + // block by busy-looping to trigger the issue | ||
| 57 | + common.busyLoop(TIMEOUT); | ||
| 58 | + | ||
| 59 | + timeCallbackScheduled = Timer.now(); | ||
| 60 | + setTimeout(blockingCallback, TIMEOUT); | ||
| 61 | + } | ||
| 62 | + } | ||
| 63 | + | ||
| 64 | + function testAddingTimerToEmptyTimersList(callback) { | ||
| 65 | + initTest(); | ||
| 66 | + // Call setTimeout just once to make sure the timers list is | ||
| 67 | + // empty when blockingCallback is called. | ||
| 68 | + setTimeout(blockingCallback.bind(null, callback), TIMEOUT); | ||
| 69 | + } | ||
| 70 | + | ||
| 71 | + function testAddingTimerToNonEmptyTimersList() { | ||
| 72 | + initTest(); | ||
| 73 | + // Call setTimeout twice with the same timeout to make | ||
| 74 | + // sure the timers list is not empty when blockingCallback is called. | ||
| 75 | + setTimeout(blockingCallback, TIMEOUT); | ||
| 76 | + setTimeout(blockingCallback, TIMEOUT); | ||
| 77 | + } | ||
| 78 | + | ||
| 79 | + // Run the test for the empty timers list case, and then for the non-empty | ||
| 80 | + // timers list one | ||
| 81 | + testAddingTimerToEmptyTimersList(testAddingTimerToNonEmptyTimersList); | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,39 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + const assert = require('assert'); | ||
| 4 | + const common = require('../common'); | ||
| 5 | + | ||
| 6 | + // Make sure we test 0ms timers, since they would had always wanted to run on | ||
| 7 | + // the current tick, and greater than 0ms timers, for scenarios where the | ||
| 8 | + // outer timer takes longer to complete than the delay of the nested timer. | ||
| 9 | + // Since the process of recreating this is identical regardless of the timer | ||
| 10 | + // delay, these scenarios are in one test. | ||
| 11 | + const scenarios = [0, 100]; | ||
| 12 | + | ||
| 13 | + scenarios.forEach(function(delay) { | ||
| 14 | + var nestedCalled = false; | ||
| 15 | + | ||
| 16 | + setTimeout(function A() { | ||
| 17 | + // Create the nested timer with the same delay as the outer timer so that it | ||
| 18 | + // gets added to the current list of timers being processed by | ||
| 19 | + // listOnTimeout. | ||
| 20 | + setTimeout(function B() { | ||
| 21 | + nestedCalled = true; | ||
| 22 | + }, delay); | ||
| 23 | + | ||
| 24 | + // Busy loop for the same timeout used for the nested timer to ensure that | ||
| 25 | + // we are in fact expiring the nested timer. | ||
| 26 | + common.busyLoop(delay); | ||
| 27 | + | ||
| 28 | + // The purpose of running this assert in nextTick is to make sure it runs | ||
| 29 | + // after A but before the next iteration of the libuv event loop. | ||
| 30 | + process.nextTick(function() { | ||
| 31 | + assert.ok(!nestedCalled); | ||
| 32 | + }); | ||
| 33 | + | ||
| 34 | + // Ensure that the nested callback is indeed called prior to process exit. | ||
| 35 | + process.on('exit', function onExit() { | ||
| 36 | + assert.ok(nestedCalled); | ||
| 37 | + }); | ||
| 38 | + }, delay); | ||
| 39 | + }); | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments