Question

If I set a set_wait_callback on a boost::unique_future, is it guaranteed to only run once?

I'm a bit suspicious since when looking at the source code I find the following:

struct relocker
{
    boost::unique_lock<boost::mutex>& lock;

    relocker(boost::unique_lock<boost::mutex>& lock_):
        lock(lock_)
    {
        lock.unlock();
    }
    ~relocker()
    {
        lock.lock();
    }
private:
    relocker& operator=(relocker const&);
};

void do_callback(boost::unique_lock<boost::mutex>& lock)
{
    if(callback && !done)
    {
        boost::function<void()> local_callback=callback;
        relocker relock(lock); // unlock mutex?
        local_callback();
    }
}    

void wait(bool rethrow=true)
{
    boost::unique_lock<boost::mutex> lock(mutex);
    do_callback(lock);
    while(!done)
    {
        waiters.wait(lock);
    }
    if(rethrow && exception)
    {
        boost::rethrow_exception(exception);
    }
}

Where in do_callback the mutex is actually unlocked while the callback is invoked, which from my understanding can lead to the callback being called multiple times if multiple threads call the wait function?

Can the callback be called multiple times? Is it by design? Or am I missing something?

The reason I'm a bit surprised is that in the C++11 standard the async(std::launch::deferred, ...) (to which set_wait_callback is a cousin), seems to have single invocation guarantee:

§30.6.8

The shared state is not made ready until the function has completed. The first call to a non-timed waiting function (30.6.4) on an asynchronous return object referring to this shared state shall invoke the deferred function in the thread that called the waiting function.

Was it helpful?

Solution

I think your suspicion is founded. The code should look like

void do_callback(boost::unique_lock<boost::mutex>& lock)
{
    if(callback && !done)
    {
        boost::function<void()> local_callback=callback;
        callback=boost::function<void()>;
        relocker relock(lock); // unlock mutex?
        local_callback();
    }
}    

or even

void do_callback(boost::unique_lock<boost::mutex>& lock)
{
    if(callback && !done)
    {
        boost::function<void()> local_callback=boos::move(callback);
        relocker relock(lock); // unlock mutex?
        local_callback();
    }
}    

once Boost.Function will support move semantics.

This has yet some issues as another thread could call set_wait_callback, so callback could be reassigned and two callbacks could be called. It seems that an additional state is needed to state if the callback has already been done.

void do_callback(boost::unique_lock<boost::mutex>& lock)
{
    if(callback && ! callback_done && !done)
    {
        boost::function<void()> local_callback=callback;
        callback_done=true;
        relocker relock(lock); // unlock mutex?
        local_callback();
    }
}    

BTW, set_wait_callback is not thread-safe.

    template<typename F,typename U>
    void set_wait_callback(F f,U* u)
    {
        callback=boost::bind(f,boost::ref(*u));
    }

and must be protected

    template<typename F,typename U>
    void set_wait_callback(F f,U* u)
    {
        boost::lock_guard<boost::mutex> lock(mutex);
        callback=boost::bind(f,boost::ref(*u));
    }

Please, could you create a Trac ticket to Boost Thread so this issue is not lost?

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top