<html>
<head>
<base href="https://bugs.llvm.org/">
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW - Race condition in __make_async_assoc_state"
href="https://bugs.llvm.org/show_bug.cgi?id=38682">38682</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>Race condition in __make_async_assoc_state
</td>
</tr>
<tr>
<th>Product</th>
<td>libc++
</td>
</tr>
<tr>
<th>Version</th>
<td>unspecified
</td>
</tr>
<tr>
<th>Hardware</th>
<td>PC
</td>
</tr>
<tr>
<th>OS</th>
<td>All
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>enhancement
</td>
</tr>
<tr>
<th>Priority</th>
<td>P
</td>
</tr>
<tr>
<th>Component</th>
<td>All Bugs
</td>
</tr>
<tr>
<th>Assignee</th>
<td>unassignedclangbugs@nondot.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>ldionne@apple.com
</td>
</tr>
<tr>
<th>CC</th>
<td>llvm-bugs@lists.llvm.org, mclow.lists@gmail.com
</td>
</tr></table>
<p>
<div>
<pre>The following program exhibits a race condition, which can be seen by running
it under Tsan:
------------------------------------------------------------------------------
#include <cassert>
#include <future>
#include <numeric>
#include <vector>
static int worker(std::vector<int> const& data) {
return std::accumulate(data.begin(), data.end(), 0);
}
int main() {
std::vector<int> const v{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
for (int i = 0; i != 20; ++i) {
std::future<int> fut = std::async(std::launch::async, worker, v);
std::future<int> 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<int>::set_value<int>(int&&) <null>: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<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >::__execute()
<null>: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_delete<std::__1::__thread_struct> >, void
(std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >::*)(),
std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >*> >(void*) <null>:1062560
(main.exe:x86_64+0x100006b82)
Previous read of size 4 at 0x7b2c00000088 by main thread:
#0 std::__1::future<int>::future(std::__1::__assoc_state<int>*)
<null>:1062560 (main.exe:x86_64+0x1000071f1)
#1 std::__1::future<int>::future(std::__1::__assoc_state<int>*)
<null>:1062560 (main.exe:x86_64+0x1000043c8)
#2 std::__1::future<int> std::__1::__make_async_assoc_state<int,
std::__1::__async_func<int (*)(std::__1::vector<int, std::__1::allocator<int> >
const&), std::__1::vector<int, std::__1::allocator<int> > >
<span class="quote">>(std::__1::__async_func<int (*)(std::__1::vector<int, std::__1::allocator<int>
> const&), std::__1::vector<int, std::__1::allocator<int> > >&&) <null>:1062560</span >
(main.exe:x86_64+0x100003a13)
#3 std::__1::future<std::__1::__invoke_of<std::__1::decay<int
(&)(std::__1::vector<int, std::__1::allocator<int> > const&)>::type,
std::__1::decay<std::__1::vector<int, std::__1::allocator<int> >
const&>::type>::type> std::__1::async<int (&)(std::__1::vector<int,
std::__1::allocator<int> > const&), std::__1::vector<int,
std::__1::allocator<int> > const&>(std::__1::launch, int
(&&&)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > const&&&) <null>:1062560
(main.exe:x86_64+0x10000196a)
#4 main <null>:1062560 (main.exe:x86_64+0x10000104c)
Location is heap block of size 176 at 0x7b2c00000000 allocated by main
thread:
#0 operator new(unsigned long) <null>:1062592
(libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x6e733)
#1 std::__1::future<int> std::__1::__make_async_assoc_state<int,
std::__1::__async_func<int (*)(std::__1::vector<int, std::__1::allocator<int> >
const&), std::__1::vector<int, std::__1::allocator<int> > >
<span class="quote">>(std::__1::__async_func<int (*)(std::__1::vector<int, std::__1::allocator<int>
> const&), std::__1::vector<int, std::__1::allocator<int> > >&&) <null>:1062592</span >
(main.exe:x86_64+0x1000036c8)
#2 std::__1::future<std::__1::__invoke_of<std::__1::decay<int
(&)(std::__1::vector<int, std::__1::allocator<int> > const&)>::type,
std::__1::decay<std::__1::vector<int, std::__1::allocator<int> >
const&>::type>::type> std::__1::async<int (&)(std::__1::vector<int,
std::__1::allocator<int> > const&), std::__1::vector<int,
std::__1::allocator<int> > const&>(std::__1::launch, int
(&&&)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > const&&&) <null>:1062592
(main.exe:x86_64+0x10000196a)
#3 main <null>:1062592 (main.exe:x86_64+0x10000104c)
Mutex M14 (0x7b2c00000018) created at:
#0 pthread_mutex_lock <null>:1062448
(libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x3945e)
#1 std::__1::mutex::lock() <null>: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<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >::__execute()
<null>: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_delete<std::__1::__thread_struct> >, void
(std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >::*)(),
std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >*> >(void*) <null>:1062448
(main.exe:x86_64+0x100006b82)
Thread T1 (tid=572648, running) created by main thread at:
#0 pthread_create <null>: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<int> >
const&), std::__1::vector<int, std::__1::allocator<int> > > >::*)(),
std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >*, void>(void
(std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >::*&&)(),
std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >*&&) <null>: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<int> >
const&), std::__1::vector<int, std::__1::allocator<int> > > >::*)(),
std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >*, void>(void
(std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >::*&&)(),
std::__1::__async_assoc_state<int, std::__1::__async_func<int
(*)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > > >*&&) <null>:1062640
(main.exe:x86_64+0x100004358)
#3 std::__1::future<int> std::__1::__make_async_assoc_state<int,
std::__1::__async_func<int (*)(std::__1::vector<int, std::__1::allocator<int> >
const&), std::__1::vector<int, std::__1::allocator<int> > >
<span class="quote">>(std::__1::__async_func<int (*)(std::__1::vector<int, std::__1::allocator<int>
> const&), std::__1::vector<int, std::__1::allocator<int> > >&&) <null>:1062640</span >
(main.exe:x86_64+0x1000039b0)
#4 std::__1::future<std::__1::__invoke_of<std::__1::decay<int
(&)(std::__1::vector<int, std::__1::allocator<int> > const&)>::type,
std::__1::decay<std::__1::vector<int, std::__1::allocator<int> >
const&>::type>::type> std::__1::async<int (&)(std::__1::vector<int,
std::__1::allocator<int> > const&), std::__1::vector<int,
std::__1::allocator<int> > const&>(std::__1::launch, int
(&&&)(std::__1::vector<int, std::__1::allocator<int> > const&),
std::__1::vector<int, std::__1::allocator<int> > const&&&) <null>:1062640
(main.exe:x86_64+0x10000196a)
#5 main <null>:1062640 (main.exe:x86_64+0x10000104c)
SUMMARY: ThreadSanitizer: data race (main.exe:x86_64+0x10000556b) in void
std::__1::__assoc_state<int>::set_value<int>(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();
}</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>