Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 42 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (64)
📝 WalkthroughWalkthroughThis PR refactors backend implementations into a traits/types-based reactor layer: it removes many per-backend op/service/socket headers (epoll/kqueue/select), adds new backend traits/types, and introduces generic reactor templates and helpers to centralize socket/acceptor/service logic. (49 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Acceptor as Reactor Acceptor (CRTP)
participant Traits as Traits::accept_policy
participant Kernel as OS (accept/accept4)
participant Scheduler as Reactor Scheduler
participant SocketSvc as Stream Service / Socket Final
participant Continuation as Awaiting Coroutine
Acceptor->>Traits: call do_accept(fd)
Traits->>Kernel: sys_accept (accept4 / accept)
Kernel-->>Traits: fd or errno
alt accepted fd
Traits->>Scheduler: register_descriptor(fd)
Scheduler-->>SocketSvc: create/register socket impl
SocketSvc-->>Acceptor: set endpoints, set *impl_out, *ec = success
Acceptor->>Continuation: dispatch continuation (resume)
else EAGAIN / would-block
Traits->>Scheduler: work_started / set desc_state.read_op
Scheduler-->>Acceptor: later readiness triggers op completion
Acceptor->>Continuation: post completion (resume)
else error
Traits->>Acceptor: populate errno -> *ec
Acceptor->>Continuation: post failure (resume)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
An automated preview of the documentation is available at https://232.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-04-16 21:37:30 UTC |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/corosio/native/detail/reactor/reactor_datagram_socket.hpp (1)
690-692:⚠️ Potential issue | 🟠 MajorPass the write-direction flag for datagram
connect()too.
do_connect()also waits on writability, so leavingis_write_directionat its defaultfalsemeans select schedulers may not interrupt the currentselect()loop when this op is registered after the fd-set snapshot. That can stall nonblocking datagram connects until an unrelated event arrives.Suggested fix
this->register_op( op, this->desc_state_.connect_op, this->desc_state_.write_ready, - this->desc_state_.connect_cancel_pending); + this->desc_state_.connect_cancel_pending, true);Based on learnings: In the select backend, write/connect ops registered after the snapshot must notify the reactor so the next iteration picks them up in
write_fds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/reactor/reactor_datagram_socket.hpp` around lines 690 - 692, The datagram connect registration omits the write-direction flag so the select backend may not wake for connect operations; update the register_op call in the datagram connect path (the place calling this->register_op with this->desc_state_.connect_op, this->desc_state_.write_ready, this->desc_state_.connect_cancel_pending) to pass is_write_direction=true (matching do_connect’s writability wait) so the reactor/select backend will mark the op as write-directed and notify the reactor for correct write_fds handling.
🧹 Nitpick comments (1)
include/boost/corosio/native/detail/kqueue/kqueue_types.hpp (1)
17-28: Place the file overview block after the include block.The high-level
/* */overview is useful, but this guideline expects it after includes for implementation files with non-trivial logic.♻️ Suggested placement update
-/* Named per-backend types for the kqueue reactor. - - Each class is a final, named wrapper around the parameterized - reactor_*_impl templates. Forward-declarable from backend.hpp - so the concrete layer never pulls in platform headers. -*/ - `#include` <boost/corosio/detail/config.hpp> `#include` <boost/corosio/native/detail/kqueue/kqueue_traits.hpp> `#include` <boost/corosio/native/detail/kqueue/kqueue_scheduler.hpp> `#include` <boost/corosio/native/detail/reactor/reactor_backend.hpp> + +/* Named per-backend types for the kqueue reactor. + + Each class is a final, named wrapper around the parameterized + reactor_*_impl templates. Forward-declarable from backend.hpp + so the concrete layer never pulls in platform headers. +*/As per coding guidelines: “Files containing non-trivial implementation logic should include a
/* */block comment after the includes that provides a high-level overview…”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/kqueue/kqueue_types.hpp` around lines 17 - 28, Move the top-level block comment that describes "Named per-backend types for the kqueue reactor" so it appears immediately after the include directives (the lines including kqueue_traits.hpp, kqueue_scheduler.hpp, and reactor_backend.hpp) instead of before them; update the file-level overview placement in kqueue_types.hpp so the /* ... */ overview follows the `#include` block and precedes the rest of the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/native/detail/reactor/reactor_stream_socket.hpp`:
- Around line 233-242: The current override of do_release_socket() clears
remote_endpoint_ after calling base_type::do_release_socket(), which breaks
established reactor release semantics; update the override in
reactor_stream_socket::do_release_socket() to call
base_type::do_release_socket() and return its fd without modifying
remote_endpoint_, and ensure only reactor_basic_socket resets local_endpoint_
(leave remote_endpoint_ intact) so the cached remote endpoint is preserved
across release().
In `@include/boost/corosio/native/detail/select/select_traits.hpp`:
- Around line 142-152: The current code treats a failed setsockopt(SO_NOSIGPIPE,
...) as fatal by closing new_fd and returning -1; instead make SO_NOSIGPIPE
best-effort in the select backend by ignoring errors from ::setsockopt for
SO_NOSIGPIPE — do not close new_fd or return on failure (optionally log/debug
the errno), and proceed normally after the setsockopt call; apply the same
change to the other SO_NOSIGPIPE branch(s) in select_traits.hpp that handle
new_fd/accept.
---
Outside diff comments:
In `@include/boost/corosio/native/detail/reactor/reactor_datagram_socket.hpp`:
- Around line 690-692: The datagram connect registration omits the
write-direction flag so the select backend may not wake for connect operations;
update the register_op call in the datagram connect path (the place calling
this->register_op with this->desc_state_.connect_op,
this->desc_state_.write_ready, this->desc_state_.connect_cancel_pending) to pass
is_write_direction=true (matching do_connect’s writability wait) so the
reactor/select backend will mark the op as write-directed and notify the reactor
for correct write_fds handling.
---
Nitpick comments:
In `@include/boost/corosio/native/detail/kqueue/kqueue_types.hpp`:
- Around line 17-28: Move the top-level block comment that describes "Named
per-backend types for the kqueue reactor" so it appears immediately after the
include directives (the lines including kqueue_traits.hpp, kqueue_scheduler.hpp,
and reactor_backend.hpp) instead of before them; update the file-level overview
placement in kqueue_types.hpp so the /* ... */ overview follows the `#include`
block and precedes the rest of the implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3da70766-115f-4d50-b2c3-5cd5aeafb10c
📒 Files selected for processing (64)
include/boost/corosio/native/detail/epoll/epoll_local_datagram_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_datagram_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_op.hppinclude/boost/corosio/native/detail/epoll/epoll_scheduler.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_acceptor.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_service.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_traits.hppinclude/boost/corosio/native/detail/epoll/epoll_types.hppinclude/boost/corosio/native/detail/epoll/epoll_udp_service.hppinclude/boost/corosio/native/detail/epoll/epoll_udp_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_datagram_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_datagram_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_op.hppinclude/boost/corosio/native/detail/kqueue/kqueue_scheduler.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_traits.hppinclude/boost/corosio/native/detail/kqueue/kqueue_types.hppinclude/boost/corosio/native/detail/kqueue/kqueue_udp_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_udp_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_acceptor.hppinclude/boost/corosio/native/detail/reactor/reactor_acceptor_service.hppinclude/boost/corosio/native/detail/reactor/reactor_backend.hppinclude/boost/corosio/native/detail/reactor/reactor_basic_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_datagram_ops.hppinclude/boost/corosio/native/detail/reactor/reactor_datagram_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_op_complete.hppinclude/boost/corosio/native/detail/reactor/reactor_service_finals.hppinclude/boost/corosio/native/detail/reactor/reactor_socket_finals.hppinclude/boost/corosio/native/detail/reactor/reactor_socket_service.hppinclude/boost/corosio/native/detail/reactor/reactor_stream_ops.hppinclude/boost/corosio/native/detail/reactor/reactor_stream_socket.hppinclude/boost/corosio/native/detail/select/select_local_datagram_service.hppinclude/boost/corosio/native/detail/select/select_local_datagram_socket.hppinclude/boost/corosio/native/detail/select/select_local_stream_acceptor.hppinclude/boost/corosio/native/detail/select/select_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/select/select_local_stream_service.hppinclude/boost/corosio/native/detail/select/select_local_stream_socket.hppinclude/boost/corosio/native/detail/select/select_op.hppinclude/boost/corosio/native/detail/select/select_scheduler.hppinclude/boost/corosio/native/detail/select/select_tcp_acceptor.hppinclude/boost/corosio/native/detail/select/select_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/select/select_tcp_service.hppinclude/boost/corosio/native/detail/select/select_tcp_socket.hppinclude/boost/corosio/native/detail/select/select_traits.hppinclude/boost/corosio/native/detail/select/select_types.hppinclude/boost/corosio/native/detail/select/select_udp_service.hppinclude/boost/corosio/native/detail/select/select_udp_socket.hppinclude/boost/corosio/native/native_tcp_acceptor.hppinclude/boost/corosio/native/native_tcp_socket.hppinclude/boost/corosio/native/native_udp_socket.hppsrc/corosio/src/io_context.cpp
💤 Files with no reviewable changes (39)
- include/boost/corosio/native/detail/epoll/epoll_udp_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_socket.hpp
- include/boost/corosio/native/detail/select/select_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_udp_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_udp_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor.hpp
- include/boost/corosio/native/detail/select/select_tcp_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_acceptor.hpp
- include/boost/corosio/native/detail/select/select_local_datagram_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/select/select_tcp_acceptor.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_datagram_service.hpp
- include/boost/corosio/native/detail/select/select_local_datagram_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_datagram_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_op.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_service.hpp
- include/boost/corosio/native/detail/select/select_tcp_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_op.hpp
- include/boost/corosio/native/detail/select/select_local_stream_service.hpp
- include/boost/corosio/native/detail/select/select_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_udp_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_datagram_socket.hpp
- include/boost/corosio/native/detail/select/select_udp_socket.hpp
- include/boost/corosio/native/detail/select/select_op.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/select/select_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_datagram_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_socket.hpp
- include/boost/corosio/native/detail/select/select_local_stream_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_socket.hpp
- include/boost/corosio/native/detail/select/select_udp_service.hpp
|
GCOVR code coverage report https://232.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-04-16 21:45:36 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #232 +/- ##
===========================================
- Coverage 77.73% 77.71% -0.02%
===========================================
Files 96 96
Lines 7306 7298 -8
Branches 1787 1787
===========================================
- Hits 5679 5672 -7
+ Misses 1109 1108 -1
Partials 518 518
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
mvandeberg
force-pushed
the
pr/reactor-consolidation
branch
from
0d4753c to
ab7ec63
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/corosio/native/detail/reactor/reactor_datagram_socket.hpp (1)
690-692:⚠️ Potential issue | 🟠 MajorWake select when parking datagram connect ops.
connect_opis also a write-side wait in the select backend. Because this call leavesis_write_directionat its default, a UDP/local-datagramconnect()that hitsEINPROGRESSafterselect()snapshotswrite_fdscan sit idle until some unrelated event wakes the loop.Suggested fix
this->register_op( op, this->desc_state_.connect_op, this->desc_state_.write_ready, - this->desc_state_.connect_cancel_pending); + this->desc_state_.connect_cancel_pending, true);Based on learnings: In the select backend, write/connect ops registered after the snapshot must call
notify_reactor()so the next iteration picks them up inwrite_fds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/reactor/reactor_datagram_socket.hpp` around lines 690 - 692, The connect_op is a write-side wait in the select backend and must wake the select loop if registered after its snapshot; update the registration code around register_op(op, this->desc_state_.connect_op, this->desc_state_.write_ready, this->desc_state_.connect_cancel_pending) so that after registering a write/connect op you call notify_reactor() (or set the is_write_direction flag to true when calling register_op if that overload exists) — specifically ensure the path that registers desc_state_.connect_op triggers notify_reactor() so the next select iteration includes the new write_fds.
🧹 Nitpick comments (3)
include/boost/corosio/native/detail/reactor/reactor_backend.hpp (1)
94-100: Consider a more descriptive error thanENOENTfor a missing stream service.A null
stream_service()here indicates an internal service-wiring bug (per existing learnings,io_contextalways co-creates socket and acceptor services, so this branch should be unreachable in practice).ENOENT("no such file or directory") is a confusing choice to surface to user code for what is effectively an invariant violation. ConsiderEINVAL,ENOPROTOOPT, or a library-specificcapy::error::...value to make debugging easier if this branch ever does fire.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/reactor/reactor_backend.hpp` around lines 94 - 100, The branch that sets *ec = make_err(ENOENT) when stream_service() is null should use a more descriptive error to indicate an internal service-wiring/invariant violation; replace ENOENT with a clearer errno or library-specific error (e.g. EINVAL or a dedicated capy::error::internal_service_error) in the reactor_backend.hpp code path that handles accepted sockets (references: stream_service(), make_err(), accepted, impl_out, ec) so callers see an explicit "internal service missing" error rather than "no such file or directory."include/boost/corosio/native/detail/reactor/reactor_service_finals.hpp (2)
73-128: Consider documentingdo_assign_fdfd ownership on failure.On success this function registers
fdwith the impl (line 105). On the early validation failures (lines 86-98),fdhas not yet been adopted, so the caller retains ownership — butclose_socket()at line 80 has already dropped the impl's previous fd, leaving it "empty". This contract (adopt on success; impl emptied on failure; caller always owns the passed fd until success) is non-obvious. A brief comment at the top, or an invariant note, would help future maintainers. Also note: ifconfigure_local_socket(fd)at line 101 fails after line 80, we've already torn down the old fd without adopting the new one; confirm that's the intended observable state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/reactor/reactor_service_finals.hpp` around lines 73 - 128, Add a short invariant comment at the top of do_assign_fd describing ownership semantics: after calling socket_impl->close_socket() the impl's previous fd is released/emptied and the passed fd remains owned by the caller until socket_impl->init_and_register(fd) succeeds; on any early validation failure (getsockname/getsockopt) or if Traits::configure_local_socket(fd) fails, the caller retains ownership of fd and the impl remains empty. Also add a sentence noting that configure_local_socket failure leaves the impl without a bound fd (intended observable state), and reference the key symbols do_assign_fd, socket_impl->close_socket(), Traits::configure_local_socket, and socket_impl->init_and_register in the comment so future maintainers can find the relevant code paths.
197-205: Inconsistent hook lifecycle wiring between TCP and local-stream services.
reactor_tcp_service_implexposespre_shutdown/pre_destroythat forward toimpl->hook_, butreactor_local_stream_service_impl(lines 212-248) does not — even though both useTraits::stream_socket_hook(seereactor_socket_finals.hpp:79-97). The local-stream backends compensate ad-hoc insiderelease_socket()overrides (e.g.hook_ = {}inkqueue_local_stream_socket/epoll_local_stream_socket), which is asymmetric. Worth either centralizing these hooks in a common base or adding the same forwarders here for consistency and to avoid future backends forgetting one path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/reactor/reactor_service_finals.hpp` around lines 197 - 205, The local-stream service lacks the same hook lifecycle forwarders as TCP: add pre_shutdown and pre_destroy forwarding methods for reactor_local_stream_service_impl that call impl->hook_.pre_shutdown(impl->native_handle()) and impl->hook_.pre_destroy(impl->native_handle()) (matching the signatures and noexcept of the TCP versions), or alternatively move those forwarders into the shared reactor socket base so both reactor_tcp_service_impl and reactor_local_stream_service_impl use the same implementation; ensure you remove ad-hoc hook clearing in release_socket() overrides (kqueue_local_stream_socket / epoll_local_stream_socket) once centralized to avoid double-handling and keep Traits::stream_socket_hook semantics consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/native/detail/reactor/reactor_backend.hpp`:
- Around line 66-112: The inline-budget branch in the accept logic fails to
protect the acceptor lifetime: before returning from the if-branch that calls
dispatch_coro(ex, op.cont_op.cont) you must set op.impl_ptr =
this->shared_from_this() (same pattern used on the async/EAGAIN and post paths)
so the acceptor (this / acc_) is kept alive while the continuation returned by
dispatch_coro may be dispatched asynchronously; add that assignment immediately
prior to the dispatch_coro return in the same block that configures impl and ec.
---
Outside diff comments:
In `@include/boost/corosio/native/detail/reactor/reactor_datagram_socket.hpp`:
- Around line 690-692: The connect_op is a write-side wait in the select backend
and must wake the select loop if registered after its snapshot; update the
registration code around register_op(op, this->desc_state_.connect_op,
this->desc_state_.write_ready, this->desc_state_.connect_cancel_pending) so that
after registering a write/connect op you call notify_reactor() (or set the
is_write_direction flag to true when calling register_op if that overload
exists) — specifically ensure the path that registers desc_state_.connect_op
triggers notify_reactor() so the next select iteration includes the new
write_fds.
---
Nitpick comments:
In `@include/boost/corosio/native/detail/reactor/reactor_backend.hpp`:
- Around line 94-100: The branch that sets *ec = make_err(ENOENT) when
stream_service() is null should use a more descriptive error to indicate an
internal service-wiring/invariant violation; replace ENOENT with a clearer errno
or library-specific error (e.g. EINVAL or a dedicated
capy::error::internal_service_error) in the reactor_backend.hpp code path that
handles accepted sockets (references: stream_service(), make_err(), accepted,
impl_out, ec) so callers see an explicit "internal service missing" error rather
than "no such file or directory."
In `@include/boost/corosio/native/detail/reactor/reactor_service_finals.hpp`:
- Around line 73-128: Add a short invariant comment at the top of do_assign_fd
describing ownership semantics: after calling socket_impl->close_socket() the
impl's previous fd is released/emptied and the passed fd remains owned by the
caller until socket_impl->init_and_register(fd) succeeds; on any early
validation failure (getsockname/getsockopt) or if
Traits::configure_local_socket(fd) fails, the caller retains ownership of fd and
the impl remains empty. Also add a sentence noting that configure_local_socket
failure leaves the impl without a bound fd (intended observable state), and
reference the key symbols do_assign_fd, socket_impl->close_socket(),
Traits::configure_local_socket, and socket_impl->init_and_register in the
comment so future maintainers can find the relevant code paths.
- Around line 197-205: The local-stream service lacks the same hook lifecycle
forwarders as TCP: add pre_shutdown and pre_destroy forwarding methods for
reactor_local_stream_service_impl that call
impl->hook_.pre_shutdown(impl->native_handle()) and
impl->hook_.pre_destroy(impl->native_handle()) (matching the signatures and
noexcept of the TCP versions), or alternatively move those forwarders into the
shared reactor socket base so both reactor_tcp_service_impl and
reactor_local_stream_service_impl use the same implementation; ensure you remove
ad-hoc hook clearing in release_socket() overrides (kqueue_local_stream_socket /
epoll_local_stream_socket) once centralized to avoid double-handling and keep
Traits::stream_socket_hook semantics consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d837de37-f428-495c-bb59-9bda0d05afde
📒 Files selected for processing (64)
include/boost/corosio/native/detail/epoll/epoll_local_datagram_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_datagram_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_op.hppinclude/boost/corosio/native/detail/epoll/epoll_scheduler.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_acceptor.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_service.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_traits.hppinclude/boost/corosio/native/detail/epoll/epoll_types.hppinclude/boost/corosio/native/detail/epoll/epoll_udp_service.hppinclude/boost/corosio/native/detail/epoll/epoll_udp_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_datagram_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_datagram_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_op.hppinclude/boost/corosio/native/detail/kqueue/kqueue_scheduler.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_traits.hppinclude/boost/corosio/native/detail/kqueue/kqueue_types.hppinclude/boost/corosio/native/detail/kqueue/kqueue_udp_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_udp_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_acceptor.hppinclude/boost/corosio/native/detail/reactor/reactor_acceptor_service.hppinclude/boost/corosio/native/detail/reactor/reactor_backend.hppinclude/boost/corosio/native/detail/reactor/reactor_basic_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_datagram_ops.hppinclude/boost/corosio/native/detail/reactor/reactor_datagram_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_op_complete.hppinclude/boost/corosio/native/detail/reactor/reactor_service_finals.hppinclude/boost/corosio/native/detail/reactor/reactor_socket_finals.hppinclude/boost/corosio/native/detail/reactor/reactor_socket_service.hppinclude/boost/corosio/native/detail/reactor/reactor_stream_ops.hppinclude/boost/corosio/native/detail/reactor/reactor_stream_socket.hppinclude/boost/corosio/native/detail/select/select_local_datagram_service.hppinclude/boost/corosio/native/detail/select/select_local_datagram_socket.hppinclude/boost/corosio/native/detail/select/select_local_stream_acceptor.hppinclude/boost/corosio/native/detail/select/select_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/select/select_local_stream_service.hppinclude/boost/corosio/native/detail/select/select_local_stream_socket.hppinclude/boost/corosio/native/detail/select/select_op.hppinclude/boost/corosio/native/detail/select/select_scheduler.hppinclude/boost/corosio/native/detail/select/select_tcp_acceptor.hppinclude/boost/corosio/native/detail/select/select_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/select/select_tcp_service.hppinclude/boost/corosio/native/detail/select/select_tcp_socket.hppinclude/boost/corosio/native/detail/select/select_traits.hppinclude/boost/corosio/native/detail/select/select_types.hppinclude/boost/corosio/native/detail/select/select_udp_service.hppinclude/boost/corosio/native/detail/select/select_udp_socket.hppinclude/boost/corosio/native/native_tcp_acceptor.hppinclude/boost/corosio/native/native_tcp_socket.hppinclude/boost/corosio/native/native_udp_socket.hppsrc/corosio/src/io_context.cpp
💤 Files with no reviewable changes (39)
- include/boost/corosio/native/detail/epoll/epoll_udp_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_socket.hpp
- include/boost/corosio/native/detail/select/select_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/epoll/epoll_udp_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor.hpp
- include/boost/corosio/native/detail/select/select_tcp_acceptor.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/select/select_local_datagram_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_acceptor.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_datagram_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_datagram_service.hpp
- include/boost/corosio/native/detail/select/select_local_datagram_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_udp_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_service.hpp
- include/boost/corosio/native/detail/select/select_tcp_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_service.hpp
- include/boost/corosio/native/detail/select/select_local_stream_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_op.hpp
- include/boost/corosio/native/detail/select/select_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_udp_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/select/select_tcp_service.hpp
- include/boost/corosio/native/detail/select/select_udp_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_op.hpp
- include/boost/corosio/native/detail/select/select_op.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_service.hpp
- include/boost/corosio/native/detail/select/select_local_stream_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_datagram_socket.hpp
- include/boost/corosio/native/detail/select/select_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_datagram_service.hpp
- include/boost/corosio/native/detail/select/select_udp_service.hpp
✅ Files skipped from review due to trivial changes (6)
- include/boost/corosio/native/detail/select/select_scheduler.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_scheduler.hpp
- src/corosio/src/io_context.cpp
- include/boost/corosio/native/native_udp_socket.hpp
- include/boost/corosio/native/native_tcp_acceptor.hpp
- include/boost/corosio/native/detail/reactor/reactor_stream_ops.hpp
🚧 Files skipped from review as they are similar to previous changes (7)
- include/boost/corosio/native/native_tcp_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_scheduler.hpp
- include/boost/corosio/native/detail/reactor/reactor_socket_service.hpp
- include/boost/corosio/native/detail/reactor/reactor_stream_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_traits.hpp
- include/boost/corosio/native/detail/reactor/reactor_acceptor_service.hpp
- include/boost/corosio/native/detail/reactor/reactor_datagram_ops.hpp
mvandeberg
force-pushed
the
pr/reactor-consolidation
branch
from
ab7ec63 to
893a5dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
include/boost/corosio/native/detail/epoll/epoll_types.hpp (2)
85-112: Consider a dedicatedno_acceptorplaceholder instead ofepoll_tcp_acceptorfor datagram wrappers.Both
epoll_udp_socket(Line 88) andepoll_local_datagram_socket(Line 102/107) passepoll_tcp_acceptoras theDummyAcctemplate argument toreactor_dgram_socket_impl. Datagrams have no acceptor, and reusing the TCP acceptor type here is semantically misleading (especially for the local-datagram case, where it also couples UDP-unrelated types into the instantiation). A small empty placeholder type (e.g.,struct no_acceptor {};) exposed from the reactor layer would make intent explicit and avoid spurious cross-type coupling in generated symbol names.Not blocking — purely a readability/maintainability nit.
🤖 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_types.hpp` around lines 85 - 112, The datagram socket specializations epoll_udp_socket and epoll_local_datagram_socket currently pass epoll_tcp_acceptor as the DummyAcc template parameter which is semantically incorrect and couples TCP types into datagram symbols; define a small empty placeholder type (e.g., struct no_acceptor {}) in the reactor layer and replace epoll_tcp_acceptor with that placeholder in the template instantiations for reactor_dgram_socket_impl used by epoll_udp_socket and epoll_local_datagram_socket, and update any related forward/declaration uses so the new no_acceptor is exposed where reactor_dgram_socket_impl is instantiated.
76-128: Makerelease_socket()consistent between local stream and local datagram.
epoll_local_stream_socket::release_socket()resetshook_ = {};before delegating, whileepoll_local_datagram_socket::release_socket()does not. For the currentepoll_traits::stream_socket_hook(stateless) this is a no-op either way, but the asymmetry will silently diverge if the hook ever gains per-socket state (e.g., mirroring BSDSO_NOSIGPIPEtracking in kqueue). Consider aligning both overrides — either both resethook_, or neither does and the base handles it.♻️ Proposed change
native_handle_type release_socket() noexcept override { + hook_ = {}; return this->do_release_socket(); } };🤖 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_types.hpp` around lines 76 - 128, The two release_socket overrides are inconsistent: epoll_local_stream_socket::release_socket() clears hook_ (hook_ = {}) before calling do_release_socket(), but epoll_local_datagram_socket::release_socket() does not; make them consistent by clearing hook_ in epoll_local_datagram_socket::release_socket() as well (or alternatively remove the clear from the stream override and move the hook reset into the shared do_release_socket() implementation) so both epoll_local_stream_socket and epoll_local_datagram_socket behave identically with respect to stream_socket_hook state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/native/detail/reactor/reactor_service_finals.hpp`:
- Around line 104-106: The change calls Traits::configure_local_socket(fd)
inside assign_socket(), which mutates caller-owned flags; restore adopt-only
semantics by removing that call for local descriptors: in the assign_socket()
implementation in reactor_service_finals.hpp, either delete the std::error_code
ec = Traits::configure_local_socket(fd); if (ec) return ec; lines or wrap them
behind a check so they run only for non-local sockets (i.e., skip calling
Traits::configure_local_socket(fd) when assigning a local stream/datagram fd),
leaving assign_socket() as a pure adoption operation.
In `@include/boost/corosio/native/detail/reactor/reactor_socket_finals.hpp`:
- Around line 71-73: The member-initializers for the derived templates use
invalid syntax like reactor_stream_socket_impl::reactor_stream_socket(...);
change them to initialize the base via a typedef/using alias and then call that
alias in the mem-init. For example, inside class reactor_stream_socket_impl add
a using base_type = reactor_stream_socket; (or similar unique alias) and replace
the initializer reactor_stream_socket_impl::reactor_stream_socket(svc) with
base_type(svc); apply the same pattern for the other constructors/members that
currently use Derived::Base(...) so the compiler sees a proper base-type
initializer.
---
Nitpick comments:
In `@include/boost/corosio/native/detail/epoll/epoll_types.hpp`:
- Around line 85-112: The datagram socket specializations epoll_udp_socket and
epoll_local_datagram_socket currently pass epoll_tcp_acceptor as the DummyAcc
template parameter which is semantically incorrect and couples TCP types into
datagram symbols; define a small empty placeholder type (e.g., struct
no_acceptor {}) in the reactor layer and replace epoll_tcp_acceptor with that
placeholder in the template instantiations for reactor_dgram_socket_impl used by
epoll_udp_socket and epoll_local_datagram_socket, and update any related
forward/declaration uses so the new no_acceptor is exposed where
reactor_dgram_socket_impl is instantiated.
- Around line 76-128: The two release_socket overrides are inconsistent:
epoll_local_stream_socket::release_socket() clears hook_ (hook_ = {}) before
calling do_release_socket(), but epoll_local_datagram_socket::release_socket()
does not; make them consistent by clearing hook_ in
epoll_local_datagram_socket::release_socket() as well (or alternatively remove
the clear from the stream override and move the hook reset into the shared
do_release_socket() implementation) so both epoll_local_stream_socket and
epoll_local_datagram_socket behave identically with respect to
stream_socket_hook state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 556747ba-9c3b-48da-84b5-4c1d076af245
📒 Files selected for processing (64)
include/boost/corosio/native/detail/epoll/epoll_local_datagram_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_datagram_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_service.hppinclude/boost/corosio/native/detail/epoll/epoll_local_stream_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_op.hppinclude/boost/corosio/native/detail/epoll/epoll_scheduler.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_acceptor.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_service.hppinclude/boost/corosio/native/detail/epoll/epoll_tcp_socket.hppinclude/boost/corosio/native/detail/epoll/epoll_traits.hppinclude/boost/corosio/native/detail/epoll/epoll_types.hppinclude/boost/corosio/native/detail/epoll/epoll_udp_service.hppinclude/boost/corosio/native/detail/epoll/epoll_udp_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_datagram_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_datagram_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_local_stream_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_op.hppinclude/boost/corosio/native/detail/kqueue/kqueue_scheduler.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_tcp_socket.hppinclude/boost/corosio/native/detail/kqueue/kqueue_traits.hppinclude/boost/corosio/native/detail/kqueue/kqueue_types.hppinclude/boost/corosio/native/detail/kqueue/kqueue_udp_service.hppinclude/boost/corosio/native/detail/kqueue/kqueue_udp_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_acceptor.hppinclude/boost/corosio/native/detail/reactor/reactor_acceptor_service.hppinclude/boost/corosio/native/detail/reactor/reactor_backend.hppinclude/boost/corosio/native/detail/reactor/reactor_basic_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_datagram_ops.hppinclude/boost/corosio/native/detail/reactor/reactor_datagram_socket.hppinclude/boost/corosio/native/detail/reactor/reactor_op_complete.hppinclude/boost/corosio/native/detail/reactor/reactor_service_finals.hppinclude/boost/corosio/native/detail/reactor/reactor_socket_finals.hppinclude/boost/corosio/native/detail/reactor/reactor_socket_service.hppinclude/boost/corosio/native/detail/reactor/reactor_stream_ops.hppinclude/boost/corosio/native/detail/reactor/reactor_stream_socket.hppinclude/boost/corosio/native/detail/select/select_local_datagram_service.hppinclude/boost/corosio/native/detail/select/select_local_datagram_socket.hppinclude/boost/corosio/native/detail/select/select_local_stream_acceptor.hppinclude/boost/corosio/native/detail/select/select_local_stream_acceptor_service.hppinclude/boost/corosio/native/detail/select/select_local_stream_service.hppinclude/boost/corosio/native/detail/select/select_local_stream_socket.hppinclude/boost/corosio/native/detail/select/select_op.hppinclude/boost/corosio/native/detail/select/select_scheduler.hppinclude/boost/corosio/native/detail/select/select_tcp_acceptor.hppinclude/boost/corosio/native/detail/select/select_tcp_acceptor_service.hppinclude/boost/corosio/native/detail/select/select_tcp_service.hppinclude/boost/corosio/native/detail/select/select_tcp_socket.hppinclude/boost/corosio/native/detail/select/select_traits.hppinclude/boost/corosio/native/detail/select/select_types.hppinclude/boost/corosio/native/detail/select/select_udp_service.hppinclude/boost/corosio/native/detail/select/select_udp_socket.hppinclude/boost/corosio/native/native_tcp_acceptor.hppinclude/boost/corosio/native/native_tcp_socket.hppinclude/boost/corosio/native/native_udp_socket.hppsrc/corosio/src/io_context.cpp
💤 Files with no reviewable changes (39)
- include/boost/corosio/native/detail/kqueue/kqueue_local_datagram_socket.hpp
- include/boost/corosio/native/detail/select/select_local_stream_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_socket.hpp
- include/boost/corosio/native/detail/select/select_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/select/select_local_datagram_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor.hpp
- include/boost/corosio/native/detail/select/select_op.hpp
- include/boost/corosio/native/detail/select/select_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_acceptor.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_datagram_service.hpp
- include/boost/corosio/native/detail/select/select_tcp_acceptor.hpp
- include/boost/corosio/native/detail/select/select_local_stream_service.hpp
- include/boost/corosio/native/detail/select/select_local_datagram_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_datagram_service.hpp
- include/boost/corosio/native/detail/select/select_tcp_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_udp_service.hpp
- include/boost/corosio/native/detail/select/select_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_tcp_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_udp_service.hpp
- include/boost/corosio/native/detail/select/select_tcp_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_op.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_op.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_udp_socket.hpp
- include/boost/corosio/native/detail/select/select_udp_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_udp_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_socket.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_local_stream_acceptor_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_datagram_socket.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_local_stream_service.hpp
- include/boost/corosio/native/detail/select/select_udp_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_tcp_acceptor_service.hpp
✅ Files skipped from review due to trivial changes (6)
- include/boost/corosio/native/detail/kqueue/kqueue_scheduler.hpp
- include/boost/corosio/native/detail/select/select_scheduler.hpp
- include/boost/corosio/native/native_tcp_socket.hpp
- include/boost/corosio/native/native_tcp_acceptor.hpp
- src/corosio/src/io_context.cpp
- include/boost/corosio/native/detail/reactor/reactor_socket_service.hpp
🚧 Files skipped from review as they are similar to previous changes (8)
- include/boost/corosio/native/detail/reactor/reactor_acceptor_service.hpp
- include/boost/corosio/native/detail/epoll/epoll_scheduler.hpp
- include/boost/corosio/native/native_udp_socket.hpp
- include/boost/corosio/native/detail/reactor/reactor_basic_socket.hpp
- include/boost/corosio/native/detail/reactor/reactor_acceptor.hpp
- include/boost/corosio/native/detail/reactor/reactor_op_complete.hpp
- include/boost/corosio/native/detail/epoll/epoll_traits.hpp
- include/boost/corosio/native/detail/kqueue/kqueue_types.hpp
The epoll, kqueue, and select backends each had 13 nearly-identical
per-protocol files for the socket, service, and acceptor types of
TCP, UDP, and Unix stream/datagram. Replace those 39 files with
shared parameterized templates plus per-backend traits and types
files, dropping each backend from 14 files (the 13 plus the
scheduler) to 3 (scheduler, traits, types).
New per-backend files:
- {backend}_traits.hpp: platform-specific behavior (socket creation
flags, write and accept policies, fd configuration, stream socket
hook for SO_LINGER, descriptor_state)
- {backend}_types.hpp: named final classes that instantiate the
shared templates
New shared templates in detail/reactor/:
- reactor_stream_ops.hpp and reactor_datagram_ops.hpp: parameterized
op types
- reactor_socket_finals.hpp: reactor_stream_socket_impl,
reactor_dgram_socket_impl, reactor_acceptor_impl
- reactor_service_finals.hpp: per-protocol service implementations
plus do_open_socket, do_assign_fd, and do_open_acceptor helpers
- reactor_backend.hpp: the accept() method body, which needs all
the named types to be complete
The existing reactor base templates (reactor_basic_socket,
reactor_stream_socket, reactor_datagram_socket, reactor_acceptor)
gain ImplBase and Endpoint template parameters so a single
template works for both IP and Unix domain sockets.
io_context.cpp and the native/ tcp_socket, tcp_acceptor, and
udp_socket headers switch to including {backend}_types.hpp in
place of the individual service headers.
mvandeberg
force-pushed
the
pr/reactor-consolidation
branch
from
893a5dc to
4a81ba6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmarks
accept_churn
burst/10burst/100burst_lockless/10burst_lockless/100concurrent/1concurrent/4concurrent/16sequentialsequential_locklessfan_out
concurrent_parents/1concurrent_parents/4concurrent_parents/16concurrent_parents_lockless/1concurrent_parents_lockless/4concurrent_parents_lockless/16fork_join/1fork_join/4fork_join/16fork_join/64fork_join_lockless/1fork_join_lockless/4fork_join_lockless/16fork_join_lockless/64nested/4nested/16nested_lockless/4nested_lockless/16http_server
concurrent/1concurrent/4concurrent/16concurrent/32multithread/1multithread/2multithread/4multithread/8multithread/16single_connsingle_conn_locklesslocal_socket_latency
concurrent/1concurrent/4concurrent/16concurrent_lockless/1concurrent_lockless/4concurrent_lockless/16pingpong/1pingpong/64pingpong/1024pingpong_lockless/1pingpong_lockless/64pingpong_lockless/1024local_socket_throughput
bidirectional/1024bidirectional/4096bidirectional/16384bidirectional/65536bidirectional/262144bidirectional/1048576bidirectional_lockless/1024bidirectional_lockless/4096bidirectional_lockless/16384bidirectional_lockless/65536bidirectional_lockless/262144bidirectional_lockless/1048576unidirectional/1024unidirectional/4096unidirectional/16384unidirectional/65536unidirectional/262144unidirectional/1048576unidirectional_lockless/1024unidirectional_lockless/4096unidirectional_lockless/16384unidirectional_lockless/65536unidirectional_lockless/262144unidirectional_lockless/1048576socket_latency
concurrent/1concurrent/4concurrent/16concurrent_lockless/1concurrent_lockless/4concurrent_lockless/16pingpong/1pingpong/64pingpong/1024pingpong_lockless/1pingpong_lockless/64pingpong_lockless/1024socket_throughput
bidirectional/1024bidirectional/4096bidirectional/16384bidirectional/65536bidirectional/262144bidirectional/1048576bidirectional_lockless/1024bidirectional_lockless/4096bidirectional_lockless/16384bidirectional_lockless/65536bidirectional_lockless/262144bidirectional_lockless/1048576multithread/2multithread/4multithread/8unidirectional/1024unidirectional/4096unidirectional/16384unidirectional/65536unidirectional/262144unidirectional/1048576unidirectional_lockless/1024unidirectional_lockless/4096unidirectional_lockless/16384unidirectional_lockless/65536unidirectional_lockless/262144unidirectional_lockless/1048576timer
concurrent/10concurrent/100concurrent/1000fire_ratefire_rate_locklessschedule_cancelschedule_cancel_locklessSummary by CodeRabbit