What is wrong with the mutexes in this attempt at a `std::future` implementation for C++03?

boost, boost-thread, c++, c++03, future

Solution

There are data races on `future_shared_state::value` and `future_shared_state::error`, `set_value` and `set_exception` access them without acquiring the mutex that `wait` uses to guard them.

The actual problem you are experiencing is due to your use of `boost::mutex::scoped_lock` in the callers of `future_shared_state::wait`: you successfully avoid locking the mutex twice with `adopt_lock` inside `future_shared_state::wait`, but both of the `scoped_lock` destructors run and unlock the mutex twice.

Both problems are easily fixed by making the locking all internal to `future_shared_state` (Demo at Coliru):

namespace detail {

    template <typename T>
    struct future_shared_state
    {
    public:

        void wait() const
        {
            boost::mutex::scoped_lock lock(m);
            while (!(state || error))
                available.wait(lock);
        }

        T& get()
        {
            if (state)
                return *state;

            if (error)
                throw *error;

            throw std::runtime_error("WTF");
        }

        template <typename U>
        void set_value(const U& value)
        {
            {
                boost::mutex::scoped_lock lock(m);
                state = value;
            }
            available.notify_all();
        }

        void set_exception(boost::shared_ptr<std::exception> e)
        {
            {
                boost::mutex::scoped_lock lock(m);
                error = e;
            }
            available.notify_all();
        }

    private:
        mutable boost::condition_variable available;
        boost::optional<T> state;
        boost::shared_ptr<std::exception> error;

        mutable boost::mutex m;
    };

}

template <typename T>
class promise;

template <typename T>
struct future
{
public:
    future() {}
    ~future() {}

    T get()
    {
        boost::shared_ptr<detail::future_shared_state<T> > old_box;
        swap(box, old_box);

        old_box->wait();
        return old_box->get();
    }

    bool valid() const
    {
        return !!box;
    }

    void wait() const
    {
        box->wait();
    }

private:
    boost::shared_ptr<detail::future_shared_state<T> > box;

    friend class promise<T>;
    future(boost::shared_ptr<detail::future_shared_state<T> > const& box) : box(box) {}
};

template <typename T>
struct promise
{
public:
    promise() : box(new detail::future_shared_state<T>) {}
    ~promise() {}   

    void swap(promise& other)
    {
        box.swap(other.box);
    }

    future<T> get_future()
    {
        return future<T>(box);
    }

    void set_value(T const& value)
    {
        box->set_value(value);
    }

    void set_exception(boost::shared_ptr<std::exception> e)
    {
        box->set_exception(e);
    }

private:
    boost::shared_ptr<detail::future_shared_state<T> > box;
};

Problem

