Skip to content
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.

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

setInterval and setTimeout are limited to 24 days delay #34

Closed
mau31415 opened this issue Nov 28, 2023 · 6 comments
Closed

setInterval and setTimeout are limited to 24 days delay #34

mau31415 opened this issue Nov 28, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@mau31415
Copy link

In old node version setTimeout was limited to 32bit timeout which translate to 24 days. On a larger numbers it would overflow. To avoid this issue, "Later" had capped the maximum delay to 32bit delay. However, this limit in node was resolved many years ago.
This adjustment in "later" should be removed also.

t = diff < 2147483647 ? setTimeout(fn, diff) : setTimeout(scheduleTimeout, 2147483647);

this if needs to be removed.

@mau31415 mau31415 added the bug Something isn't working label Nov 28, 2023
@titanism
Copy link
Contributor

@mau31415 can you please submit a PR? Thank you for this finding. 🙏

@mau31415
Copy link
Author

#35

@titanism titanism mentioned this issue Nov 28, 2023
6 tasks
@titanism
Copy link
Contributor

This limit is not resolved as far as I can tell.

If you look at the docs at https://nodejs.org/api/timers.html#settimeoutcallback-delay-args it states:

When delay is larger than 2147483647 or less than 1, the delay will be set to 1. Non-integer delays are truncated to an integer.

Screen Shot 2023-11-28 at 3 05 07 PM

@titanism
Copy link
Contributor

I am reverting that PR in the interim until further comments show that this is not the case. Also, a working test case would be great.

@mau31415
Copy link
Author

you are right, it is still the case...
I'll try and have a better fix for it.

@titanism
Copy link
Contributor

Okay! No worries, going to close this issue for now. If you have a workaround you want to implement please do. Note that I'm pushing a bunch of updates in the next few minutes to master branch, so you may want to update your fork (or delete it and re-fork it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants