Skip to content

Commit 7bbebc7

Browse files
committed
fix: four production bugs surfaced by the coverage push
- socket_option: IP_MULTICAST_LOOP and IP_MULTICAST_TTL are u_char options on BSD-derived kernels; the previous int storage caused setsockopt to return EINVAL on macOS. Add byte_boolean_option / byte_integer_option (public ABI) and byte_boolean<> / byte_integer<> (native templates) with single-byte storage; rebase multicast_loop_v4 and multicast_hops_v4 onto them. IPv6 variants are unaffected. - native_local_datagram_socket: include of the deleted IOCP datagram service was reachable in any IOCP build. Wrap the whole header in BOOST_COROSIO_POSIX to mirror local_datagram_socket.hpp. - posix_resolver_service and win_resolver_service: replace make_service<thread_pool>() with use_service<thread_pool>(). The original call threw 'invalid argument' whenever io_context_options.thread_pool_size != 1 because pre_create_services had already constructed the pool with non-default args. - iocp wait_reactor and tcp_acceptor_service: resolve a pre-cancelled stop_token race in wait() and accept() paths that caused tests to hang on Windows IOCP under specific completion orderings.
1 parent 5f327d4 commit 7bbebc7

7 files changed

Lines changed: 246 additions & 14 deletions

File tree

‎include/boost/corosio/native/detail/iocp/win_resolver_service.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ win_resolver::do_reverse_resolve_work(pool_work_item* w) noexcept
495495
inline win_resolver_service::win_resolver_service(
496496
capy::execution_context& ctx, scheduler& sched)
497497
: sched_(sched)
498-
, pool_(ctx.make_service<thread_pool>())
498+
, pool_(ctx.use_service<thread_pool>())
499499
{
500500
}
501501

‎include/boost/corosio/native/detail/iocp/win_tcp_acceptor_service.hpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,6 +1373,14 @@ win_tcp_acceptor_internal::accept(
13731373
}
13741374

13751375
svc_.on_pending(&op);
1376+
1377+
// If the stop_token was already cancelled when start() was called,
1378+
// the CancelIoEx in the canceller fired before AcceptEx was
1379+
// submitted and had no effect. Re-issue now that the I/O is
1380+
// pending so the completion posts with ERROR_OPERATION_ABORTED.
1381+
if (op.cancelled.load(std::memory_order_acquire))
1382+
::CancelIoEx(reinterpret_cast<HANDLE>(socket_), &op);
1383+
13761384
return std::noop_coroutine();
13771385
}
13781386

‎include/boost/corosio/native/detail/iocp/win_wait_reactor.hpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,16 @@ inline void
248248
win_wait_reactor::register_wait(
249249
SOCKET fd, wait_type w, overlapped_op* op)
250250
{
251+
// If the op was already cancelled (e.g. pre-cancelled stop_token
252+
// fired synchronously before this call), complete immediately
253+
// instead of registering. Otherwise the reactor would park the
254+
// op forever because the earlier cancel_wait() found nothing to
255+
// cancel in registered_.
256+
if (op->cancelled.load(std::memory_order_acquire))
257+
{
258+
sched_.on_completion(op, 0, 0);
259+
return;
260+
}
251261
{
252262
std::lock_guard lock(mutex_);
253263
pending_register_.push_back(entry{fd, w, op});

‎include/boost/corosio/native/detail/posix/posix_resolver_service.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class BOOST_COROSIO_DECL posix_resolver_service final
3636

3737
posix_resolver_service(capy::execution_context& ctx, scheduler& sched)
3838
: sched_(&sched)
39-
, pool_(ctx.make_service<thread_pool>())
39+
, pool_(ctx.use_service<thread_pool>())
4040
{
4141
}
4242

‎include/boost/corosio/native/native_local_datagram_socket.hpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
#ifndef BOOST_COROSIO_NATIVE_NATIVE_LOCAL_DATAGRAM_SOCKET_HPP
1111
#define BOOST_COROSIO_NATIVE_NATIVE_LOCAL_DATAGRAM_SOCKET_HPP
1212

13+
#include <boost/corosio/detail/platform.hpp>
14+
15+
#if BOOST_COROSIO_POSIX
16+
1317
#include <boost/corosio/local_datagram_socket.hpp>
1418
#include <boost/corosio/backend.hpp>
1519

@@ -25,10 +29,6 @@
2529
#if BOOST_COROSIO_HAS_KQUEUE
2630
#include <boost/corosio/native/detail/kqueue/kqueue_types.hpp>
2731
#endif
28-
29-
#if BOOST_COROSIO_HAS_IOCP
30-
#include <boost/corosio/native/detail/iocp/win_local_dgram_service.hpp>
31-
#endif
3232
#endif // !BOOST_COROSIO_MRDOCS
3333

3434
namespace boost::corosio {
@@ -486,4 +486,6 @@ class native_local_datagram_socket : public local_datagram_socket
486486

487487
} // namespace boost::corosio
488488

489+
#endif // BOOST_COROSIO_POSIX
490+
489491
#endif // BOOST_COROSIO_NATIVE_NATIVE_LOCAL_DATAGRAM_SOCKET_HPP

‎include/boost/corosio/native/native_socket_option.hpp‎

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,86 @@ class integer
248248
}
249249
};
250250