I'm trying to adapt Martinho Fernandes's sample `std::future` implementation so that it can run under C++03 with Boost 1.40, as a cheap, stop-gap measure until I can gain access to either Boost 1.41 or C++11 itself. My adaption is hardly beautiful and it's certainly not optimal, but I was rather hoping it would at least work. However, under gcc version 4.4.1 20090725 (Red Hat 4.4.1-2), it does not. Here's `futures.h`: ``` #include <boost/thread.hpp> #include <boost/shared_ptr.hpp> #include <stdexcept> template <typename T> class future; template <typename T> class promise; namespace detail { template <typename T> struct future_shared_state { public: void wait(boost::mutex& m) const { boost::mutex::scoped_lock lock(m, boost::adopt_lock); while (!(state || error)) available.wait(lock); } T& get() { if (state) return *state; if (error) throw *error; throw std::runtime_error("WTF"); } template <typename U> void set_value(const U& value) { state = value; available.notify_all(); } void set_exception(boost::shared_ptr<std::exception> e) { error = e; available.notify_all(); } private: mutable boost::condition_variable available; boost::optional<T> state; boost::shared_ptr<std::exception> error; friend class promise<T>; friend class future<T>; mutable boost::mutex m; }; } template <typename T> struct future { public: future() {} ~future() {} T get() { boost::shared_ptr<detail::future_shared_state<T> > old_box; swap(box, old_box); boost::mutex::scoped_lock lock(old_box->m); old_box->wait(old_box->m); return old_box->get(); } bool valid() const { return !!box; } void wait() const { boost::mutex::scoped_lock lock(box->m); box->wait(box->m); } private: boost::shared_ptr<detail::future_shared_state<T> > box; friend class promise<T>; future(boost::shared_ptr<detail::future_shared_state<T> > const& box) : box(box) {} }; template <typename T> struct promise { public: promise() : box(new detail::future_shared_state<T>) {} ~promise() {} void swap(promise& other) { box.swap(other.box); } future<T> get_future() { return future<T>(box); } void set_value(T const& value) { box->set_value(value); } void set_exception(boost::shared_ptr<std::exception> e) { box->set_exception(e); } private: boost::shared_ptr<detail::future_shared_state<T> > box; }; template<typename T> void swap(promise<T>& lhs, promise<T>& rhs) { lhs.swap(rhs); } ``` And the execution: ``` #include "futures.h" #include <iostream> #include <boost/thread.hpp> void foo(promise<unsigned int> p) { sleep(1); p.set_value(42); } int main() { promise<unsigned int> p; future <unsigned int> f(p.get_future()); boost::thread t(&foo, p); std::cout << f.get() << std::endl; t.join(); } // g++ -O0 -g test.cpp -lboost_thread -pthread -o test ``` The result, consistently, is the following: ``` 42 test: /usr/local/include/boost/thread/pthread/mutex.hpp:45: boost::mutex::~mutex(): Assertion `!pthread_mutex_destroy(&m)' failed. Aborted ``` Backtrace from gdb: ``` #0 0x00679422 in __kernel_vsyscall () #1 0x002ac781 in *__GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #2 0x002ae04a in *__GI_abort () at abort.c:88 #3 0x002a58de in *__GI___assert_fail (assertion=0x805de56 "!pthread_mutex_destroy(&m)", file=0x805de24 "/usr/local/include/boost/thread/pthread/mutex.hpp", line=45, function=0x805e071 "boost::mutex::~mutex()") at assert.c:78 #4 0x0804bdd5 in boost::mutex::~mutex (this=0x806c9c0, __in_chrg=<value optimized out>) at /usr/local/include/boost/thread/pthread/mutex.hpp:45 #5 0x0804d020 in detail::future_shared_state<unsigned int>::~future_shared_state (this=0x806c980, __in_chrg=<value optimized out>) at futures.h:35 #6 0x0804d099 in boost::checked_delete<detail::future_shared_state<unsigned int> > (x=0x806c980) at /usr/local/include/boost/checked_delete.hpp:34 #7 0x0804d69c in boost::detail::sp_counted_impl_p<detail::future_shared_state<unsigned int> >::dispose (this=0x806c9e0) at /usr/local/include/boost/smart_ptr/detail/sp_counted_impl.hpp:78 #8 0x0804bb68 in boost::detail::sp_counted_base::release (this=0x806c9e0) at /usr/local/include/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:145 #9 0x0804bbfe in boost::detail::shared_count::~shared_count (this=0xbffff634, __in_chrg=<value optimized out>) at /usr/local/include/boost/smart_ptr/detail/shared_count.hpp:217 #10 0x0804c2d4 in boost::shared_ptr<detail::future_shared_state<unsigned int> >::~shared_ptr (this=0xbffff630, __in_chrg=<value optimized out>) at /usr/local/include/boost/smart_ptr/shared_ptr.hpp:169 #11 0x0804c535 in promise<unsigned int>::~promise (this=0xbffff630, __in_chrg=<value optimized out>) at futures.h:125 #12 0x0804b937 in main () at test.cpp:19 ``` Awful style aside, what am I doing wrong with my mutexes?

Original source