[libcxx] r239577 - Fix PR23293 - Do not unlock shared state before notifying consumers.

[Eric Fiselier]

Author: ericwf
Date: Thu Jun 11 19:41:34 2015
New Revision: 239577

URL: http://llvm.org/viewvc/llvm-project?rev=239577&view=rev
Log:
Fix PR23293 - Do not unlock shared state before notifying consumers.

Within the shared state methods do not unlock the lock guards manually. This
could cause a race condition where the shared state is destroyed before the
method is complete.

Added:
    libcxx/trunk/test/std/thread/futures/futures.async/async_race.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=239577&r1=239576&r2=239577&view=diff
==============================================================================
--- libcxx/trunk/include/future (original)
+++ libcxx/trunk/include/future Thu Jun 11 19:41:34 2015
@@ -651,7 +651,6 @@ __assoc_state<_Rp>::set_value(_Arg& __ar
 #endif
     ::new(&__value_) _Rp(_VSTD::forward<_Arg>(__arg));
     this->__state_ |= base::__constructed | base::ready;
-    __lk.unlock();
     __cv_.notify_all();
 }
 
@@ -672,7 +671,6 @@ __assoc_state<_Rp>::set_value_at_thread_
     ::new(&__value_) _Rp(_VSTD::forward<_Arg>(__arg));
     this->__state_ |= base::__constructed;
     __thread_local_data()->__make_ready_at_thread_exit(this);
-    __lk.unlock();
 }
 
 template <class _Rp>
@@ -733,7 +731,6 @@ __assoc_state<_Rp&>::set_value(_Rp& __ar
 #endif
     __value_ = _VSTD::addressof(__arg);
     this->__state_ |= base::__constructed | base::ready;
-    __lk.unlock();
     __cv_.notify_all();
 }
 
@@ -749,7 +746,6 @@ __assoc_state<_Rp&>::set_value_at_thread
     __value_ = _VSTD::addressof(__arg);
     this->__state_ |= base::__constructed;
     __thread_local_data()->__make_ready_at_thread_exit(this);
-    __lk.unlock();
 }
 
 template <class _Rp>

Modified: libcxx/trunk/src/future.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/future.cpp?rev=239577&r1=239576&r2=239577&view=diff
==============================================================================
--- libcxx/trunk/src/future.cpp (original)
+++ libcxx/trunk/src/future.cpp Thu Jun 11 19:41:34 2015
@@ -98,7 +98,6 @@ __assoc_sub_state::set_value()
 #endif
     __state_ |= __constructed | ready;
     __cv_.notify_all();
-    __lk.unlock();
 }
 
 void
@@ -111,7 +110,6 @@ __assoc_sub_state::set_value_at_thread_e
 #endif
     __state_ |= __constructed;
     __thread_local_data()->__make_ready_at_thread_exit(this);
-    __lk.unlock();
 }
 
 void
@@ -124,7 +122,6 @@ __assoc_sub_state::set_exception(excepti
 #endif
     __exception_ = __p;
     __state_ |= ready;
-    __lk.unlock();
     __cv_.notify_all();
 }
 
@@ -138,7 +135,6 @@ __assoc_sub_state::set_exception_at_thre
 #endif
     __exception_ = __p;
     __thread_local_data()->__make_ready_at_thread_exit(this);
-    __lk.unlock();
 }
 
 void
@@ -146,7 +142,6 @@ __assoc_sub_state::__make_ready()
 {
     unique_lock<mutex> __lk(__mut_);
     __state_ |= ready;
-    __lk.unlock();
     __cv_.notify_all();
 }
 

Added: libcxx/trunk/test/std/thread/futures/futures.async/async_race.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/futures/futures.async/async_race.pass.cpp?rev=239577&view=auto
==============================================================================
--- libcxx/trunk/test/std/thread/futures/futures.async/async_race.pass.cpp (added)
+++ libcxx/trunk/test/std/thread/futures/futures.async/async_race.pass.cpp Thu Jun 11 19:41:34 2015
@@ -0,0 +1,67 @@
+//===----------------------------------------------------------------------===//
+//
+//                     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
+
+// <future>
+
+// template <class F, class... Args>
+//     future<typename result_of<F(Args...)>::type>
+//     async(F&& f, Args&&... args);
+
+// template <class F, class... Args>
+//     future<typename result_of<F(Args...)>::type>
+//     async(launch policy, F&& f, Args&&... args);
+
+// This test is designed to cause and allow TSAN to detect the race condition
+// reported in PR23293. (http://llvm.org/PR23293).
+
+#include <future>
+#include <chrono>
+#include <thread>
+#include <memory>
+#include <cassert>
+
+int f_async() {
+    typedef std::chrono::milliseconds ms;
+    std::this_thread::sleep_for(ms(200));
+    return 42;
+}
+
+bool ran = false;
+
+int f_deferred() {
+    ran = true;
+    return 42;
+}
+
+void test_each() {
+    {
+        std::future<int> f = std::async(f_async);
+        int const result = f.get();
+        assert(result == 42);
+    }
+    {
+        std::future<int> f = std::async(std::launch::async, f_async);
+        int const result = f.get();
+        assert(result == 42);
+    }
+    {
+        ran = false;
+        std::future<int> f = std::async(std::launch::deferred, f_deferred);
+        assert(ran == false);
+        int const result = f.get();
+        assert(ran == true);
+        assert(result == 42);
+    }
+}
+
+int main() {
+    for (int i=0; i < 25; ++i) test_each();
+}