re PR libstdc++/60966 (std::call_once sometime hangs)

	PR libstdc++/60966
	* include/std/future (__future_base::_State_baseV2::_M_set_result):
	Pass lock into _M_do_set and hold it until the function returns.
	Signal condition variable after call_once completes.
	(__future_base::_State_baseV2::_M_do_set): Use lock argument. Do not
	signal here.
	* testsuite/30_threads/promise/60966.cc: New.

From-SVN: r210556

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 7dca067a96753875e6f07db3ed2e98b5416d9b6d..8c6b15b346f1543287cdb19bd4bd5e4d7996eb7c 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,8 +1,18 @@
+2014-05-17  Jonathan Wakely  <jwakely@redhat.com>
+
+       PR libstdc++/60966
+       * include/std/future (__future_base::_State_baseV2::_M_set_result):
+       Pass lock into _M_do_set and hold it until the function returns.
+       Signal condition variable after call_once completes.
+       (__future_base::_State_baseV2::_M_do_set): Use lock argument. Do not
+       signal here.
+       * testsuite/30_threads/promise/60966.cc: New.
+
 2014-05-16  Iain Sandoe  <iain@codesourcery.com>
            Sandra Loosemore  <sandra@codesourcery.com>
 
-        * testsuite/libstdc++-abi/abi.exp: Defer setting of baseline_subdir
-        until after checking that the test is eligible to be run.
+       * testsuite/libstdc++-abi/abi.exp: Defer setting of baseline_subdir
+       until after checking that the test is eligible to be run.
 
 2014-05-16  Jonathan Wakely  <jwakely@redhat.com>
 

diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index 717ce7105df0ad0d8eb115637fb93e692fbcb161..ea11f3f03e970292bff4fa87c9e4c9367d4367bc 100644 (file)
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -365,12 +365,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       void
       _M_set_result(function<_Ptr_type()> __res, bool __ignore_failure = false)
       {
-        bool __set = __ignore_failure;
+       unique_lock<mutex> __lock(_M_mutex, defer_lock);
         // all calls to this function are serialized,
         // side-effects of invoking __res only happen once
-        call_once(_M_once, &_State_baseV2::_M_do_set, this, ref(__res),
-            ref(__set));
-        if (!__set)
+       call_once(_M_once, &_State_baseV2::_M_do_set, this,
+                 ref(__res), ref(__lock));
+       if (__lock.owns_lock())
+         _M_cond.notify_all();
+       else if (!__ignore_failure)
           __throw_future_error(int(future_errc::promise_already_satisfied));
       }
 
@@ -478,15 +480,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     private:
       void
-      _M_do_set(function<_Ptr_type()>& __f, bool& __set)
+      _M_do_set(function<_Ptr_type()>& __f, unique_lock<mutex>& __lock)
       {
-        _Ptr_type __res = __f();
-        {
-          lock_guard<mutex> __lock(_M_mutex);
-          _M_result.swap(__res);
-        }
-        _M_cond.notify_all();
-        __set = true;
+        _Ptr_type __res = __f(); // do not hold lock while running setter
+       __lock.lock();
+        _M_result.swap(__res);
       }
 
       bool _M_ready() const noexcept { return static_cast<bool>(_M_result); }
@@ -495,6 +493,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       virtual void _M_complete_async() { }
 
       // Return true if state contains a deferred function.
+      // Caller must own _M_mutex.
       virtual bool _M_has_deferred() const { return false; }
     };
 

diff --git a/libstdc++-v3/testsuite/30_threads/promise/60966.cc b/libstdc++-v3/testsuite/30_threads/promise/60966.cc
new file mode 100644 (file)
index 0000000..269268b
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/promise/60966.cc
@@ -0,0 +1,67 @@
+// { dg-do run { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-gnu* *-*-solaris* *-*-cygwin *-*-darwin* powerpc-ibm-aix* } }
+// { dg-options " -std=gnu++11 -pthread" { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-gnu* powerpc-ibm-aix* } }
+// { dg-options " -std=gnu++11 -pthreads" { target *-*-solaris* } }
+// { dg-options " -std=gnu++11 " { target *-*-cygwin *-*-darwin* } }
+// { dg-require-cstdint "" }
+// { dg-require-gthreads "" }
+// { dg-require-atomic-builtins "" }
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// libstdc++/60966
+// This test hangs if std::promise::~promise() destroys the
+// shared state before std::promise::set_value() finishes using it.
+
+#include <future>
+#include <thread>
+#include <vector>
+
+const int THREADS = 10;
+
+void run_task(std::promise<void>* pr)
+{
+  std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  pr->set_value();
+}
+
+int main()
+{
+  std::vector<std::promise<void>*> tasks(THREADS);
+  std::vector<std::thread> threads(THREADS);
+  std::vector<std::future<void>> futures(THREADS);
+
+  for (int i = 0; i < THREADS; ++i)
+  {
+    std::promise<void>* task = new std::promise<void>;
+    tasks[i] = task;
+    futures[i] = task->get_future();
+    threads[i] = std::thread(run_task, task);
+  }
+
+  for (int i = 0; i < THREADS; ++i)
+  {
+    // the temporary future releases the state as soon as wait() returns
+    std::future<void>(std::move(futures[i])).wait();
+    // state is ready, should now be safe to delete promise, so it
+    // releases the shared state too
+    delete tasks[i];
+  }
+
+  for (auto& t : threads)
+    t.join();
+}