[libcxx] r340608 - [libc++] Remove race condition in std::async
Louis Dionne via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 24 07:01:00 PDT 2018
Author: ldionne
Date: Fri Aug 24 07:00:59 2018
New Revision: 340608
URL: http://llvm.org/viewvc/llvm-project?rev=340608&view=rev
Log:
[libc++] Remove race condition in std::async
Summary:
The state associated to the future was set in one thread (with synchronization)
but read in another thread without synchronization, which led to a data race.
https://bugs.llvm.org/show_bug.cgi?id=38181
rdar://problem/42548261
Reviewers: mclow.lists, EricWF
Subscribers: christof, dexonsmith, cfe-commits
Differential Revision: https://reviews.llvm.org/D51170
Added:
libcxx/trunk/test/std/thread/futures/futures.async/async_race.38682.pass.cpp
Modified:
libcxx/trunk/include/future
libcxx/trunk/src/future.cpp
Modified: libcxx/trunk/include/future
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/future?rev=340608&r1=340607&r2=340608&view=diff
==============================================================================
--- libcxx/trunk/include/future (original)
+++ libcxx/trunk/include/future Fri Aug 24 07:00:59 2018
@@ -556,13 +556,14 @@ public:
{return (__state_ & __constructed) || (__exception_ != nullptr);}
_LIBCPP_INLINE_VISIBILITY
- void __set_future_attached()
- {
+ void __attach_future() {
lock_guard<mutex> __lk(__mut_);
+ bool __has_future_attached = (__state_ & __future_attached) != 0;
+ if (__has_future_attached)
+ __throw_future_error(future_errc::future_already_retrieved);
+ this->__add_shared();
__state_ |= __future_attached;
}
- _LIBCPP_INLINE_VISIBILITY
- bool __has_future_attached() const {return (__state_ & __future_attached) != 0;}
_LIBCPP_INLINE_VISIBILITY
void __set_deferred() {__state_ |= deferred;}
@@ -1154,10 +1155,7 @@ 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();
+ __state_->__attach_future();
}
struct __release_shared_count
@@ -1257,10 +1255,7 @@ 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();
+ __state_->__attach_future();
}
template <class _Rp>
Modified: libcxx/trunk/src/future.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/future.cpp?rev=340608&r1=340607&r2=340608&view=diff
==============================================================================
--- libcxx/trunk/src/future.cpp (original)
+++ libcxx/trunk/src/future.cpp Fri Aug 24 07:00:59 2018
@@ -179,10 +179,7 @@ __assoc_sub_state::__execute()
future<void>::future(__assoc_sub_state* __state)
: __state_(__state)
{
- if (__state_->__has_future_attached())
- __throw_future_error(future_errc::future_already_retrieved);
- __state_->__add_shared();
- __state_->__set_future_attached();
+ __state_->__attach_future();
}
future<void>::~future()
Added: libcxx/trunk/test/std/thread/futures/futures.async/async_race.38682.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/futures/futures.async/async_race.38682.pass.cpp?rev=340608&view=auto
==============================================================================
--- libcxx/trunk/test/std/thread/futures/futures.async/async_race.38682.pass.cpp (added)
+++ libcxx/trunk/test/std/thread/futures/futures.async/async_race.38682.pass.cpp Fri Aug 24 07:00:59 2018
@@ -0,0 +1,58 @@
+//===----------------------------------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++98, c++03
+
+// This test is designed to cause and allow TSAN to detect a race condition
+// in std::async, as reported in https://bugs.llvm.org/show_bug.cgi?id=38682.
+
+#include <cassert>
+#include <functional>
+#include <future>
+#include <numeric>
+#include <vector>
+
+
+static int worker(std::vector<int> const& data) {
+ return std::accumulate(data.begin(), data.end(), 0);
+}
+
+static int& worker_ref(int& i) { return i; }
+
+static void worker_void() { }
+
+int main() {
+ // future<T>
+ {
+ 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);
+ int answer = fut.get();
+ assert(answer == 55);
+ }
+ }
+
+ // future<T&>
+ {
+ for (int i = 0; i != 20; ++i) {
+ std::future<int&> fut = std::async(std::launch::async, worker_ref, std::ref(i));
+ int& answer = fut.get();
+ assert(answer == i);
+ }
+ }
+
+ // future<void>
+ {
+ for (int i = 0; i != 20; ++i) {
+ std::future<void> fut = std::async(std::launch::async, worker_void);
+ fut.get();
+ }
+ }
+}
More information about the cfe-commits
mailing list