Repository navigation
Description
Timer says that "a callback that has been replaced or cancelled is never called" (include/anyhttp/asio/runtime.hpp). On Asio it calls one anyway if its wait has expired already and the handler is queued when cancel() or arm() comes, or when the timer is destroyed. The Corosio Timer keeps the promise: each of its waits has a stop token of its own.
Cause
steady_timer::cancel() and expires_after() cannot take back a wait that has expired already. The Asio documentation of cancel() says so: such handlers "can no longer be cancelled, and therefore are passed an error code that indicates the successful completion of the wait operation". The handler in Timer::arm() looks at the error code only, so it calls the callback.
Reproduction
Commit 2ab497b, ASIO build, clang and libc++, Boost 1.92. The timer expires before the context runs, so the reactor's first pass queues the callback ahead of the handler that cancels it:
boost::asio::io_context context;
int calls = 0;
auto timer = std::make_unique<anyhttp::Timer>(context.get_executor());
timer->arm(1ms, [&calls] { ++calls; });
std::this_thread::sleep_for(20ms);
boost::asio::post(context, [&] { timer->cancel(); }); // or arm() again, or timer.reset()
context.run_for(200ms);The callback runs once in all three cases:
callback calls after cancel(): 1 (expected 0)
callback calls after re-arm(): 1 (expected 0)
callback calls after destruction: 1 (expected 0)
In a running program, the window is one turn of the event loop in which both the expiry and whatever cancels the timer come in.
Why it matters
Three places keep a Timer on the frame of a coroutine, with a callback that holds a reference, and cancel it once what they wait for is done:
io::drain()ininclude/anyhttp/net.hpp:[&stream] { io::cancel(stream); }- the shutdown in
include/anyhttp/h2/session_details.hppand ininclude/anyhttp/h2v2/session_details.hpp:[this] { io::cancel(stream_); }
If the timeout expires in the same turn as the read or the event that it guards, the callback runs after the coroutine has moved on. drain() is followed by io::close(stream_), and by the end of the session soon after, so the callback may cancel a stream that has been destroyed.
The session timers of QUIC and h2v2 capture a weak_ptr and are safe from that. A wait that has been replaced can still call their handle_expiry() once more, before the new expiry.
Fix
The same as the Corosio Timer, and as c-ares-async has had since its Corosio port: every wait carries a stop token, and the next arm(), cancel() and the destructor trigger it. With this, the reproduction prints 0 three times:
class Timer
{
public:
explicit Timer(const Executor& executor) : timer_(executor) {}
+ Timer(const Timer&) = delete;
+ Timer& operator=(const Timer&) = delete;
+ ~Timer() { cancel(); }
template <typename Rep, typename Period, typename F>
void arm(std::chrono::duration<Rep, Period> delay, F&& on_expiry)
{
+ cancel();
+ stop_.emplace();
timer_.expires_after(delay);
- timer_.async_wait([on_expiry = std::forward<F>(on_expiry)](const error_code& ec) mutable {
- if (!ec)
+ timer_.async_wait([on_expiry = std::forward<F>(on_expiry),
+ token = stop_->get_token()](const error_code& ec) mutable {
+ if (!ec && !token.stop_requested())
on_expiry();
});
}
- void cancel() { timer_.cancel(); }
+ void cancel()
+ {
+ if (stop_)
+ stop_->request_stop();
+ stop_.reset();
+ timer_.cancel();
+ }
private:
boost::asio::steady_timer timer_;
+ std::optional<std::stop_source> stop_;
};It needs <optional> and <stop_token>. The destructor also makes Timer no longer movable. Nothing I found moves one, but I have not built anyhttp with the change.
Done when
A test runs the three cases of the reproduction on both runtimes, and no callback runs. The existing Corosio test, WHEN_timer_is_rearmed_or_cancelled_THEN_only_the_last_callback_runs, re-arms before the first wait expires, so it does not cover this.
Activity
fix: timeout callbacks hold their session, not this
pgit commentedon Nov 9, 2026
Resolved differently from the proposed fix, in 1271bc5 (PR #44); the issue closes when that is merged.
Decision
Timerkeeps Asio's contract and does not try to improve on it. A stop token per wait was rejected as too heavy for what it buys:arm()runs after every packet on QUIC, and it would cost astd::stop_sourceallocation each time. The guarantee also belongs to the handler, not the timer: cancelling an asynchronous operation means it completes soon, either cancelled or successfully, and a wait whose delay has expired already completes successfully. A callback that runs once late, shortly before the cancellation or the new expiry would have taken effect, is acceptable.What was wrong was the promise in the doc comment ("never called") and the three callbacks that relied on it by referring to what they cancel.
What changed
Timer(include/anyhttp/asio/runtime.hpp): code unchanged. The doc comment now says thatarm(),cancel()and the destructor stop a wait that is still pending, but that a callback whose delay has expired already still runs once, soon after. So a callback holds what it uses rather than refer to it, and a late call must do no harm.h2/session_details.hppandh2v2/session_details.hppfollow the pattern for such handlers: name the weak pointerweakand the locked oneself, and capture nothis:io::drain()(include/anyhttp/net.hpp) has a new second parameter, the owner of its stream,std::weak_ptr<const void> owner. Since it is generic over the stream, it still captures&stream, but uses it only while the owner can be locked:[&stream, weak = std::move(owner)] { if (auto self = weak.lock()) io::cancel(stream); }. Its callers passweak_from_this():h1/session.cpp,h2/session_details.hpp,h2v2/session_details.hpp. Every session comes frommake_shared, so that pointer is never empty while a session runs.weak_ptrand stay as they are. A latehandle_expiry()finds nothing due.On the Corosio
TimerIt is not proof that the runtime offers more. corosio's timer cannot take back an expired wait either. With the stop-token re-check in our corosio
Timertaken out, the three cases of the reproduction ran the callback once there too, as on Asio. The stronger behavior is that one line of anyhttp's wrapper, which costs nothing extra there only because the stop token is how a detached capy wait gets cancelled at all. It stays for now, but code must not rely on it, since the generic contract is Asio's.Tests
The "done when" of this issue no longer applies: the three after-expiry cases would test a promise
Timerno longer makes.WHEN_timer_is_rearmed_or_cancelled_THEN_only_the_last_callback_runsmoved fromtest_corosio.cpptotest_runtime.cppand runs on both runtimes now. The timeouts still fire through the weak pointers:TlsConnectionEnd.WHEN_client_never_answers_the_close_notify_THEN_server_ends_anyway(about 2.0 s) andClientTlsEnd.WHEN_write_is_stuck_THEN_shutdown_interrupts_it_and_says_so/HTTP2(about 2.7 s). The full suites pass on ASIO, COROSIO and h2v2.For code that adapts this
cancel(), re-arm or destruction. Hold aweak_ptrand lock it (weak/self), neverthisor a reference into a frame.io::drain(stream, executor, timeout)becomeio::drain(stream, weak_from_this(), executor, timeout).fix: timeout callbacks hold their session, not this
Bring the runtime layer to anyhttp's, as it is now