Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Timers sometimes fire early on Windows #8960

Closed
orangemocha opened this issue Dec 31, 2014 · 7 comments · Fixed by nodejs/node#5994
Closed

Timers sometimes fire early on Windows #8960

orangemocha opened this issue Dec 31, 2014 · 7 comments · Fixed by nodejs/node#5994
Assignees
@orangemocha
Labels
windows

Comments

@orangemocha
Copy link
Contributor

Test simple/test-timers-first-fire.js occasionally fails on Windows, because timers fire early. I have seen the timer in the test fire up to 15ms early.

This was supposed to be fixed for the most part by joyent/libuv@6ced8c2 and for small rounding errors by #7211.

When I opened the original PR against libuv (joyent/libuv#1165) it did fix the issue, but I guess that by the time it was accepted and finally merged into node (c5f5d4c) something else must have changed that still causes timers to fire early.
@orangemocha orangemocha self-assigned this Dec 31, 2014
@orangemocha
Copy link
Contributor Author

cc @misterdjules

@orangemocha orangemocha mentioned this issue Dec 31, 2014
Closed
@Fishrock123
Copy link
Member

Just got a timer to fire -1.25ms early on the io.js CI...:

nodejs/node#1272 (comment)

@misterdjules
Copy link

@orangemocha Do you have an idea of how often this problem can be reproduced on Windows? Basically, is there anything we need to know to be able to reproduce it (Windows version, etc.)?

@Fishrock123
Copy link
Member

@trevnorris mentioned that he thought the time was in us not ms.

@Fishrock123
Copy link
Member

Also, we had an interesting post that i'm going to attempt to look into (with my limited C knowledge) about QPC floating-point rounding issues: nodejs/node#1272 (comment)

@Fishrock123 Fishrock123 mentioned this issue Mar 26, 2015
Closed
@orangemocha
Copy link
Contributor Author

The test output is actually in ms, not us.
When I opened this issue I could repro just by running the test in a loop on a Win 8.1 machine. I just tried the same on a Windows Server 2012 R2 machine and couldn't reproduce it.
I will look into this further...

@orangemocha orangemocha mentioned this issue Jun 3, 2015
Closed
@jasnell jasnell added the windows label Jun 5, 2015
@orangemocha orangemocha mentioned this issue Jun 25, 2015
Merged
@orangemocha
Copy link
Contributor Author

This is also flaky in v0.10 (and a2f879f marked it so).

@shigeki shigeki mentioned this issue Aug 21, 2015
Closed
joaocgreis added a commit to JaneaSystems/libuv that referenced this issue Sep 24, 2015
@joaocgreis
win: wait for full timeout duration
57c9876 
uv_poll should wait for at least the full timeout duration when there
is nothing else to do. This was not happening because
GetQueuedCompletionStatus can occasionally return up to 15ms early.

The added test reproduces https://github.com/joyent/node/blob/d13d7f74d794340ac5e126cfb4ce507fe0f803d5/test/simple/test-timers-first-fire.js
on libuv, being flaky before this fix.

Fix: nodejs/node-v0.x-archive#8960
joaocgreis added a commit to JaneaSystems/libuv that referenced this issue Dec 16, 2015
@joaocgreis @orangemocha
win: wait for full timeout duration
be8b3f5 
uv_poll should wait for at least the full timeout duration when there
is nothing else to do. This was not happening because
GetQueuedCompletionStatus can occasionally return up to 15ms early.

The added test reproduces https://github.com/joyent/node/blob/d13d7f74d794340ac5e126cfb4ce507fe0f803d5/test/simple/test-timers-first-fire.js
on libuv, being flaky before this fix.

Fix: nodejs/node-v0.x-archive#8960
joaocgreis added a commit to JaneaSystems/libuv that referenced this issue Dec 16, 2015
@joaocgreis
win: wait for full timeout duration
69941c8 
uv_poll should wait for at least the full timeout duration when there
is nothing else to do. This was not happening because
GetQueuedCompletionStatus can occasionally return up to 15ms early.

The added test reproduces https://github.com/joyent/node/blob/d13d7f74d794340ac5e126cfb4ce507fe0f803d5/test/simple/test-timers-first-fire.js
on libuv, being flaky before this fix.

Fix: nodejs/node-v0.x-archive#8960
@saghul saghul mentioned this issue Apr 1, 2016
Merged
4 tasks
saghul added a commit to saghul/node that referenced this issue Apr 7, 2016
@saghul
deps: upgrade libuv to 1.9.0
55a964e 
Fixes: nodejs#5737
Fixes: nodejs#4643
Fixes: nodejs#4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: nodejs#3594
PR-URL: nodejs#5994
saghul added a commit to nodejs/node that referenced this issue Apr 7, 2016
@saghul
deps: upgrade libuv to 1.9.0
c3cec1e 
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to nodejs/node that referenced this issue Apr 19, 2016
@saghul
deps: upgrade libuv to 1.9.0
27e77c5 
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
saghul added a commit to saghul/node that referenced this issue Jul 11, 2016
@saghul
deps: upgrade libuv to 1.9.0
395b161 
Fixes: nodejs#5737
Fixes: nodejs#4643
Fixes: nodejs#4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: nodejs#3594
PR-URL: nodejs#5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to nodejs/node that referenced this issue Jul 11, 2016
@saghul
deps: upgrade libuv to 1.9.0
50da0ae 
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to nodejs/node that referenced this issue Jul 11, 2016
@saghul
deps: upgrade libuv to 1.9.0
af9bbf5 
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to nodejs/node that referenced this issue Jul 12, 2016
@saghul
deps: upgrade libuv to 1.9.0
43c16ec 
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to nodejs/node that referenced this issue Jul 14, 2016
@saghul
deps: upgrade libuv to 1.9.0
f56125e 
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to nodejs/node that referenced this issue Jul 14, 2016
@saghul
deps: upgrade libuv to 1.9.0
94eb980 
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@orangemocha orangemocha

Labels
windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

deps: upgrade libuv to 1.9.0 saghul/node
win: wait for full timeout duration JaneaSystems/libuv
4 participants
@jasnell @misterdjules @Fishrock123 @orangemocha