Fix run_for blocking indefinitely with outstanding work#220
Conversation
run_task() had no timeout parameter — the reactor always blocked forever or polled instantly, ignoring the timeout from do_one(). Propagate the timeout to each backend's I/O syscall (epoll_wait, select, kevent) and return after a timed reactor poll so run_one_until can recheck the deadline.
📝 WalkthroughWalkthroughThis pull request adds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
include/boost/corosio/native/detail/epoll/epoll_scheduler.hpp (1)
301-307: Potential integer overflow for very largetimeout_usvalues.The expression
timeout_us + 999can overflow whentimeout_usis close toLONG_MAX(signed overflow is undefined behavior). Additionally, the cast tointcan truncate if the result exceedsINT_MAX(about 24.8 days in milliseconds).The
kqueue_schedulerandselect_schedulerbackends usecalculate_timeout()which caps values appropriately. Consider applying similar bounds checking here for consistency and robustness:♻️ Suggested fix with overflow protection
int timeout_ms; if (task_interrupted_) timeout_ms = 0; else if (timeout_us < 0) timeout_ms = -1; else - timeout_ms = static_cast<int>((timeout_us + 999) / 1000); +{ + // Cap to INT_MAX milliseconds (~24.8 days) to prevent overflow + constexpr long max_timeout_us = static_cast<long>(INT_MAX) * 1000; + long capped_us = (std::min)(timeout_us, max_timeout_us); + timeout_ms = static_cast<int>((capped_us + 999) / 1000); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/epoll/epoll_scheduler.hpp` around lines 301 - 307, The timeout conversion can overflow: avoid signed overflow from `timeout_us + 999` and cap millisecond result to `INT_MAX`. In the `epoll_scheduler` branch where `timeout_us` and `timeout_ms` are computed (referencing `timeout_us`, `timeout_ms`, and `task_interrupted_`), replace the naive `(timeout_us + 999) / 1000` with a safe check that if `timeout_us < 0` set `timeout_ms = -1`, else if `timeout_us` is large enough that converting to milliseconds would exceed `INT_MAX` set `timeout_ms = INT_MAX`, otherwise compute `(timeout_us + 999) / 1000` using a cast to an unsigned or wider type to avoid overflow; alternatively reuse the existing `calculate_timeout()` helper used by `kqueue_scheduler`/`select_scheduler` for consistent bounds handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@include/boost/corosio/native/detail/epoll/epoll_scheduler.hpp`:
- Around line 301-307: The timeout conversion can overflow: avoid signed
overflow from `timeout_us + 999` and cap millisecond result to `INT_MAX`. In the
`epoll_scheduler` branch where `timeout_us` and `timeout_ms` are computed
(referencing `timeout_us`, `timeout_ms`, and `task_interrupted_`), replace the
naive `(timeout_us + 999) / 1000` with a safe check that if `timeout_us < 0` set
`timeout_ms = -1`, else if `timeout_us` is large enough that converting to
milliseconds would exceed `INT_MAX` set `timeout_ms = INT_MAX`, otherwise
compute `(timeout_us + 999) / 1000` using a cast to an unsigned or wider type to
avoid overflow; alternatively reuse the existing `calculate_timeout()` helper
used by `kqueue_scheduler`/`select_scheduler` for consistent bounds handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d17ef516-16e9-4c80-ba49-5e9e5a7717fe
⛔ Files ignored due to path filters (1)
test/unit/io_context.cppis excluded by!**/test/**
📒 Files selected for processing (4)
include/boost/corosio/native/detail/epoll/epoll_scheduler.hppinclude/boost/corosio/native/detail/kqueue/kqueue_scheduler.hppinclude/boost/corosio/native/detail/reactor/reactor_scheduler.hppinclude/boost/corosio/native/detail/select/select_scheduler.hpp
|
An automated preview of the documentation is available at https://220.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-03-30 19:27:13 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #220 +/- ##
===========================================
- Coverage 77.90% 77.90% -0.01%
===========================================
Files 96 96
Lines 7397 7395 -2
Branches 1806 1805 -1
===========================================
- Hits 5763 5761 -2
Misses 1110 1110
Partials 524 524
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
GCOVR code coverage report https://220.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-03-30 19:38:08 UTC |
2 participants
run_task() had no timeout parameter — the reactor always blocked forever or polled instantly, ignoring the timeout from do_one(). Propagate the timeout to each backend's I/O syscall (epoll_wait, select, kevent) and return after a timed reactor poll so run_one_until can recheck the deadline.
Summary by CodeRabbit
Release Notes