Skip to content

Port from joyent/node: timers: fix timeout when added in timer's callback #2232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Fishrock123 wants to merge 3 commits into from
Closed

Port from joyent/node: timers: fix timeout when added in timer's callback #2232

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1facc67
timers: fix timeout when added in timer's callback
Apr 21, 2015
6c0cb86
!fixup nits
Fishrock123 Jul 27, 2015
dc7bcc0
!fixup address nits
Fishrock123 Jul 28, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
!fixup address nits
  • Loading branch information
@Fishrock123
Fishrock123 committed Jul 28, 2015
commit dc7bcc076470a8cb7aaa259df88a5b852dc3c61f
4 changes: 2 additions & 2 deletions test/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ exports.fileExists = function(pathname) {
};

exports.busyLoop = function busyLoop(time) {
var startTime = new Date().getTime();
var startTime = Date.now();
Copy link
Copy Markdown
Contributor

@thefourtheye thefourtheye Jul 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const? Also do we really need this? I mean we can directly use Date.now() in the following statement no?

Copy link
Copy Markdown
Contributor Author

@Fishrock123 Fishrock123 Jul 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice, true.

var stopTime = startTime + time;
while (new Date().getTime() < stopTime);
while (Date.now() < stopTime);
Copy link
Copy Markdown
Contributor

@thefourtheye thefourtheye Jul 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am on mobile right now. I would like to check if this will be an infinite loop if stopTime is NaN

Copy link
Copy Markdown
Contributor Author

@Fishrock123 Fishrock123 Jul 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything < or > NaN is false. :)

Copy link
Copy Markdown
Contributor

@thefourtheye thefourtheye Jul 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah cool then :)

};
8 changes: 5 additions & 3 deletions test/parallel/test-timers-blocking-callback.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use strict';

/*
* This is a regression test for https://github.com/joyent/node/issues/15447
* and https://github.com/joyent/node/issues/9333.
Expand Down Expand Up @@ -39,7 +41,7 @@ function blockingCallback(callback) {
++nbBlockingCallbackCalls;

if (nbBlockingCallbackCalls > 1) {
latestDelay = new Date().getTime() - timeCallbackScheduled;
latestDelay = Date.now() - timeCallbackScheduled;
// Even if timers can fire later than when they've been scheduled
// to fire, they should more than 50% later with a timeout of
// 100ms. Firing later than that would mean that we hit the regression
Expand All @@ -53,7 +55,7 @@ function blockingCallback(callback) {
// block by busy-looping to trigger the issue
common.busyLoop(TIMEOUT);

timeCallbackScheduled = new Date().getTime();
timeCallbackScheduled = Date.now();
setTimeout(blockingCallback, TIMEOUT);
}
}
Expand All @@ -62,7 +64,7 @@ function testAddingTimerToEmptyTimersList(callback) {
initTest();
// Call setTimeout just once to make sure the timers list is
// empty when blockingCallback is called.
setTimeout(blockingCallback.bind(global, callback), TIMEOUT);
setTimeout(blockingCallback.bind(null, callback), TIMEOUT);
}

function testAddingTimerToNonEmptyTimersList() {
Expand Down