StreamSelectLoop timer interval limit fix (on 32-bit systems) #23
Conversation
|
I have yet to do some more in-depth testing, but on first sight the changes LGTM 👍 Afaict 32-bit systems are becoming increasingly less common, yet this change affects all versions. Is this worth considering, i.e. is the overhead even noticeable? A slightly less intrusive alternative might be to just cap the maximum |
|
2147 is just over 35 minutes. Tbh if you need a timer to run in 10 minutes or later you maybe should consider a more persistent storage of your timer event. For example dead letter exchanges in rabbitmq. Capping it to |
|
Unfortunately this PR is technically a BC break, as consumers could depend on the I think it makes sense to fix the underlying issue, but we won't be able merge this into the v0.4 series like this. |
|
Ping @alexmnv, what do you think about the above comments? Also, may I ask you to rebase this on the new "0.4" branch so we get this in for the upcoming v0.4.3 release? |
| return stream_select($read, $write, $except, $timeout === null ? null : 0, $timeout); | ||
| $tv_sec = $timeout === null ?: floor($timeout); | ||
| // convert the fractional part of $timeout to microseconds | ||
| $tv_usec = $timeout === null ?: round(fmod($timeout, 1) * self::MICROSECONDS_PER_SECOND); |
There was a problem hiding this comment.
When $timeout === null, both $tv_sec and $tv_usec will become true because of the elvis operator.
This should be
$tv_sec = $timeout === null ? null : floor($timeout);
// convert the fractional part of $timeout to microseconds
$tv_usec = $timeout === null ? null : round(fmod($timeout, 1) * self::MICROSECONDS_PER_SECOND);
Fix timer interval limit issue in \React\EventLoop\StreamSelectLoop (issue #19)
On 32-bit systems it's not possible to set a timer interval higher than 2147 seconds.