Add multi-waiter support, cancel_one, and scheduler_impl intermediary#134
Add multi-waiter support, cancel_one, and scheduler_impl intermediary#134sgerbino merged 1 commit intocppalliance:developfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughRefactors timer internals to use per-waiter state (waiter_node) enabling multiple concurrent waiters, introduces scheduler_impl as an intermediary caching timer_service pointers used by platform schedulers, and changes timer APIs to return std::size_t counts; several scheduler headers/source files and dispatch logic adjusted accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Coro1 as Coroutine 1
participant Coro2 as Coroutine 2
participant Timer as Timer
participant WN as Waiter Pool
participant TS as Timer Service
participant Sched as Scheduler
Coro1->>Timer: call wait()
Timer->>WN: allocate waiter_node for Coro1
WN-->>Timer: waiter_node created
Timer->>TS: register waiter_node (schedule/insert timer)
TS->>TS: insert/update timer heap/list
Coro2->>Timer: call wait() (same timer)
Timer->>WN: allocate waiter_node for Coro2
Timer->>TS: add waiter_node to timer's waiter list
rect rgba(100, 200, 100, 0.5)
Note over TS: timer expires
TS->>WN: process_expired -> collect waiter_nodes
TS->>Sched: post completions for waiter_nodes
end
Sched->>Coro1: resume (completion)
Sched->>Coro2: resume (completion)
Coro1->>WN: destroy/recycle waiter_node
Coro2->>WN: destroy/recycle waiter_node
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
|
An automated preview of the documentation is available at https://134.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-12 04:35:58 UTC |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #134 +/- ##
===========================================
+ Coverage 81.03% 81.15% +0.11%
===========================================
Files 64 64
Lines 5557 5666 +109
===========================================
+ Hits 4503 4598 +95
- Misses 1054 1068 +14
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/corosio/src/detail/timer_service.cpp`:
- Around line 708-722: The insertion into the intrusive list in wait() (the
block calling waiters_.push_back(w)) races with cancel_waiter()'s
waiters_.remove(w); fix by serializing list mutations with the existing mutex_
(or by performing the push inside insert_timer so the insertion happens within
the same lock scope used for timer insertion). Concretely, either acquire mutex_
around the push_back in wait() and around any waiters_.empty() checks used in
update_timer/cancel_timer, or modify/extend insert_timer (e.g.,
insert_timer_and_waiter) to perform the first-waiter push inside its lock;
ensure cancel_waiter() continues to hold mutex_ while removing from
waiters_.remove.
- Around line 810-819: timer_service_create currently dereferences timer_svc_
from timer_service_access::get_scheduler(ioctx) without checking for nullptr;
add a null check after retrieving the pointer returned by
static_cast<timer_service_impl*>(...timer_svc_) and call
detail::throw_logic_error() (or equivalent) if svc is null before calling
svc->create_impl(), so the function safely handles an uninitialized timer_svc_.
🧹 Nitpick comments (2)
src/corosio/src/detail/timer_service.cpp (1)
331-356: Waiter node recycling doesn't resetstop_cb_or other optional state.
create_waiter()hands back a previously-used node without clearingstop_cb_,ec_value_, orimpl_. Whilewait()overwrites most fields andcompletion_op::operator()()resetsstop_cb_before recycling, it's worth verifying that every recycle path (especiallyshutdown(), where nodes are deleted rather than recycled) leaves no stalestop_cb_engaged. A defensivestop_cb_.reset()increate_waiter()(or a reinitialise helper) would make the contract self-documenting.include/boost/corosio/timer.hpp (1)
218-222:duration_casttruncation in templateexpires_afteris worth a@note.The
duration_castsilently truncates sub-nanosecond precision (e.g.,std::chrono::duration<double>→steady_clock::duration). Consider adding a brief@notethat the duration is truncated (not rounded) to the native resolution, mirroring Asio's documentation of the same overload.
Refactor timer internals to support multiple concurrent waiters on a single timer via per-waiter waiter_node linked with intrusive_list. Add constructors with initial expiry, cancel_one() for FIFO single cancellation, and return cancelled waiter counts from cancel(), expires_at(), and expires_after(). Add a waiter node cache for the wait-then-complete hot path. Introduce scheduler_impl as a private intermediary between the public scheduler interface and concrete backend schedulers (epoll, select, kqueue, iocp). Move timer_svc_ from each backend into scheduler_impl and remove the timer_svc() pure virtual from the public scheduler interface. Timer service lookup in timer_service_create now goes through timer_service_access to get the scheduler_impl and its cached timer_svc_ pointer. Update timer guide documentation with cancel_one, multi-waiter usage, and return values. Increase timer durations in cancel_one and stop-token-cancels-one tests from 50ms to 500ms to avoid CI flakiness on slow machines.
2 participants
Refactor timer internals to support multiple concurrent waiters on a single timer via per-waiter waiter_node linked with intrusive_list. Add constructors with initial expiry, cancel_one() for FIFO single cancellation, and return cancelled waiter counts from cancel(), expires_at(), and expires_after(). Add a waiter node cache for the wait-then-complete hot path.
Introduce scheduler_impl as a private intermediary between the public scheduler interface and concrete backend schedulers (epoll, select, kqueue, iocp). Move timer_svc_ from each backend into scheduler_impl and remove the timer_svc() pure virtual from the public scheduler interface. Timer service lookup in timer_service_create now goes through timer_service_access to get the scheduler_impl and its cached timer_svc_ pointer. Update timer guide documentation with cancel_one, multi-waiter usage, and return values.
Resolves #126.
Resolves #97.
Summary by CodeRabbit
New Features
Improvements