-
Notifications
You must be signed in to change notification settings - Fork 6
Refactor details directory structure #32
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
Conversation
📝 WalkthroughWalkthroughThis pull request consolidates backend selection from platform-based detection ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span 30+ files with consistent, homogeneous renaming patterns (POSIX→EPOLL, WIN→IOCP). While the volume is substantial, the repetitive nature of the refactoring—identical renaming applied across multiple files with similar structure—reduces per-file complexity. No new logic, behavioral changes, or intricate cross-component interactions are introduced. Primary review effort focuses on verifying naming consistency and correct macro guard application across the codebase. Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
An automated preview of the documentation is available at https://32.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-01-19 21:27:58 UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/corosio/src/detail/iocp/resolver_service.cpp (1)
346-346: Closing comment mismatch with opening guard.The opening guard at line 12 uses
BOOST_COROSIO_BACKEND_IOCP, but the closing#endifcomment references the old_WIN32guard.Suggested fix
-#endif // _WIN32 +#endif // BOOST_COROSIO_BACKEND_IOCPsrc/corosio/src/detail/win/signals.cpp (1)
676-676: Closing comment mismatch with opening guard.The opening guard at line 12 uses
BOOST_COROSIO_SIGNAL_WIN, but the closing#endifcomment references the old_WIN32guard.Suggested fix
-#endif // _WIN32 +#endif // BOOST_COROSIO_SIGNAL_WINsrc/corosio/src/detail/iocp/overlapped_op.hpp (1)
87-99: Incorrect indentation in error handling logic.The
else ifblock for EOF detection (lines 94-98) has incorrect indentation that breaks the visual structure of the if-else chain. This makes the code misleading and error-prone during maintenance.Proposed fix for indentation
if (ec_out) { if (cancelled.load(std::memory_order_acquire)) *ec_out = make_error_code(system::errc::operation_canceled); else if (error != 0) *ec_out = system::error_code( static_cast<int>(error), system::system_category()); - else if (is_read_operation() && bytes_transferred == 0) - { - // EOF: 0 bytes transferred with no error indicates end of stream - *ec_out = make_error_code(capy::error::eof); - } + else if (is_read_operation() && bytes_transferred == 0) + { + // EOF: 0 bytes transferred with no error indicates end of stream + *ec_out = make_error_code(capy::error::eof); + } }
🤖 Fix all issues with AI agents
In `@src/corosio/src/detail/config_backend.hpp`:
- Around line 28-48: The platform defaulting in config_backend.hpp
unconditionally defines BOOST_COROSIO_BACKEND_* and thus overrides user
overrides and can produce multiple backend macros; change the logic so defaults
are only defined when none of the backend macros (e.g.,
BOOST_COROSIO_BACKEND_IOCP, BOOST_COROSIO_BACKEND_IO_URING,
BOOST_COROSIO_BACKEND_EPOLL, BOOST_COROSIO_BACKEND_KQUEUE,
BOOST_COROSIO_BACKEND_POLL) are already defined, and add a compile-time sanity
check that triggers a preprocessor error if more than one
BOOST_COROSIO_BACKEND_* is defined (or none after platform/default resolution);
update the platform branches (the `#if` defined(_WIN32), __linux__, __APPLE__, BSD
branch, and the fallback) to guard their `#define` with a prior check for existing
backend defines and add a final block that checks for multiple definitions and
emits `#error` if found.
In `@src/corosio/src/detail/posix/signals.hpp`:
- Around line 37-38: The header incorrectly forward-declares epoll_scheduler for
all POSIX signal builds, coupling posix_signals to epoll; change the forward
declaration to be backend-aware by either (A) replacing epoll_scheduler with a
generic scheduler type used across backends (e.g., scheduler or io_scheduler)
and update posix_signals to use that abstract scheduler symbol, or (B) guard the
epoll_scheduler forward-declaration and any epoll-only uses with the EPOLL
backend macro (e.g., BOOST_COROSIO_BACKEND_EPOLL) so non-epoll backends
(KQUEUE/POLL/IO_URING) do not see epoll_scheduler; update references in
posix_signals and other usages to match the selected symbol.
In `@src/corosio/src/detail/timer_service.cpp`:
- Around line 379-409: The wait() method currently reads and writes shared
fields (heap_index_, waiting_, h_, d_, token_, ec_out_) without holding the
timer service mutex, allowing a race with timer_service_impl methods
(process_expired(), update_timer(), cancel_timer()); change wait() so the
check-and-set is performed under the same mutex used by timer_service_impl: move
the logic that checks heap_index_ and sets waiting_, h_, d_, token_, ec_out_
into timer_service_impl (or call into a new
timer_service_impl::start_wait(timer_impl&, h, d, token, ec) method) while
holding the lock, and only perform the executor dispatch/resume
(svc_->get_scheduler().on_work_started() and symmetric-transfer handling)
outside the lock; ensure all code paths that remove the timer from the heap
update heap_index_ under the same mutex so the operation is atomic with respect
to wait().
In `@src/corosio/src/socket.cpp`:
- Around line 13-19: The inclusion guard currently only handles
BOOST_COROSIO_BACKEND_IOCP and BOOST_COROSIO_BACKEND_EPOLL, leaving
socket_service and socket_impl_type undefined for other backends defined in
config_backend.hpp (KQUEUE/POLL/IO_URING); update the preprocessor branching in
src/corosio/src/socket.cpp to either add includes/implementations for those
backends or provide an explicit fallback `#error` that clearly states the
unsupported backend and the missing socket_service/socket_impl_type definitions
(repeat the same fix for the similar block at lines 27-33), referencing the
symbols socket_service and socket_impl_type so the compiler errors become
actionable until those backends are implemented.
🧹 Nitpick comments (9)
src/corosio/src/detail/timer_service.cpp (2)
360-367: Consider using std::swap for efficiency.The
swap_heapfunction manually copies heap entries. Usingstd::swapwould be more idiomatic and potentially more efficient.♻️ Proposed refactor
void swap_heap(std::size_t i1, std::size_t i2) { - heap_entry tmp = heap_[i1]; - heap_[i1] = heap_[i2]; - heap_[i2] = tmp; + std::swap(heap_[i1], heap_[i2]); heap_[i1].timer_->heap_index_ = i1; heap_[i2].timer_->heap_index_ = i2; }
191-194: Consider extracting the coroutine resume pattern.The pattern of dispatching through an executor and conditionally resuming appears four times (lines 191-194, 234-237, 290-294, 396-399). Extracting this into a helper function would reduce duplication.
♻️ Proposed helper function
Add a helper function in the anonymous namespace or as a private static method:
namespace { void dispatch_and_resume( capy::any_executor_ref& executor, std::coroutine_handle<> handle) { auto resume_h = executor.dispatch(handle); if (resume_h.address() == handle.address()) resume_h.resume(); } }Then replace the repeated pattern with:
dispatch_and_resume(d, h);Also applies to: 234-237, 290-294, 396-399
src/corosio/src/detail/posix/signals.cpp (1)
10-16: Make the SIGNAL_POSIX → EPOLL dependency explicit.
posix_signalsnow hard-depends onepoll_scheduler; adding a guard prevents invalid macro combos from silently breaking builds.🛠️ Suggested guard
`#include` "src/detail/config_backend.hpp" +#if defined(BOOST_COROSIO_SIGNAL_POSIX) && !defined(BOOST_COROSIO_BACKEND_EPOLL) +#error "BOOST_COROSIO_SIGNAL_POSIX requires BOOST_COROSIO_BACKEND_EPOLL" +#endif + `#if` defined(BOOST_COROSIO_SIGNAL_POSIX)Also applies to: 178-181
src/corosio/src/io_context.cpp (1)
12-29: Add a clear compile-time error when no backend is selected.
This avoids cryptic failures if neither macro is defined.🛠️ Suggested guard
`#include` "src/detail/config_backend.hpp" `#if` defined(BOOST_COROSIO_BACKEND_IOCP) `#include` "src/detail/iocp/scheduler.hpp" `#elif` defined(BOOST_COROSIO_BACKEND_EPOLL) `#include` "src/detail/epoll/scheduler.hpp" +#else +#error "Define BOOST_COROSIO_BACKEND_IOCP or BOOST_COROSIO_BACKEND_EPOLL" `#endif` ... `#if` defined(BOOST_COROSIO_BACKEND_IOCP) using scheduler_type = detail::win_scheduler; `#elif` defined(BOOST_COROSIO_BACKEND_EPOLL) using scheduler_type = detail::epoll_scheduler; +#else +#error "Define BOOST_COROSIO_BACKEND_IOCP or BOOST_COROSIO_BACKEND_EPOLL" `#endif`src/corosio/src/detail/iocp/timers_thread.cpp (1)
121-121: Consider adding a closing comment for consistency.Other files in this PR use closing comments to clarify which
#endifmatches which conditional. Adding a comment here would improve readability and maintain consistency.Suggested fix
-#endif +#endif // BOOST_COROSIO_BACKEND_IOCPsrc/corosio/src/detail/iocp/timers_nt.cpp (1)
196-196: Consider adding a comment to the closing#endif.For consistency with other files in this PR (e.g.,
timers_none.hpp,wsa_init.hpp), consider adding a comment to identify which guard is being closed.Suggested change
-#endif +#endif // BOOST_COROSIO_BACKEND_IOCPsrc/corosio/src/detail/iocp/timers.cpp (1)
37-37: Consider adding a comment to the closing#endif.For consistency with header files in this PR, consider annotating the closing guard.
Suggested change
-#endif +#endif // BOOST_COROSIO_BACKEND_IOCPsrc/corosio/src/resolver.cpp (1)
14-18: Add compile-time error for undefined backend.If neither
BOOST_COROSIO_BACKEND_IOCPnorBOOST_COROSIO_BACKEND_EPOLLis defined, this file will fail to compile with unclear errors due to missing type aliases and includes. Consider adding an#elsewith#errorto provide a clear diagnostic.Suggested improvement
`#if` defined(BOOST_COROSIO_BACKEND_IOCP) `#include` "src/detail/iocp/resolver_service.hpp" `#elif` defined(BOOST_COROSIO_BACKEND_EPOLL) `#include` "src/detail/epoll/resolver_service.hpp" +#else +#error "No backend defined. Define BOOST_COROSIO_BACKEND_IOCP or BOOST_COROSIO_BACKEND_EPOLL." `#endif`src/corosio/src/acceptor.cpp (1)
14-18: Add compile-time error for undefined backend.Same issue as in
resolver.cpp— if neither backend macro is defined, compilation will fail with unclear errors. Add an#elsewith#errorfor a clear diagnostic.Suggested improvement
`#if` defined(BOOST_COROSIO_BACKEND_IOCP) `#include` "src/detail/iocp/sockets.hpp" `#elif` defined(BOOST_COROSIO_BACKEND_EPOLL) `#include` "src/detail/epoll/sockets.hpp" +#else +#error "No backend defined. Define BOOST_COROSIO_BACKEND_IOCP or BOOST_COROSIO_BACKEND_EPOLL." `#endif`
| #if defined(_WIN32) | ||
| #define BOOST_COROSIO_BACKEND_IOCP 1 | ||
| #define BOOST_COROSIO_SIGNAL_WIN 1 | ||
| #elif defined(__linux__) | ||
| #if defined(BOOST_COROSIO_USE_IO_URING) | ||
| #define BOOST_COROSIO_BACKEND_IO_URING 1 | ||
| #else | ||
| #define BOOST_COROSIO_BACKEND_EPOLL 1 | ||
| #endif | ||
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | ||
| #elif defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__DragonFly__) | ||
| #define BOOST_COROSIO_BACKEND_KQUEUE 1 | ||
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | ||
| #elif defined(__APPLE__) | ||
| #define BOOST_COROSIO_BACKEND_KQUEUE 1 | ||
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | ||
| #else | ||
| // Fallback to poll for other POSIX systems | ||
| #define BOOST_COROSIO_BACKEND_POLL 1 | ||
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow explicit backend overrides and prevent multiple backend defines.
This block always defines platform defaults, which defeats explicit backend selection and can yield multiple BOOST_COROSIO_BACKEND_* macros (e.g., user-defined IO_URING plus default EPOLL). That leads to ambiguous downstream #if selection. Consider only defining defaults when no backend is already selected and add a single-backend sanity check.
🛠️ Proposed fix (guard defaults + single-backend check)
-#if defined(_WIN32)
- `#define` BOOST_COROSIO_BACKEND_IOCP 1
- `#define` BOOST_COROSIO_SIGNAL_WIN 1
-#elif defined(__linux__)
- `#if` defined(BOOST_COROSIO_USE_IO_URING)
- `#define` BOOST_COROSIO_BACKEND_IO_URING 1
- `#else`
- `#define` BOOST_COROSIO_BACKEND_EPOLL 1
- `#endif`
- `#define` BOOST_COROSIO_SIGNAL_POSIX 1
-#elif defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__DragonFly__)
- `#define` BOOST_COROSIO_BACKEND_KQUEUE 1
- `#define` BOOST_COROSIO_SIGNAL_POSIX 1
-#elif defined(__APPLE__)
- `#define` BOOST_COROSIO_BACKEND_KQUEUE 1
- `#define` BOOST_COROSIO_SIGNAL_POSIX 1
-#else
- // Fallback to poll for other POSIX systems
- `#define` BOOST_COROSIO_BACKEND_POLL 1
- `#define` BOOST_COROSIO_SIGNAL_POSIX 1
-#endif
+#if !defined(BOOST_COROSIO_BACKEND_IOCP) && \
+ !defined(BOOST_COROSIO_BACKEND_EPOLL) && \
+ !defined(BOOST_COROSIO_BACKEND_IO_URING) && \
+ !defined(BOOST_COROSIO_BACKEND_KQUEUE) && \
+ !defined(BOOST_COROSIO_BACKEND_POLL)
+ `#if` defined(_WIN32)
+ `#define` BOOST_COROSIO_BACKEND_IOCP 1
+ `#elif` defined(__linux__)
+ `#if` defined(BOOST_COROSIO_USE_IO_URING)
+ `#define` BOOST_COROSIO_BACKEND_IO_URING 1
+ `#else`
+ `#define` BOOST_COROSIO_BACKEND_EPOLL 1
+ `#endif`
+ `#elif` defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__DragonFly__)
+ `#define` BOOST_COROSIO_BACKEND_KQUEUE 1
+ `#elif` defined(__APPLE__)
+ `#define` BOOST_COROSIO_BACKEND_KQUEUE 1
+ `#else`
+ // Fallback to poll for other POSIX systems
+ `#define` BOOST_COROSIO_BACKEND_POLL 1
+ `#endif`
+#endif
+
+#if defined(_WIN32)
+ `#ifndef` BOOST_COROSIO_SIGNAL_WIN
+ `#define` BOOST_COROSIO_SIGNAL_WIN 1
+ `#endif`
+#else
+ `#ifndef` BOOST_COROSIO_SIGNAL_POSIX
+ `#define` BOOST_COROSIO_SIGNAL_POSIX 1
+ `#endif`
+#endif
+
+#if (defined(BOOST_COROSIO_BACKEND_IOCP) + defined(BOOST_COROSIO_BACKEND_EPOLL) + \
+ defined(BOOST_COROSIO_BACKEND_IO_URING) + defined(BOOST_COROSIO_BACKEND_KQUEUE) + \
+ defined(BOOST_COROSIO_BACKEND_POLL)) != 1
+#error "Select exactly one BOOST_COROSIO_BACKEND_*"
+#endif📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #if defined(_WIN32) | |
| #define BOOST_COROSIO_BACKEND_IOCP 1 | |
| #define BOOST_COROSIO_SIGNAL_WIN 1 | |
| #elif defined(__linux__) | |
| #if defined(BOOST_COROSIO_USE_IO_URING) | |
| #define BOOST_COROSIO_BACKEND_IO_URING 1 | |
| #else | |
| #define BOOST_COROSIO_BACKEND_EPOLL 1 | |
| #endif | |
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | |
| #elif defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__DragonFly__) | |
| #define BOOST_COROSIO_BACKEND_KQUEUE 1 | |
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | |
| #elif defined(__APPLE__) | |
| #define BOOST_COROSIO_BACKEND_KQUEUE 1 | |
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | |
| #else | |
| // Fallback to poll for other POSIX systems | |
| #define BOOST_COROSIO_BACKEND_POLL 1 | |
| #define BOOST_COROSIO_SIGNAL_POSIX 1 | |
| #endif | |
| `#if` !defined(BOOST_COROSIO_BACKEND_IOCP) && \ | |
| !defined(BOOST_COROSIO_BACKEND_EPOLL) && \ | |
| !defined(BOOST_COROSIO_BACKEND_IO_URING) && \ | |
| !defined(BOOST_COROSIO_BACKEND_KQUEUE) && \ | |
| !defined(BOOST_COROSIO_BACKEND_POLL) | |
| `#if` defined(_WIN32) | |
| `#define` BOOST_COROSIO_BACKEND_IOCP 1 | |
| `#elif` defined(__linux__) | |
| `#if` defined(BOOST_COROSIO_USE_IO_URING) | |
| `#define` BOOST_COROSIO_BACKEND_IO_URING 1 | |
| `#else` | |
| `#define` BOOST_COROSIO_BACKEND_EPOLL 1 | |
| `#endif` | |
| `#elif` defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__DragonFly__) | |
| `#define` BOOST_COROSIO_BACKEND_KQUEUE 1 | |
| `#elif` defined(__APPLE__) | |
| `#define` BOOST_COROSIO_BACKEND_KQUEUE 1 | |
| `#else` | |
| // Fallback to poll for other POSIX systems | |
| `#define` BOOST_COROSIO_BACKEND_POLL 1 | |
| `#endif` | |
| `#endif` | |
| `#if` defined(_WIN32) | |
| `#ifndef` BOOST_COROSIO_SIGNAL_WIN | |
| `#define` BOOST_COROSIO_SIGNAL_WIN 1 | |
| `#endif` | |
| `#else` | |
| `#ifndef` BOOST_COROSIO_SIGNAL_POSIX | |
| `#define` BOOST_COROSIO_SIGNAL_POSIX 1 | |
| `#endif` | |
| `#endif` | |
| `#if` (defined(BOOST_COROSIO_BACKEND_IOCP) + defined(BOOST_COROSIO_BACKEND_EPOLL) + \ | |
| defined(BOOST_COROSIO_BACKEND_IO_URING) + defined(BOOST_COROSIO_BACKEND_KQUEUE) + \ | |
| defined(BOOST_COROSIO_BACKEND_POLL)) != 1 | |
| `#error` "Select exactly one BOOST_COROSIO_BACKEND_*" | |
| `#endif` |
🤖 Prompt for AI Agents
In `@src/corosio/src/detail/config_backend.hpp` around lines 28 - 48, The platform
defaulting in config_backend.hpp unconditionally defines BOOST_COROSIO_BACKEND_*
and thus overrides user overrides and can produce multiple backend macros;
change the logic so defaults are only defined when none of the backend macros
(e.g., BOOST_COROSIO_BACKEND_IOCP, BOOST_COROSIO_BACKEND_IO_URING,
BOOST_COROSIO_BACKEND_EPOLL, BOOST_COROSIO_BACKEND_KQUEUE,
BOOST_COROSIO_BACKEND_POLL) are already defined, and add a compile-time sanity
check that triggers a preprocessor error if more than one
BOOST_COROSIO_BACKEND_* is defined (or none after platform/default resolution);
update the platform branches (the `#if` defined(_WIN32), __linux__, __APPLE__, BSD
branch, and the fallback) to guard their `#define` with a prior check for existing
backend defines and add a final block that checks for multiple definitions and
emits `#error` if found.
| class epoll_scheduler; | ||
| class posix_signals; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix scheduler type coupling to epoll-only.
This header is compiled whenever BOOST_COROSIO_SIGNAL_POSIX is set, but it now hard-codes epoll_scheduler. That will fail on non-epoll backends (KQUEUE/POLL/IO_URING). Consider selecting the scheduler type by backend or tightening the guard to EPOLL-only until other backends are wired.
🛠️ Proposed fix (backend-selected scheduler or explicit error)
-class epoll_scheduler;
+#if defined(BOOST_COROSIO_BACKEND_EPOLL)
+class epoll_scheduler;
+using signal_scheduler = epoll_scheduler;
+#else
+#error "POSIX signals need a scheduler mapping for the selected backend."
+#endif
@@
- epoll_scheduler& sched_;
+ signal_scheduler& sched_;Also applies to: 213-214
🤖 Prompt for AI Agents
In `@src/corosio/src/detail/posix/signals.hpp` around lines 37 - 38, The header
incorrectly forward-declares epoll_scheduler for all POSIX signal builds,
coupling posix_signals to epoll; change the forward declaration to be
backend-aware by either (A) replacing epoll_scheduler with a generic scheduler
type used across backends (e.g., scheduler or io_scheduler) and update
posix_signals to use that abstract scheduler symbol, or (B) guard the
epoll_scheduler forward-declaration and any epoll-only uses with the EPOLL
backend macro (e.g., BOOST_COROSIO_BACKEND_EPOLL) so non-epoll backends
(KQUEUE/POLL/IO_URING) do not see epoll_scheduler; update references in
posix_signals and other usages to match the selected symbol.
| void | ||
| timer_impl:: | ||
| wait( | ||
| std::coroutine_handle<> h, | ||
| capy::any_executor_ref d, | ||
| std::stop_token token, | ||
| system::error_code* ec) | ||
| { | ||
| // Check if timer already expired (not in heap anymore) | ||
| bool already_expired = (heap_index_ == (std::numeric_limits<std::size_t>::max)()); | ||
|
|
||
| if (already_expired) | ||
| { | ||
| // Timer already expired - dispatch immediately | ||
| if (ec) | ||
| *ec = {}; | ||
| // Note: no work tracking needed - we dispatch synchronously | ||
| auto resume_h = d.dispatch(h); | ||
| // Resume the handle if executor returned it for symmetric transfer | ||
| if (resume_h.address() == h.address()) | ||
| resume_h.resume(); | ||
| return; | ||
| } | ||
|
|
||
| h_ = h; | ||
| d_ = std::move(d); | ||
| token_ = std::move(token); | ||
| ec_out_ = ec; | ||
| waiting_ = true; | ||
| svc_->get_scheduler().on_work_started(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Data race in wait() method.
The wait() method accesses shared state (heap_index_, waiting_, h_, etc.) without holding the mutex, while other threads (in update_timer, cancel_timer, and process_expired) access these same fields under lock.
Race condition scenario:
- Thread A (user coroutine): calls
wait(), readsheap_index_at line 388 (timer is in heap), proceeds to line 403 - Thread B (timer service): calls
process_expired(), acquires lock, checkswaiting_ == false(not set yet by Thread A), removes timer from heap and updatesheap_index_, releases lock - Thread A: sets
waiting_ = trueat line 407
Result: Timer is removed from heap but waiting_ is true, so the coroutine will never be resumed.
The wait() method needs to be redesigned to properly synchronize with the timer service. Consider moving this logic into timer_service_impl under the mutex, or at minimum, the check-and-set of the waiting state must be atomic with respect to the timer service operations.
🤖 Prompt for AI Agents
In `@src/corosio/src/detail/timer_service.cpp` around lines 379 - 409, The wait()
method currently reads and writes shared fields (heap_index_, waiting_, h_, d_,
token_, ec_out_) without holding the timer service mutex, allowing a race with
timer_service_impl methods (process_expired(), update_timer(), cancel_timer());
change wait() so the check-and-set is performed under the same mutex used by
timer_service_impl: move the logic that checks heap_index_ and sets waiting_,
h_, d_, token_, ec_out_ into timer_service_impl (or call into a new
timer_service_impl::start_wait(timer_impl&, h, d, token, ec) method) while
holding the lock, and only perform the executor dispatch/resume
(svc_->get_scheduler().on_work_started() and symmetric-transfer handling)
outside the lock; ensure all code paths that remove the timer from the heap
update heap_index_ under the same mutex so the operation is atomic with respect
to wait().
| #include "src/detail/config_backend.hpp" | ||
|
|
||
| #if defined(BOOST_COROSIO_BACKEND_IOCP) | ||
| #include "src/detail/iocp/sockets.hpp" | ||
| #elif defined(BOOST_COROSIO_BACKEND_EPOLL) | ||
| #include "src/detail/epoll/sockets.hpp" | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add handling (or explicit error) for non-IOCP/EPOLL backends.
With config_backend.hpp now defining KQUEUE/POLL/IO_URING on those platforms, socket_service and socket_impl_type are left undefined here, which yields hard-to-decipher compile errors. Add branches or an explicit #error fallback until those backends are implemented.
🛠️ Suggested explicit fallback
`#if` defined(BOOST_COROSIO_BACKEND_IOCP)
`#include` "src/detail/iocp/sockets.hpp"
`#elif` defined(BOOST_COROSIO_BACKEND_EPOLL)
`#include` "src/detail/epoll/sockets.hpp"
+#else
+#error "Unsupported backend in src/corosio/src/socket.cpp"
`#endif`
@@
`#if` defined(BOOST_COROSIO_BACKEND_IOCP)
using socket_service = detail::win_sockets;
using socket_impl_type = detail::win_socket_impl;
`#elif` defined(BOOST_COROSIO_BACKEND_EPOLL)
using socket_service = detail::epoll_sockets;
using socket_impl_type = detail::epoll_socket_impl;
+#else
+#error "Unsupported backend in src/corosio/src/socket.cpp"
`#endif`Also applies to: 27-33
🤖 Prompt for AI Agents
In `@src/corosio/src/socket.cpp` around lines 13 - 19, The inclusion guard
currently only handles BOOST_COROSIO_BACKEND_IOCP and
BOOST_COROSIO_BACKEND_EPOLL, leaving socket_service and socket_impl_type
undefined for other backends defined in config_backend.hpp
(KQUEUE/POLL/IO_URING); update the preprocessor branching in
src/corosio/src/socket.cpp to either add includes/implementations for those
backends or provide an explicit fallback `#error` that clearly states the
unsupported backend and the missing socket_service/socket_impl_type definitions
(repeat the same fix for the similar block at lines 27-33), referencing the
symbols socket_service and socket_impl_type so the compiler errors become
actionable until those backends are implemented.
Summary by CodeRabbit
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.