2

I have a boost io_service running in a thread, and I would like to fire a callback in that thread 6 seconds after a certain event happens to a client, and reset the timer for that client if it is already running.

I maintain a unordered_map<string, shared_ptr<deadline_timer>> with a timer for each client.

However, upon setting async_wait, my callback does not fire after the alloted amount of time (the io_service IS running), neither does it fire (with an error code) when I reset the pointer (which should call the destructor for the existing timer, causing it to post to the service). How can I fix this?

This is the relevant part of my code:

auto it = timersByClientId.find(clientId);
if (it == timersByClientId.end())
{
    onLogonChangeCallback(clientId, true);
    timersByClientId[clientId].reset(
        new boost::asio::deadline_timer(replyService, boost::posix_time::seconds(6))
    );
    it = timersByClientId.find(clientId);
}
else
{
    // Cancel current wait operation (should fire the callback with an error code)
    it->second.reset(
        new boost::asio::deadline_timer(replyService, boost::posix_time::seconds(6))
    );
}
it->second->async_wait([this, clientId](const boost::system::error_code& err) {
    if (!err)
    {
        onLogonChangeCallback(clientId, false);
    }
});

If it changes anything, I'm running under Visual C++ 2010 and boost 1.47.0.

Renan Gemignani
  • 2,683
  • 1
  • 21
  • 23
  • The first thing I would check is to verify that the service is running, and hasn't exited prematurely for any reason. The one that I see most often is that there was no work on the queue, causing the `run()` to return. From the snippet above, I don't see anything obviously wrong. – Dave S May 02 '14 at 18:55
  • The service IS running (I have initialized the service with a `boost::asio::io_service::work`), and if I dispatch from another source (via `replyService.post([](){ /* something */ });`, it runs normally. – Renan Gemignani May 02 '14 at 19:00

1 Answers1

5

Your code /looks/ okay-ish.

I'm not sure how you are reaching the conclusion that your completion handler doesn't "[...] fire (with an error code) when I reset the pointer". You are ignoring this case (there is no else branch in the lambda).

How about writing the logic more clearly?

void foo(int clientId) {
    shared_timer& timer = timersByClientId[clientId];

    if (!timer) 
        onLogonChangeCallback(clientId, true);

    timer = make_timer(); // reset

    timer->async_wait([this, clientId](const boost::system::error_code& err) {
        if (!err)
            onLogonChangeCallback(clientId, false);
    });
}

Here's a full demo with that else branch to let you see what is going on. I assumed 1 service thread.

See it Live On Coliru.

The test load is 100 session activities on 16 accounts in ~0.5s. The total running time is ~1.5s because I have reduced the session expiration from 6s to 1s for Coliru.

If you didn't want the destructor of LogonManager to wait for all sessions to expire, then clear the session table before joining the background thread:

~LogonMonitor() {
    work = boost::none;
    timersByClientId.clear();
    background.join();
}

Full Listing

#include <iostream>
#include <boost/asio.hpp>
#include <boost/thread.hpp>
#include <boost/optional.hpp>
#include <boost/make_shared.hpp>

struct LogonMonitor {
    LogonMonitor() 
        : work(io_service::work(replyService)), background([this]{ replyService.run(); })
    { }

    ~LogonMonitor() {
        work = boost::none;
        // timersByClientId.clear();
        background.join();
    }

    void foo(int clientId) {
        shared_timer& timer = timersByClientId[clientId];

        if (!timer) 
            onLogonChangeCallback(clientId, true);

        timer = make_timer(); // reset

        timer->async_wait([this, clientId](const boost::system::error_code& err) {
            if (!err)
                onLogonChangeCallback(clientId, false);
            else
                std::cout << "(cancel " << clientId << " timer)" << std::endl;
        });
    }

  private:
    using io_service   = boost::asio::io_service;
    using timer        = boost::asio::deadline_timer;
    using shared_timer = boost::shared_ptr<timer>;

    io_service replyService;
    boost::optional<io_service::work> work;
    boost::thread background;

    std::map<int, shared_timer> timersByClientId;

    shared_timer make_timer() { 
        return boost::make_shared<timer>(replyService, boost::posix_time::seconds(/*6*/1)); 
    }

    void onLogonChangeCallback(int clientId, bool newLogon) 
    {
        std::cout << __FUNCTION__ << "(" << clientId << ", " << newLogon << ")" << std::endl;
    }
};

int main()
{
    LogonMonitor instance;
    for (int i = 0; i < 100; ++i)
    {
        instance.foo(rand() % 16);
        boost::this_thread::sleep_for(boost::chrono::milliseconds(rand() % 10));
    }
}
BЈовић
  • 62,405
  • 41
  • 173
  • 273
sehe
  • 374,641
  • 47
  • 450
  • 633
  • @RenanGemignani now I'm curious. What part of the answer helped? – sehe May 02 '14 at 20:34
  • My conclusion was wrong. My issue was elsewhere. Apparently, VS2010 wasn't breaking when I put a breakpoint inside the lambda, and since the callback I was using was a dummy, I (wrongly) assumed it wasn't hitting the function. *hangs head in shame* I accepted your answer because it said my code looked okay-ish. – Renan Gemignani May 02 '14 at 20:35
  • Cheers :) It happens to everyone, sometimes. Thanks for the feedback. – sehe May 02 '14 at 20:35
  • I am wondering - why is `work` optional? You do set it the constructor. Because you release it in the destructor? – BЈовић Sep 17 '20 at 09:35
  • @BЈовић It's optional so it can be reset (more generally: the lifetime of the work can be managed independently of the member). Note that in more recent version of Asio, the [`executor_work_guard<>::reset`](https://www.boost.org/doc/libs/1_74_0/doc/html/boost_asio/reference/executor_work_guard.html) feature is builtin, so you don't need `optional<>` or `unique_ptr<>` for that anymore. – sehe Sep 18 '20 at 13:39