From 039634700f6e13118ea3ca2f40e77ca52859a485 Mon Sep 17 00:00:00 2001 From: Michael Vandeberg Date: Wed, 17 Jun 2026 09:43:32 -0600 Subject: [PATCH] fix(timer): resolve service safely and add executor constructors Fix UB in the timer constructor (cppalliance/corosio#213). The old timer_service_direct did an unchecked static_cast on a capy::execution_context and dereferenced io_context::timer_svc_, so constructing a timer from any non-io_context (a plain execution_context, a thread_pool, or a user-derived context) read memory at a bogus offset and crashed. Resolve the timer service through the shared io_object::create_handle() path that every other I/O object uses; it throws std::logic_error when the service is absent. Delete the bespoke timer_service_direct helper along with the now-dead timer_service_access struct and io_context::timer_svc_ cache. Bring timer in line with the other public I/O objects (cppalliance/corosio#231): add executor constructors to both timer and native_timer (Ex const&; Ex const&, time_point; Ex const&, duration), mirroring the tcp_acceptor/signal_set shape and delegating to the executor's context. For the owning cancel_after/cancel_at path the timer is built inside a noexcept await_suspend from the awaiting coroutine's executor context. A non-io_context executor cannot supply a timer; rather than silently running the operation with no deadline, translate the failure into a clear precondition diagnostic (which aborts by design) and document the requirement on the owning overloads. Tighten timer_service.hpp to include detail/scheduler.hpp directly instead of pulling it transitively through io_context.hpp. Add tests for the throw path and for the new executor constructors. --- include/boost/corosio/cancel.hpp | 12 ++++ .../corosio/detail/cancel_at_awaitable.hpp | 25 ++++++- .../boost/corosio/detail/timer_service.hpp | 26 +------- include/boost/corosio/io_context.hpp | 4 -- .../boost/corosio/native/native_cancel.hpp | 12 ++++ include/boost/corosio/native/native_timer.hpp | 46 +++++++++++++ include/boost/corosio/timer.hpp | 66 ++++++++++++++++++- src/corosio/src/timer.cpp | 2 +- test/unit/native/native_timer.cpp | 13 ++++ test/unit/timer.cpp | 64 ++++++++++++++++++ 10 files changed, 237 insertions(+), 33 deletions(-) diff --git a/include/boost/corosio/cancel.hpp b/include/boost/corosio/cancel.hpp index 8e6901f15..77a939377 100644 --- a/include/boost/corosio/cancel.hpp +++ b/include/boost/corosio/cancel.hpp @@ -133,6 +133,12 @@ cancel_after(capy::IoAwaitable auto&& op, timer& t, timer::duration timeout) @note Creates a timer per call. Use the explicit-timer overload to amortize allocation across multiple timeouts. + @note The awaiting coroutine's executor must be backed by an + io_context (the deadline timer is built from it). Awaiting this + on a non-io_context executor is a precondition violation and + aborts; use the explicit-timer overload to construct the timer + yourself if you need a catchable error. + @par Example @code auto [ec, n] = co_await cancel_at( @@ -175,6 +181,12 @@ cancel_at(capy::IoAwaitable auto&& op, timer::time_point deadline) @note Creates a timer per call. Use the explicit-timer overload to amortize allocation across multiple timeouts. + @note The awaiting coroutine's executor must be backed by an + io_context (the deadline timer is built from it). Awaiting this + on a non-io_context executor is a precondition violation and + aborts; use the explicit-timer overload to construct the timer + yourself if you need a catchable error. + @par Example @code auto [ec, n] = co_await cancel_after( diff --git a/include/boost/corosio/detail/cancel_at_awaitable.hpp b/include/boost/corosio/detail/cancel_at_awaitable.hpp index b7cf559bb..a757a077a 100644 --- a/include/boost/corosio/detail/cancel_at_awaitable.hpp +++ b/include/boost/corosio/detail/cancel_at_awaitable.hpp @@ -11,12 +11,14 @@ #define BOOST_COROSIO_DETAIL_CANCEL_AT_AWAITABLE_HPP #include +#include #include #include #include #include #include +#include #include #include #include @@ -126,7 +128,28 @@ struct cancel_at_awaitable auto await_suspend(std::coroutine_handle<> h, capy::io_env const* env) { if constexpr (Owning) - timer_.emplace(env->executor.context()); + { + // The deadline timer is built here from the awaiting + // coroutine's executor context, the first point at which it + // is known. await_suspend is driven through a noexcept + // wrapper, so a failure cannot be surfaced as a catchable + // exception. An executor whose context is not an io_context + // cannot supply a timer service; silently running the + // operation with no deadline would be a worse failure than + // aborting, so translate the service-lookup error into a + // clear precondition diagnostic. This terminates by design + // (a usage error) rather than dropping the requested timeout. + try + { + timer_.emplace(env->executor.context()); + } + catch (std::logic_error const&) + { + throw_logic_error( + "cancel_after/cancel_at requires an " + "io_context-backed executor"); + } + } timer_->expires_at(deadline_); diff --git a/include/boost/corosio/detail/timer_service.hpp b/include/boost/corosio/detail/timer_service.hpp index 45aeb9690..eb192a59b 100644 --- a/include/boost/corosio/detail/timer_service.hpp +++ b/include/boost/corosio/detail/timer_service.hpp @@ -12,7 +12,7 @@ #define BOOST_COROSIO_DETAIL_TIMER_SERVICE_HPP #include -#include +#include #include #include #include @@ -891,26 +891,6 @@ timer_service::implementation::wait( // Free functions -struct timer_service_access -{ - static timer_service& get_timer(io_context& ctx) noexcept - { - return *ctx.timer_svc_; - } - - static void set_timer(io_context& ctx, timer_service& svc) noexcept - { - ctx.timer_svc_ = &svc; - } -}; - -// Bypass find_service() mutex by reading io_context's cached pointer -inline io_object::io_service& -timer_service_direct(capy::execution_context& ctx) noexcept -{ - return timer_service_access::get_timer(static_cast(ctx)); -} - inline std::size_t timer_service_update_expiry(timer::implementation& base) { @@ -935,9 +915,7 @@ timer_service_cancel_one(timer::implementation& base) noexcept inline timer_service& get_timer_service(capy::execution_context& ctx, scheduler& sched) { - auto& svc = ctx.make_service(sched); - timer_service_access::set_timer(static_cast(ctx), svc); - return svc; + return ctx.make_service(sched); } } // namespace boost::corosio::detail diff --git a/include/boost/corosio/io_context.hpp b/include/boost/corosio/io_context.hpp index e5a018d78..ec40b344c 100644 --- a/include/boost/corosio/io_context.hpp +++ b/include/boost/corosio/io_context.hpp @@ -167,7 +167,6 @@ struct io_context_options namespace detail { class timer_service; -struct timer_service_access; } // namespace detail /** An I/O context for running asynchronous operations. @@ -201,8 +200,6 @@ struct timer_service_access; */ class BOOST_COROSIO_DECL io_context : public capy::execution_context { - friend struct detail::timer_service_access; - /// Pre-create services that depend on options (before construct). void apply_options_pre_(io_context_options const& opts); @@ -215,7 +212,6 @@ class BOOST_COROSIO_DECL io_context : public capy::execution_context void configure_single_threaded_(); protected: - detail::timer_service* timer_svc_ = nullptr; detail::scheduler* sched_; public: diff --git a/include/boost/corosio/native/native_cancel.hpp b/include/boost/corosio/native/native_cancel.hpp index a652065cf..b6e61bc12 100644 --- a/include/boost/corosio/native/native_cancel.hpp +++ b/include/boost/corosio/native/native_cancel.hpp @@ -149,6 +149,12 @@ cancel_after( @note Creates a timer per call. Use the explicit-timer overload to amortize allocation across multiple timeouts. + @note The awaiting coroutine's executor must be backed by an + io_context (the deadline timer is built from it). Awaiting this + on a non-io_context executor is a precondition violation and + aborts; use the explicit-timer overload to construct the timer + yourself if you need a catchable error. + @par Example @code auto [ec, n] = co_await cancel_at( @@ -195,6 +201,12 @@ cancel_at(capy::IoAwaitable auto&& op, timer::time_point deadline) @note Creates a timer per call. Use the explicit-timer overload to amortize allocation across multiple timeouts. + @note The awaiting coroutine's executor must be backed by an + io_context (the deadline timer is built from it). Awaiting this + on a non-io_context executor is a precondition violation and + aborts; use the explicit-timer overload to construct the timer + yourself if you need a catchable error. + @par Example @code auto [ec, n] = co_await cancel_after( diff --git a/include/boost/corosio/native/native_timer.hpp b/include/boost/corosio/native/native_timer.hpp index ed1730cf8..b0716e9a3 100644 --- a/include/boost/corosio/native/native_timer.hpp +++ b/include/boost/corosio/native/native_timer.hpp @@ -118,6 +118,52 @@ class native_timer : public timer { } + /** Construct a native timer from an executor. + + The timer is associated with the executor's context, which must + be a corosio io_context. + + @param ex The executor whose context will own this timer. + + @throws std::logic_error if the executor's context is not an + io_context. + */ + template + requires(!std::same_as, native_timer>) && + capy::Executor + explicit native_timer(Ex const& ex) : native_timer(ex.context()) + { + } + + /** Construct a native timer from an executor with an absolute expiry. + + @param ex The executor whose context will own this timer. + @param t The initial expiry time point. + + @throws std::logic_error if the executor's context is not an + io_context. + */ + template + requires capy::Executor + native_timer(Ex const& ex, time_point t) : native_timer(ex.context(), t) + { + } + + /** Construct a native timer from an executor with a relative expiry. + + @param ex The executor whose context will own this timer. + @param d The initial expiry duration relative to now. + + @throws std::logic_error if the executor's context is not an + io_context. + */ + template + requires capy::Executor + native_timer(Ex const& ex, std::chrono::duration d) + : native_timer(ex.context(), d) + { + } + /** Move construct. @param other The timer to move from. diff --git a/include/boost/corosio/timer.hpp b/include/boost/corosio/timer.hpp index 931aa3225..7f8ac6660 100644 --- a/include/boost/corosio/timer.hpp +++ b/include/boost/corosio/timer.hpp @@ -17,7 +17,9 @@ #include #include +#include #include +#include namespace boost::corosio { @@ -58,21 +60,33 @@ class BOOST_COROSIO_DECL timer : public io_timer /** Construct a timer from an execution context. - @param ctx The execution context that will own this timer. + @param ctx The execution context that will own this timer. It + must be a corosio io_context; otherwise the constructor + throws (a timer service is required). + + @throws std::logic_error if @p ctx is not an io_context. */ explicit timer(capy::execution_context& ctx); /** Construct a timer with an initial absolute expiry time. - @param ctx The execution context that will own this timer. + @param ctx The execution context that will own this timer. It + must be a corosio io_context; otherwise the constructor + throws (a timer service is required). @param t The initial expiry time point. + + @throws std::logic_error if @p ctx is not an io_context. */ timer(capy::execution_context& ctx, time_point t); /** Construct a timer with an initial relative expiry time. - @param ctx The execution context that will own this timer. + @param ctx The execution context that will own this timer. It + must be a corosio io_context; otherwise the constructor + throws (a timer service is required). @param d The initial expiry duration relative to now. + + @throws std::logic_error if @p ctx is not an io_context. */ template timer(capy::execution_context& ctx, std::chrono::duration d) @@ -81,6 +95,52 @@ class BOOST_COROSIO_DECL timer : public io_timer expires_after(d); } + /** Construct a timer from an executor. + + The timer is associated with the executor's context, which must + be a corosio io_context. + + @param ex The executor whose context will own this timer. + + @throws std::logic_error if the executor's context is not an + io_context. + */ + template + requires(!std::same_as, timer>) && + capy::Executor + explicit timer(Ex const& ex) : timer(ex.context()) + { + } + + /** Construct a timer from an executor with an absolute expiry time. + + @param ex The executor whose context will own this timer. + @param t The initial expiry time point. + + @throws std::logic_error if the executor's context is not an + io_context. + */ + template + requires capy::Executor + timer(Ex const& ex, time_point t) : timer(ex.context(), t) + { + } + + /** Construct a timer from an executor with a relative expiry time. + + @param ex The executor whose context will own this timer. + @param d The initial expiry duration relative to now. + + @throws std::logic_error if the executor's context is not an + io_context. + */ + template + requires capy::Executor + timer(Ex const& ex, std::chrono::duration d) + : timer(ex.context(), d) + { + } + /** Move constructor. Transfers ownership of the timer resources. diff --git a/src/corosio/src/timer.cpp b/src/corosio/src/timer.cpp index e6ea9068a..b05b712af 100644 --- a/src/corosio/src/timer.cpp +++ b/src/corosio/src/timer.cpp @@ -16,7 +16,7 @@ namespace boost::corosio { timer::~timer() = default; timer::timer(capy::execution_context& ctx) - : io_timer(handle(ctx, detail::timer_service_direct(ctx))) + : io_timer(create_handle(ctx)) { } diff --git a/test/unit/native/native_timer.cpp b/test/unit/native/native_timer.cpp index 296f3eb40..64ba34b01 100644 --- a/test/unit/native/native_timer.cpp +++ b/test/unit/native/native_timer.cpp @@ -45,6 +45,18 @@ struct native_timer_test BOOST_TEST(t.expiry() > timer::time_point{}); } + // Issue #231: native_timer mirrors timer's executor constructors. + void testTimerConstructFromExecutor() + { + io_context ctx(Backend); + native_timer t(ctx.get_executor()); + BOOST_TEST_PASS(); + + native_timer t2( + ctx.get_executor(), std::chrono::milliseconds(100)); + BOOST_TEST(t2.expiry() > timer::time_point{}); + } + void testTimerWait() { io_context ctx(Backend); @@ -106,6 +118,7 @@ struct native_timer_test { testTimerConstruct(); testTimerConstructDuration(); + testTimerConstructFromExecutor(); testTimerWait(); testTimerWaitExpired(); testTimerPolymorphicSlice(); diff --git a/test/unit/timer.cpp b/test/unit/timer.cpp index c4731b880..98d5f9187 100644 --- a/test/unit/timer.cpp +++ b/test/unit/timer.cpp @@ -12,19 +12,29 @@ #include #include +#include #include +#include #include #include #include #include +#include #include +#include #include "context.hpp" #include "test_suite.hpp" namespace boost::corosio { +// Issue #213: a timer accepts any execution_context (so the common +// `timer(obj.context())` idiom works), but a non-io_context now throws +// rather than crashing via an unchecked downcast (see testNonIoContextThrows). +static_assert(std::is_constructible_v); +static_assert(std::is_constructible_v); + // Timer-specific tests // Focus: timer construction, expiry, wait, and cancellation // @@ -43,6 +53,19 @@ struct timer_test BOOST_TEST_PASS(); } + // Issue #213: constructing a timer from an execution_context that is + // not a corosio io_context (no timer service installed) must throw, + // not crash via an unchecked downcast. + void testNonIoContextThrows() + { + struct bare_context : capy::execution_context + { + }; + + bare_context ctx; + BOOST_TEST_THROWS(timer(ctx), std::logic_error); + } + void testConstructionWithTimePoint() { io_context ioc(Backend); @@ -52,6 +75,42 @@ struct timer_test BOOST_TEST(t.expiry() == tp); } + // Issue #231: bring timer in line with the other I/O objects, which + // all construct from an executor (delegating to the executor's context). + void testConstructFromExecutor() + { + io_context ioc(Backend); + timer t(ioc.get_executor()); + + BOOST_TEST_PASS(); + } + + void testConstructFromExecutorWithTimePoint() + { + io_context ioc(Backend); + auto tp = timer::clock_type::now() + std::chrono::seconds(10); + timer t(ioc.get_executor(), tp); + + BOOST_TEST(t.expiry() == tp); + } + + void testConstructFromExecutorWithDuration() + { + io_context ioc(Backend); + timer t(ioc.get_executor(), std::chrono::milliseconds(500)); + + BOOST_TEST(t.expiry() > timer::clock_type::now()); + } + + // The executor ctors delegate to the executor's context, so an executor + // whose context is not an io_context (e.g. a thread_pool) must throw, + // not crash — same guarantee as testNonIoContextThrows, new entry path. + void testExecutorNonIoContextThrows() + { + capy::thread_pool pool(1); + BOOST_TEST_THROWS(timer(pool.get_executor()), std::logic_error); + } + void testConstructionWithDuration() { io_context ioc(Backend); @@ -1225,6 +1284,11 @@ struct timer_test { // Construction and move semantics testConstruction(); + testNonIoContextThrows(); + testConstructFromExecutor(); + testConstructFromExecutorWithTimePoint(); + testConstructFromExecutorWithDuration(); + testExecutorNonIoContextThrows(); testConstructionWithTimePoint(); testConstructionWithDuration(); testMoveConstruct();