Implement socket options API with named methods #79
Conversation
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
📝 WalkthroughWalkthroughAdds a cross-platform socket options API (set/get for TCP_NODELAY, SO_KEEPALIVE, SO_RCVBUF, SO_SNDBUF, SO_LINGER), introduces per-operation cancellation paths in epoll, and replaces assert() checks with runtime guards or logic_error throws in socket/acceptor code. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Socket as boost::corosio::socket
participant Impl as socket_impl (epoll/iocp)
participant OS as OS Socket
User->>Socket: set_no_delay(true)
Socket->>Socket: validate impl_ (throw if null)
Socket->>Impl: set_no_delay(true)
Impl->>OS: setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,...)
OS-->>Impl: result / errno
Impl-->>Socket: system::error_code
Socket-->>User: throw on error / return
User->>Socket: cancel()
Socket->>Socket: validate impl_ (return if null)
Socket->>Impl: cancel()
Impl->>Impl: cancel_single_op(rd/wr/conn) / unregister / post completions
Impl-->>Socket: cancellations posted
Socket-->>User: cancel complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 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://79.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-26 14:53:15 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 (2)
include/boost/corosio/socket.hpp (1)
89-123: Documentlinger_optionsmembers explicitly.
This public type is part of the API surface, but the doc doesn’t describe the member semantics or whentimeoutis used. Add brief member descriptions (units and when ignored) to make the contract clear.As per coding guidelines.✏️ Suggested doc update
- /** Options for SO_LINGER socket option. */ + /** Options for SO_LINGER socket option. + `@par` Members + - enabled: Whether SO_LINGER is enabled. + - timeout: Linger timeout in seconds (ignored when enabled is false). + */include/boost/corosio/acceptor.hpp (1)
220-256: Documentaccept()throwing when not listening.
The implementation now throwsstd::logic_errorifimpl_is null; please surface that in the API docs.As per coding guidelines.✏️ Suggested doc update
@@ `@par` Preconditions The acceptor must be listening (`is_open() == true`). The peer socket must be associated with the same execution context. + `@throws` std::logic_error if the acceptor is not listening.
🤖 Fix all issues with AI agents
In `@include/boost/corosio/socket.hpp`:
- Around line 300-304: Update the documentation for the member function
connect(endpoint ep) to explicitly state it can throw std::logic_error when the
socket is not open (i.e., when impl_ is null); reference the connect(endpoint)
function and its early check of impl_ which calls detail::throw_logic_error, and
keep the existing async error notes for connect_awaitable intact.
- Around line 374-467: The documented socket option accessors (set_no_delay,
no_delay, set_keep_alive, keep_alive, set_receive_buffer_size,
receive_buffer_size, set_send_buffer_size, send_buffer_size, set_linger, linger)
currently only mention std::system_error; update their doc comments to include a
precondition that the socket must be open and a throws note that
std::logic_error is thrown when the socket is closed (either add the brief
"Precondition: socket must be open" and "Throws: std::logic_error if the socket
is closed" to each method's comment, or add a single section-wide note at the
top of this options block stating the same so individual methods don’t need
repetition). Ensure the wording matches existing style and appears alongside the
existing `@throws` std::system_error entries.
sgerbino
force-pushed
the
feature/socket-options
branch
from
db9345c to
70717b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/corosio/src/detail/epoll/sockets.cpp (1)
10-26: Add the required implementation overview block after includes.This file is non-trivial and should have a
/* */overview right after includes.
As per coding guidelines.📝 Suggested overview block
`#include` <sys/socket.h> `#include` <unistd.h> +/* + epoll Socket Options Implementation + ---------------------------------- + Provides setsockopt/getsockopt wrappers for common options and integrates + with epoll_socket_impl cancellation semantics. +*/
♻️ Duplicate comments (2)
include/boost/corosio/socket.hpp (2)
262-295: Documentconnect()throwing when the socket is closed.The runtime check now throws
std::logic_errorwhen the socket isn’t open, but the doc only lists async error codes.
As per coding guidelines.
366-459: Add open-socket precondition/std::logic_errorto option docs.All option accessors throw
std::logic_erroron a closed socket, but docs only mentionstd::system_error. Add a section-wide precondition/throws note or per-method notes.
As per coding guidelines.
Add a cross-platform socket options API using simple named getter/setter methods. This approach was chosen over the Asio-style template-based API for better discoverability and simplicity. New socket methods: - set_no_delay(bool) / no_delay() - TCP_NODELAY (disable Nagle's algorithm) - set_keep_alive(bool) / keep_alive() - SO_KEEPALIVE - set_receive_buffer_size(int) / receive_buffer_size() - SO_RCVBUF - set_send_buffer_size(int) / send_buffer_size() - SO_SNDBUF - set_linger(bool, int) / linger() - SO_LINGER Changes: - include/boost/corosio/socket.hpp: Add linger_options struct, virtual methods to socket_impl, and public methods to socket class - src/corosio/src/socket.cpp: Implement socket option methods with error handling - src/corosio/src/detail/epoll/sockets.hpp: Declare option methods for epoll_socket_impl - src/corosio/src/detail/epoll/sockets.cpp: Implement POSIX setsockopt/ getsockopt calls, add netinet/tcp.h include - src/corosio/src/detail/iocp/sockets.hpp: Implement Windows setsockopt/ getsockopt calls inline in win_socket_impl - bench/corosio/socket_latency_bench.cpp: Replace manual setsockopt workaround with new set_no_delay() API - test/unit/socket.cpp: Add unit tests for all socket options
sgerbino
force-pushed
the
feature/socket-options
branch
from
70717b5 to
fbe79e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@include/boost/corosio/socket.hpp`:
- Around line 85-90: Document the linger_options struct and its members by
adding doc comments for the type and each field: describe what linger_options
configures (SO_LINGER behavior), document enabled as a boolean that turns linger
on/off, and document timeout as the linger interval in seconds (units: seconds),
acceptable range/constraints (non-negative integer, typically >= 0), and default
values (enabled=false, timeout=0); update the public header comments adjacent to
linger_options, enabled, and timeout so the API docs clearly state semantics and
constraints.
- Around line 451-462: The docstring for set_linger currently omits the valid
range for the timeout parameter; update the comment for set_linger(bool enabled,
int timeout) to state that timeout is specified in seconds and must be
non‑negative (timeout >= 0), since negative values are rejected (EINVAL) by the
implementation; also mention that this controls the SO_LINGER option behavior
when enabled and that callers must pass a non‑negative timeout to avoid errors.
In `@src/corosio/src/detail/epoll/sockets.cpp`:
- Around line 20-23: Add a /* ... */ implementation-overview comment block
immediately after the includes in src/corosio/src/detail/epoll/sockets.cpp that
summarizes the file’s main flow and tricky areas: describe how the epoll loop is
structured (use of epoll_ctl/epoll_wait), how socket lifecycle and ownership are
managed, cancellation/shutdown semantics, and relevant socket option handling
(e.g., TCP_NODELAY, SO_REUSEADDR) so reviewers can quickly understand functions
in this file (the epoll handling code and any socket helper routines).
Comment on lines
+85
to
+90
| /** Options for SO_LINGER socket option. */ | ||
| struct linger_options | ||
| { | ||
| bool enabled = false; | ||
| int timeout = 0; // seconds | ||
| }; |
There was a problem hiding this comment.
Document linger_options members in the public header.
As a public API type, the fields should be documented (meaning, units, constraints) to meet the docstring requirements. As per coding guidelines.
📝 Suggested doc update
- /** Options for SO_LINGER socket option. */
+ /** Options for SO_LINGER socket option.
+
+ `@par` Members
+ - `enabled`: Whether SO_LINGER is enabled.
+ - `timeout`: Linger timeout in seconds (non-negative).
+ */
struct linger_options
{
bool enabled = false;
int timeout = 0; // seconds
};📝 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.
Suggested change
| /** Options for SO_LINGER socket option. */ | |
| struct linger_options | |
| { | |
| bool enabled = false; | |
| int timeout = 0; // seconds | |
| }; | |
| /** Options for SO_LINGER socket option. | |
| `@par` Members | |
| - `enabled`: Whether SO_LINGER is enabled. | |
| - `timeout`: Linger timeout in seconds (non-negative). | |
| */ | |
| struct linger_options | |
| { | |
| bool enabled = false; | |
| int timeout = 0; // seconds | |
| }; |
🤖 Prompt for AI Agents
In `@include/boost/corosio/socket.hpp` around lines 85 - 90, Document the
linger_options struct and its members by adding doc comments for the type and
each field: describe what linger_options configures (SO_LINGER behavior),
document enabled as a boolean that turns linger on/off, and document timeout as
the linger interval in seconds (units: seconds), acceptable range/constraints
(non-negative integer, typically >= 0), and default values (enabled=false,
timeout=0); update the public header comments adjacent to linger_options,
enabled, and timeout so the API docs clearly state semantics and constraints.
Comment on lines
+451
to
+462
| /** Set the SO_LINGER option. | ||
| Controls behavior when closing a socket with unsent data. | ||
| @param enabled If `true`, close() will block until data is sent | ||
| or the timeout expires. If `false`, close() returns immediately. | ||
| @param timeout The linger timeout in seconds (only used if enabled). | ||
| @throws std::logic_error if the socket is not open. | ||
| @throws std::system_error on failure. | ||
| */ | ||
| void set_linger(bool enabled, int timeout); |
There was a problem hiding this comment.
Clarify the valid range for timeout in set_linger().
The implementation rejects negative values (EINVAL). Document the precondition to avoid surprises for callers. As per coding guidelines.
📝 Suggested doc tweak
- `@param` timeout The linger timeout in seconds (only used if enabled).
+ `@param` timeout The linger timeout in seconds (only used if enabled).
+ Must be non-negative.📝 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.
Suggested change
| /** Set the SO_LINGER option. | |
| Controls behavior when closing a socket with unsent data. | |
| @param enabled If `true`, close() will block until data is sent | |
| or the timeout expires. If `false`, close() returns immediately. | |
| @param timeout The linger timeout in seconds (only used if enabled). | |
| @throws std::logic_error if the socket is not open. | |
| @throws std::system_error on failure. | |
| */ | |
| void set_linger(bool enabled, int timeout); | |
| /** Set the SO_LINGER option. | |
| Controls behavior when closing a socket with unsent data. | |
| `@param` enabled If `true`, close() will block until data is sent | |
| or the timeout expires. If `false`, close() returns immediately. | |
| `@param` timeout The linger timeout in seconds (only used if enabled). | |
| Must be non-negative. | |
| `@throws` std::logic_error if the socket is not open. | |
| `@throws` std::system_error on failure. | |
| */ | |
| void set_linger(bool enabled, int timeout); |
🤖 Prompt for AI Agents
In `@include/boost/corosio/socket.hpp` around lines 451 - 462, The docstring for
set_linger currently omits the valid range for the timeout parameter; update the
comment for set_linger(bool enabled, int timeout) to state that timeout is
specified in seconds and must be non‑negative (timeout >= 0), since negative
values are rejected (EINVAL) by the implementation; also mention that this
controls the SO_LINGER option behavior when enabled and that callers must pass a
non‑negative timeout to avoid errors.
Comment on lines
20
to
23
| #include <errno.h> | ||
| #include <netinet/in.h> | ||
| #include <netinet/tcp.h> | ||
| #include <sys/epoll.h> |
There was a problem hiding this comment.
Add the required implementation overview block after includes.
This file contains non-trivial logic; please add a /* */ overview comment after the includes to summarize the main flow and tricky parts (cancellation, lifetime, socket options). As per coding guidelines.
📝 Suggested placement
`#include` <sys/socket.h>
`#include` <unistd.h>
+/*
+ epoll socket implementation overview
+ - Non-blocking syscalls with one-shot epoll registration.
+ - Cancellation unregisters and posts completion while preserving impl lifetime.
+ - Socket option helpers wrap getsockopt/setsockopt with error_code.
+*/
+
namespace boost {
namespace corosio {
namespace detail {🤖 Prompt for AI Agents
In `@src/corosio/src/detail/epoll/sockets.cpp` around lines 20 - 23, Add a /* ...
*/ implementation-overview comment block immediately after the includes in
src/corosio/src/detail/epoll/sockets.cpp that summarizes the file’s main flow
and tricky areas: describe how the epoll loop is structured (use of
epoll_ctl/epoll_wait), how socket lifecycle and ownership are managed,
cancellation/shutdown semantics, and relevant socket option handling (e.g.,
TCP_NODELAY, SO_REUSEADDR) so reviewers can quickly understand functions in this
file (the epoll handling code and any socket helper routines).
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.
Add a cross-platform socket options API using simple named getter/setter methods. This approach was chosen over the Asio-style template-based API for better discoverability and simplicity.
New socket methods:
Resolves #73.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.