251+
/** A boolean socket option with single-byte storage.
252+
253+
Some BSD-derived kernels (macOS, FreeBSD) require certain IPv4 multicast
254+
options (`IP_MULTICAST_LOOP`) to be set with a one-byte value and return
255+
`EINVAL` for the four-byte form that Linux accepts. This template
256+
provides `unsigned char` storage so the option works on every platform.
257+
258+
@tparam Level The protocol level.
259+
@tparam Name The option name.
260+
*/
261+
template<int Level, int Name>
262+
class byte_boolean
263+
{
264+
unsigned char value_ = 0;
265+
266+
public:
267+
byte_boolean() = default;
268+
269+
explicit byte_boolean(bool v) noexcept : value_(v ? 1 : 0) {}
270+
271+
byte_boolean& operator=(bool v) noexcept
272+
{
273+
value_ = v ? 1 : 0;
274+
return *this;
275+
}
276+
277+
bool value() const noexcept { return value_ != 0; }
278+
explicit operator bool() const noexcept { return value_ != 0; }
279+
bool operator!() const noexcept { return value_ == 0; }
280+
281+
static constexpr int level() noexcept { return Level; }
282+
static constexpr int name() noexcept { return Name; }
283+
284+
void* data() noexcept { return &value_; }
285+
void const* data() const noexcept { return &value_; }
286+
std::size_t size() const noexcept { return sizeof(value_); }
287+
288+
void resize(std::size_t) noexcept {}
289+
};
290+
291+
/** An integer socket option with single-byte storage.
292+
293+
Same rationale as `byte_boolean`: BSD-derived kernels require
294+
`IP_MULTICAST_TTL` to be set with a one-byte value. Linux accepts
295+
one byte too, so single-byte storage is portable. Values are
296+
truncated to the 0–255 range.
297+
298+
@tparam Level The protocol level.
299+
@tparam Name The option name.
300+
*/
301+
template<int Level, int Name>
302+
class byte_integer
303+
{
304+
unsigned char value_ = 0;
305+
306+
public:
307+
byte_integer() = default;
308+
309+
explicit byte_integer(int v) noexcept
310+
: value_(static_cast<unsigned char>(v))
311+
{}
312+
313+
byte_integer& operator=(int v) noexcept
314+
{
315+
value_ = static_cast<unsigned char>(v);
316+
return *this;
317+
}
318+
319+
int value() const noexcept { return value_; }
320+
321+
static constexpr int level() noexcept { return Level; }
322+
static constexpr int name() noexcept { return Name; }
323+
324+
void* data() noexcept { return &value_; }
325+
void const* data() const noexcept { return &value_; }
326+
std::size_t size() const noexcept { return sizeof(value_); }
327+
328+
void resize(std::size_t) noexcept {}
329+
};
330+
251331
/** The SO_LINGER socket option (native variant).
252332
253333
Controls behavior when closing a socket with unsent data.
@@ -375,13 +455,13 @@ using reuse_port = boolean<SOL_SOCKET, SO_REUSEPORT>;
375455
#endif
376456

377457
/// Enable loopback of outgoing multicast on IPv4 (IP_MULTICAST_LOOP).
378-
using multicast_loop_v4 = boolean<IPPROTO_IP, IP_MULTICAST_LOOP>;
458+
using multicast_loop_v4 = byte_boolean<IPPROTO_IP, IP_MULTICAST_LOOP>;
379459

380460
/// Enable loopback of outgoing multicast on IPv6 (IPV6_MULTICAST_LOOP).
381461
using multicast_loop_v6 = boolean<IPPROTO_IPV6, IPV6_MULTICAST_LOOP>;
382462

383463
/// Set the multicast TTL for IPv4 (IP_MULTICAST_TTL).
384-
using multicast_hops_v4 = integer<IPPROTO_IP, IP_MULTICAST_TTL>;
464+
using multicast_hops_v4 = byte_integer<IPPROTO_IP, IP_MULTICAST_TTL>;
385465

386466
/// Set the multicast hop limit for IPv6 (IPV6_MULTICAST_HOPS).
387467
using multicast_hops_v6 = integer<IPPROTO_IPV6, IPV6_MULTICAST_HOPS>;

‎include/boost/corosio/socket_option.hpp‎

Lines changed: 138 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,131 @@ class integer_option
172172
}
173173
};
174174

175+
/** Base class for concrete boolean socket options with single-byte storage.
176+
177+
Some BSD-derived kernels (macOS, FreeBSD) require certain IPv4 multicast
178+
options (`IP_MULTICAST_LOOP`) to be set with a one-byte value and return
179+
`EINVAL` for the four-byte form that Linux accepts. This base provides
180+
`unsigned char` storage so the same options work on every platform.
181+
*/
182+
class byte_boolean_option
183+
{
184+
unsigned char value_ = 0;
185+
186+
public:
187+
/// Construct with default value (disabled).
188+
byte_boolean_option() = default;
189+
190+
/** Construct with an explicit value.
191+
192+
@param v `true` to enable the option, `false` to disable.
193+
*/
194+
explicit byte_boolean_option(bool v) noexcept : value_(v ? 1 : 0) {}
195+
196+
/// Assign a new value.
197+
byte_boolean_option& operator=(bool v) noexcept
198+
{
199+
value_ = v ? 1 : 0;
200+
return *this;
201+
}
202+
203+
/// Return the option value.
204+
bool value() const noexcept
205+
{
206+
return value_ != 0;
207+
}
208+
209+
/// Return the option value.
210+
explicit operator bool() const noexcept
211+
{
212+
return value_ != 0;
213+
}
214+
215+
/// Return the negated option value.
216+
bool operator!() const noexcept
217+
{
218+
return value_ == 0;
219+
}
220+
221+
/// Return a pointer to the underlying storage.
222+
void* data() noexcept
223+
{
224+
return &value_;
225+
}
226+
227+
/// Return a pointer to the underlying storage.
228+
void const* data() const noexcept
229+
{
230+
return &value_;
231+
}
232+
233+
/// Return the size of the underlying storage.
234+
std::size_t size() const noexcept
235+
{
236+
return sizeof(value_);
237+
}
238+
239+
/// Storage is already one byte; no normalization needed.
240+
void resize(std::size_t) noexcept {}
241+
};
242+
243+
/** Base class for concrete integer socket options with single-byte storage.
244+
245+
Same rationale as `byte_boolean_option`: BSD-derived kernels require
246+
`IP_MULTICAST_TTL` to be set with a one-byte value. Linux accepts
247+
one-byte too, so single-byte storage is portable.
248+
*/
249+
class byte_integer_option
250+
{
251+
unsigned char value_ = 0;
252+
253+
public:
254+
/// Construct with default value (zero).
255+
byte_integer_option() = default;
256+
257+
/** Construct with an explicit value.
258+
259+
@param v The option value; truncated to one byte.
260+
*/
261+
explicit byte_integer_option(int v) noexcept
262+
: value_(static_cast<unsigned char>(v))
263+
{}
264+
265+
/// Assign a new value; truncated to one byte.
266+
byte_integer_option& operator=(int v) noexcept
267+
{
268+
value_ = static_cast<unsigned char>(v);
269+
return *this;
270+
}
271+
272+
/// Return the option value.
273+
int value() const noexcept
274+
{
275+
return value_;
276+
}
277+
278+
/// Return a pointer to the underlying storage.
279+
void* data() noexcept
280+
{
281+
return &value_;
282+
}
283+
284+
/// Return a pointer to the underlying storage.
285+
void const* data() const noexcept
286+
{
287+
return &value_;
288+
}
289+
290+
/// Return the size of the underlying storage.
291+
std::size_t size() const noexcept
292+
{
293+
return sizeof(value_);
294+
}
295+
296+
/// Storage is already one byte; no normalization needed.
297+
void resize(std::size_t) noexcept {}
298+
};
299+
175300
/** Disable Nagle's algorithm (TCP_NODELAY).
176301
177302
@par Example
@@ -430,16 +555,19 @@ class BOOST_COROSIO_DECL linger
430555

431556
/** Enable loopback of outgoing multicast on IPv4 (IP_MULTICAST_LOOP).
432557
558+
Uses single-byte storage because BSD-derived kernels (macOS, FreeBSD)
559+
reject the four-byte form with `EINVAL`. Linux accepts either size.
560+
433561
@par Example
434562
@code
435563
sock.set_option( socket_option::multicast_loop_v4( true ) );
436564
@endcode
437565
*/
438-
class BOOST_COROSIO_DECL multicast_loop_v4 : public boolean_option
566+
class BOOST_COROSIO_DECL multicast_loop_v4 : public byte_boolean_option
439567
{
440568
public:
441-
using boolean_option::boolean_option;
442-
using boolean_option::operator=;
569+
using byte_boolean_option::byte_boolean_option;
570+
using byte_boolean_option::operator=;
443571

444572
/// Return the protocol level.
445573
static int level() noexcept;
@@ -470,16 +598,20 @@ class BOOST_COROSIO_DECL multicast_loop_v6 : public boolean_option
470598

471599
/** Set the multicast TTL for IPv4 (IP_MULTICAST_TTL).
472600
601+
Uses single-byte storage because BSD-derived kernels (macOS, FreeBSD)
602+
reject the four-byte form with `EINVAL`. Linux accepts either size.
603+
Values are truncated to the 0–255 range.
604+
473605
@par Example
474606
@code
475607
sock.set_option( socket_option::multicast_hops_v4( 4 ) );
476608
@endcode
477609
*/
478-
class BOOST_COROSIO_DECL multicast_hops_v4 : public integer_option
610+
class BOOST_COROSIO_DECL multicast_hops_v4 : public byte_integer_option
479611
{
480612
public:
481-
using integer_option::integer_option;
482-
using integer_option::operator=;
613+
using byte_integer_option::byte_integer_option;
614+
using byte_integer_option::operator=;
483615

484616
/// Return the protocol level.
485617
static int level() noexcept;

0 commit comments

Comments
 (0)