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

setTimeout causing full cpu usage with non-integer durations #8065

Closed
sammyt opened this issue Aug 3, 2014 · 4 comments
Closed

setTimeout causing full cpu usage with non-integer durations #8065

sammyt opened this issue Aug 3, 2014 · 4 comments
Labels

Comments

@sammyt
Copy link

sammyt commented Aug 3, 2014

I have observed node locking up with full cpu utilization when using setTimeout with non-integer values

The follwing code snipit reliably reproduces the issue on my mac running node version v0.10.30
The t value was generated using Math.random() * 1000 and any such value has the same effect for me

var t = 33.707402646541595
function noop(){}
setInterval(noop, t)
// hangs after a few seconds

The following code however has no problems

var t = 33
function noop(){}
setInterval(noop, t)
// never hangs

I attached a debugger and the runtime appear to be stuck in a tight loop, here inside timers.js

UPDATE: fixed the working code (copy-paste fail)

sammyt added a commit to sammyt/node-coap that referenced this issue Aug 3, 2014
@loyd
Copy link

loyd commented Aug 3, 2014

Confirm overhead. Likely, (1) v8 is forced to use double instead int or (2) conversion double->int occurs at the junction of native and js.

In any case, DOM-version of timers discard fractional part. I think, node's version must do so too.

@misterdjules
Copy link

@sammyt Thank you for reporting this issue, I'm looking into this right now.

@misterdjules
Copy link

@sammyt @loyd I posted the result of our investigation here: #8067. Stay tuned for more info, hopefully sooner than later.

misterdjules pushed a commit to misterdjules/node that referenced this issue Aug 4, 2014
When backporting f8193ab into v0.10, a regression was introduced. Timers
with non-integer timeout could trigger a infinite recursion with 100%
cpu usage. This commit backports 93b0624 which fixes the regression.

After backporting f8193ab, instead of using Date.now(), timers would use
Timer.now() to determine if they had expired. However, Timer.now() is
based on loop->time, which is not updated when a timer's remaining time
is > 0 and < 1. With this commit, Timer.now() updates loop->time itself.
misterdjules pushed a commit to misterdjules/node that referenced this issue Aug 4, 2014
When backporting f8193ab into v0.10, a regression was introduced. Timers
with non-integer timeout could trigger a infinite recursion with 100%
cpu usage. This commit backports 93b0624 which fixes the regression.

After backporting f8193ab, instead of using Date.now(), timers would use
Timer.now() to determine if they had expired. However, Timer.now() is
based on loop->time, which is not updated when a timer's remaining time
is > 0 and < 1. Timers would thus never timeout if their remaining time
was at some point > 0 and < 1.

With this commit, Timer.now() updates loop->time itself, and timers
always timeout eventually.

Fixes nodejs#8065 and nodejs#8068.
@hunterloftis
Copy link

@misterdjules thanks for looking into it; just got our first support ticket regarding this + 0.10.30 at Heroku.

mscdex pushed a commit to mscdex/node that referenced this issue Dec 25, 2014
When backporting f8193ab into v0.10, a regression was introduced. Timers
with non-integer timeout could trigger a infinite recursion with 100%
cpu usage. This commit backports 93b0624 which fixes the regression.

After backporting f8193ab, instead of using Date.now(), timers would use
Timer.now() to determine if they had expired. However, Timer.now() is
based on loop->time, which is not updated when a timer's remaining time
is > 0 and < 1. Timers would thus never timeout if their remaining time
was at some point > 0 and < 1.

With this commit, Timer.now() updates loop->time itself, and timers
always timeout eventually.

Fixes nodejs#8065 and nodejs#8068.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants