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

libstdc++-v3/ChangeLog

@@ -1,3 +1,13 @@
+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>
 

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,16 +480,12 @@ _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);
+        _Ptr_type __res = __f(); // do not hold lock while running setter
+	__lock.lock();
         _M_result.swap(__res);
       }
-        _M_cond.notify_all();
-        __set = true;
-      }
 
       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; }
     };
 

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();
+}