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(); + } + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
