fix: hang on macOS due to overflow/underflow of durations#55
Conversation
We also ensure that we won't overflow the msec timer on other POSIX systems.
WalkthroughTimeout handling in terminal I/O read operations was refined by adding duration bounds clamping in both select and poll code paths. Duration values are now constrained to valid ranges—10 microseconds to 1 hour for select operations, and up to 24 hours for poll operations—with updated calculations to prevent overflow and ensure correct type casting. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
source/dcell/termio.d (2)
14-14: Remove the redundant local import on line 232.The top-level import of
std.algorithmmakes the local selective import on line 232 (import std.algorithm : max;) redundant.Apply this diff to remove the redundant import:
import std.algorithm : max; - int num = select(max(fd, sigRfd) + 1, &readFds, null, null, tvp); + int num = select(max(fd, sigRfd) + 1, &readFds, null, null, tvp);Actually, simply remove line 232:
- import std.algorithm : max; - int num = select(max(fd, sigRfd) + 1, &readFds, null, null, tvp);
283-285: Consider adding minimum duration clamping for consistency.The 24-hour maximum clamp prevents overflow (86,400,000 ms fits well within int range). However, unlike the
UseSelectpath which clamps to a minimum of 10µs, this path doesn't enforce a minimum. A negative or zero non-max duration will result indly = 0, which is valid but inconsistent with the select path's behavior.Consider applying this diff for consistency:
else { - // clip to a day to prevent overrun - dur = min(hours(24), dur); + // clip between 10us and a day to prevent overrun + dur = min(hours(24), max(dur, usecs(10))); dly = cast(int)(dur.total!"msecs"); }This ensures both paths treat zero/negative durations consistently.
We also ensure that we won't overflow the msec timer on other POSIX systems.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.