Skip to content

Timer (Asio): a cancelled or replaced callback still runs once its wait has expired #43

Closed

Description

@pgit-bot

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() in include/anyhttp/net.hpp: [&stream] { io::cancel(stream); }
  • the shutdown in include/anyhttp/h2/session_details.hpp and in include/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

  1. added a commit that references this issue on Nov 9, 2026
    250f52b
  2. pgit commented on Nov 9, 2026

    @pgit
    Owner

    Resolved differently from the proposed fix, in 1271bc5 (PR #44); the issue closes when that is merged.

    Decision

    Timer keeps 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 a std::stop_source allocation 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 that arm(), 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.
    • The shutdowns in h2/session_details.hpp and h2v2/session_details.hpp follow the pattern for such handlers: name the weak pointer weak and the locked one self, and capture no this:
      timer.arm(std::chrono::seconds(2), [weak = this->weak_from_this()] {
         // may come late, see Timer
         if (auto self = std::static_pointer_cast<ClientSession>(weak.lock()))
            io::cancel(self->stream_);
      });
    • 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 pass weak_from_this(): h1/session.cpp, h2/session_details.hpp, h2v2/session_details.hpp. Every session comes from make_shared, so that pointer is never empty while a session runs.
    • The session timers of QUIC and h2v2 already capture a weak_ptr and stay as they are. A late handle_expiry() finds nothing due.

    On the Corosio Timer

    It 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 Timer taken 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 Timer no longer makes. WHEN_timer_is_rearmed_or_cancelled_THEN_only_the_last_callback_runs moved from test_corosio.cpp to test_runtime.cpp and 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) and ClientTlsEnd.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

    • A callback-style timer: cancel without a stop token, and make the callback safe to run once more after cancel(), re-arm or destruction. Hold a weak_ptr and lock it (weak / self), never this or a reference into a frame.
    • Calls to io::drain(stream, executor, timeout) become io::drain(stream, weak_from_this(), executor, timeout).
    • Where c-ares-async added a stop token to its timer for the same reason, it can drop the token on the same grounds, once its callbacks follow the pattern above.
  3. added 2 commits that reference this issue on Nov 9, 2026
    1271bc5
    0420cf7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions