Extended Description

The following program exhibits a race condition, which can be seen by running it under Tsan:

#include
#include
#include
#include

static int worker(std::vector const& data) {
return std::accumulate(data.begin(), data.end(), 0);
}

int main() {
std::vector const v{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
for (int i = 0; i != 20; ++i) {
std::future fut = std::async(std::launch::async, worker, v);
std::future fut2 = std::async(std::launch::async, worker, v);
int answer = fut.get();
int answer2 = fut2.get();
assert(answer == answer2);
}
}

$ clang++ -std=c++11 -fsanitize=thread main.cpp -o main.exe && ./main.exe

WARNING: ThreadSanitizer: data race (pid=97622)
Write of size 4 at 0x7b2c00000088 by thread T1 (mutexes: write M14):
#​0 void std::__1::__assoc_state::set_value(int&&) :1062560 (main.exe:x86_64+0x10000556b)
#​1 std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::__execute() :1062560 (main.exe:x86_64+0x1000047de)
#​2 void std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_deletestd::__1::__thread_struct >, void (std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::)(), std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >> >(void*) :1062560 (main.exe:x86_64+0x100006b82)

Previous read of size 4 at 0x7b2c00000088 by main thread:
#​0 std::__1::future::future(std::__1::__assoc_state) :1062560 (main.exe:x86_64+0x1000071f1)
#​1 std::__1::future::future(std::__1::__assoc_state) :1062560 (main.exe:x86_64+0x1000043c8)
#​2 std::__1::future std::__1::__make_async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >(std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > >&&) :1062560 (main.exe:x86_64+0x100003a13)
#​3 std::__1::future<std::__1::__invoke_of<std::__1::decay<int (&)(std::__1::vector<int, std::__1::allocator > const&)>::type, std::__1::decay<std::__1::vector<int, std::__1::allocator > const&>::type>::type> std::__1::async<int (&)(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > const&>(std::__1::launch, int (&&&)(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > const&&&) :1062560 (main.exe:x86_64+0x10000196a)
#​4 main :1062560 (main.exe:x86_64+0x10000104c)

Location is heap block of size 176 at 0x7b2c00000000 allocated by main thread:
#​0 operator new(unsigned long) :1062592 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x6e733)
#​1 std::__1::future std::__1::__make_async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >(std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > >&&) :1062592 (main.exe:x86_64+0x1000036c8)
#​2 std::__1::future<std::__1::__invoke_of<std::__1::decay<int (&)(std::__1::vector<int, std::__1::allocator > const&)>::type, std::__1::decay<std::__1::vector<int, std::__1::allocator > const&>::type>::type> std::__1::async<int (&)(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > const&>(std::__1::launch, int (&&&)(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > const&&&) :1062592 (main.exe:x86_64+0x10000196a)
#​3 main :1062592 (main.exe:x86_64+0x10000104c)

Mutex M14 (0x7b2c00000018) created at:
#​0 pthread_mutex_lock :1062448 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x3945e)
#​1 std::__1::mutex::lock() :1062448 (libc++.1.dylib:x86_64+0x3a698)
#​2 std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::__execute() :1062448 (main.exe:x86_64+0x1000047de)
#​3 void std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_deletestd::__1::__thread_struct >, void (std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::)(), std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >> >(void*) :1062448 (main.exe:x86_64+0x100006b82)

Thread T1 (tid=572648, running) created by main thread at:
#​0 pthread_create :1062640 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x2a13d)
#​1 std::__1::thread::thread<void (std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::)(), std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >, void>(void (std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::&&)(), std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >&&) :1062640 (main.exe:x86_64+0x1000060d0)
#​2 std::__1::thread::thread<void (std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::)(), std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >, void>(void (std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >::&&)(), std::__1::__async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >&&) :1062640 (main.exe:x86_64+0x100004358)
#​3 std::__1::future std::__1::__make_async_assoc_state<int, std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > > >(std::__1::__async_func<int ()(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > >&&) :1062640 (main.exe:x86_64+0x1000039b0)
#​4 std::__1::future<std::__1::__invoke_of<std::__1::decay<int (&)(std::__1::vector<int, std::__1::allocator > const&)>::type, std::__1::decay<std::__1::vector<int, std::__1::allocator > const&>::type>::type> std::__1::async<int (&)(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > const&>(std::__1::launch, int (&&&)(std::__1::vector<int, std::__1::allocator > const&), std::__1::vector<int, std::__1::allocator > const&&&) :1062640 (main.exe:x86_64+0x10000196a)
#​5 main :1062640 (main.exe:x86_64+0x10000104c)

SUMMARY: ThreadSanitizer: data race (main.exe:x86_64+0x10000556b) in void std::__1::__assoc_state::set_value(int&&)

The race seems to be between the following functions:

template <class _Rp>
future<_Rp>::future(__assoc_state<_Rp>* __state)
    : __state_(__state)
{
    if (__state_->__has_future_attached())
        __throw_future_error(future_errc::future_already_retrieved);
    __state_->__add_shared();
    __state_->__set_future_attached();
}

and:

template <class _Rp>
template <class _Arg>
void __assoc_state<_Rp>::set_value(_Arg&& __arg) {
    unique_lock<mutex> __lk(this->__mut_);
    if (this->__has_value())
        __throw_future_error(future_errc::promise_already_satisfied);
    ::new(&__value_) _Rp(_VSTD::forward<_Arg>(__arg));
    this->__state_ |= base::__constructed | base::ready;
    __cv_.notify_all();
}

Those functions are run concurrently from __make_async_assoc_state:

template <class _Rp, class _Fp>
future<_Rp> __make_async_assoc_state(_Fp&& __f) {
    unique_ptr<__async_assoc_state<_Rp, _Fp>, __release_shared_count>
        __h(new __async_assoc_state<_Rp, _Fp>(_VSTD::forward<_Fp>(__f)));
    _VSTD::thread([h = __h.get()]{
        h->set_value(h->__func_()); // call to set_value()
    }).detach();
    return future<_Rp>(__h.get()); // call to future's constructor
}

The problem is that the constructor reads the state of the future (with __has_future_attached()) without synchronization, and set_value() sets that state concurrently. set_value() properly acquires the mutex, but the constructor does not.

My proposed resolution would be to take a lock in the constructor at the very beginning, like so:

template <class _Rp>
future<_Rp>::future(__assoc_state<_Rp>* __state)
    : __state_(__state)
{
    unique_lock<mutex> __lk(__state_->__mut_);
    if (__state_->__has_future_attached())
        __throw_future_error(future_errc::future_already_retrieved);
    __state_->__add_shared();
    __state_->__set_future_attached();
}