1

I have a class called Timer that exposes two methods called start and stop.

void Timer::start() {
    _enabled.store(true, std::memory_order::memory_order_release);
    _timer.expires_from_now(_delay);
    _timer.async_wait(
        std::bind(
            &Timer::tick,
            this,
            std::placeholders::_1
        ));
}

void Timer::stop() {
    _enabled.store(false, std::memory_order::memory_order_release);
    _timer.cancel();
}

void Timer::tick(const boost::system::error_code& error) {
    if (error) return;
    if (!_enabled.load(std::memory_order::memory_order_acquire)) return;
    try {
        _task();
    } catch (...) {}
    if (_enabled.load(std::memory_order::memory_order_acquire)) {
        _timer.expires_from_now(_delay);
        _timer.async_wait(
            std::bind(
                &Timer::tick,
                this,
                std::placeholders::_1
            ));
    }
}

Another class that uses an instance of Timer (handler is executed on some other thread in a ThreadPool instance) calls stop in its destructor. From the Boost Documentation, it is now possible that the handler will be invoked and these two functions will be executed concurrently and the handler may try to access freed resources.

SomeOtherClass::~SomeOtherClass() {
    _timer.stop();
    // somehow wait for _timer handler to execute
    // delete[] some_thing;
    // other destructive things
}

Is there anyway to wait for the handler to finish execution? I've been scratching my head all day, I am quite new to Boost, so perhaps I made a design flaw. Any help would be greatly appreciated, thanks.

ZachPerkitny
  • 123
  • 3

1 Answers1

1

A pattern is to have a shared_ptr to your Timer (by using std::enable_shared_from_this).

That way you can keep the timer alive as long as the handler hasn't been executed (by keeping a copy of the shared pointer bound to the handler).

Other solutions could be:

  • having externally allocated timers (e.g. in a container with reference stability, like a std::list) where you delete them manually when they're no longer needed
  • running a dedicated io_service on your own thread, so you can join the thread to await the work on the io_service.

Depending on your use cases/load patterns, one approach will be better than the others.

Samples:

I picked answers that have some contrasting approaches (some not using Boost Asio) so you can see the trade-offs and what changes between approaches.

sehe
  • 374,641
  • 47
  • 450
  • 633
  • Hi, thanks for the response. This solves the issue of accessing the `Timer` object that was destroyed, but how do I solve the issue of the handler accessing `some_thing` (it's freed in the destructor in my exmaple) on the `SomeOtherClass` instance? I was thinking of adding a `condition_variable` and calling `notify_all` at the end of the `tick` method (it would be invoked even if there's an error, and `_enabled` is `false`). – ZachPerkitny Jun 02 '20 at 15:14
  • In general "old-fashioned" thread synchronization primitives don't work too well with Asio, because, for one thing, you might not be using threads at all. – sehe Jun 02 '20 at 15:16
  • Other than that, you can't "lock" anything across an async operation without almost definitely inviting deadlocks, (Basically, locking is counter to the idea of async operations, because no async task should ever block). – sehe Jun 02 '20 at 15:16
  • 1
    So. If you need synchronized access to resources, use a stramd: _["Strands: Use Threads Without Explicit Locking"](https://www.boost.org/doc/libs/1_73_0/doc/html/boost_asio/overview/core/strands.html)_. – sehe Jun 02 '20 at 15:21
  • If you really need some kind of barrier type signal I have found minimal need for it, but you **can** fake that with a timer. See e.g. https://stackoverflow.com/questions/26739647/proper-cleanup-with-a-suspended-coroutine/26740429#26740429 The trick is with the timer that "never" expires: `write_event_.expires_at(boost::posix_time::pos_infin);`. You can treat it as a manual-reset event. – sehe Jun 02 '20 at 15:22
  • The issue isn't really about handler synchronization. Some other thread will modify `some_thing` (not from another event handler) and the handler is responsible for checking for changes periodically. Is the design just incorrect ? – ZachPerkitny Jun 02 '20 at 16:13
  • 1
    I think I'm going to reconsider my design and have to get used to async programming, thanks. I accepted your answer. – ZachPerkitny Jun 02 '20 at 16:30
  • @ZachPerkitny yeah, in the proactor it would make more sense to push jobs when they become relevant than to poll. If you have to "poll" try making it async-waitable. A timer would seem neutral from your description ("checking for changes periodically") – sehe Jun 02 '20 at 17:37
  • Forgot to add the link to [proactor pattern](https://www.boost.org/doc/libs/1_73_0/doc/html/boost_asio/overview/core/async.html) – sehe Jun 02 '20 at 17:46
  • Note that a `weak_ptr` can be used in lieu of a `shared_ptr` if you don't want to prolong the lifetime of the object owning the timer. In the handler, you have to `lock()` the weak pointer and do nothing if it expired. – Emile Cormier Feb 18 '23 at 23:01