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

Backport from mainline
2014-05-16  Jonathan Wakely  <jwakely@redhat.com>

	PR libstdc++/60966
	* include/std/future (__future_base::_State_baseV2::_M_set_result):
	Signal condition variable after call_once returns.
	(__future_base::_State_baseV2::_M_do_set): Do not signal here.
	(promise::set_value, promise::set_exception): Increment the reference
	count on the shared state until the function returns.
	* testsuite/30_threads/promise/60966.cc: New.

From-SVN: r211198

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 4736ae49014337a61efb214d9c0c03e624a1fdfe..4c32b06a83150ed905935f3ed0e767a12fa09a2b 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -6,6 +6,17 @@
        PR libstdc++/60734
        * include/bits/stl_tree.h (_Rb_tree::_M_end): Fix invalid cast.
 
+       Backport from mainline
+       2014-05-16  Jonathan Wakely  <jwakely@redhat.com>
+
+       PR libstdc++/60966
+       * include/std/future (__future_base::_State_baseV2::_M_set_result):
+       Signal condition variable after call_once returns.
+       (__future_base::_State_baseV2::_M_do_set): Do not signal here.
+       (promise::set_value, promise::set_exception): Increment the reference
+       count on the shared state until the function returns.
+       * testsuite/30_threads/promise/60966.cc: New.
+
 2014-05-29  Jonathan Wakely  <jwakely@redhat.com>
 
        * include/tr2/bool_set: Use UTF-8 for accented characters.

diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index 00dc978f5c776651d5a8c09313a3508a8c03d14a..507c395eca80d43d74e5a30ec5deff6b0c0bbeb9 100644 (file)
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -351,12 +351,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       void
       _M_set_result(function<_Ptr_type()> __res, bool __ignore_failure = false)
       {
-        bool __set = __ignore_failure;
+        bool __set = false;
         // all calls to this function are serialized,
         // side-effects of invoking __res only happen once
         call_once(_M_once, &_State_base::_M_do_set, this, ref(__res),
             ref(__set));
-        if (!__set)
+       if (__set)
+         _M_cond.notify_all();
+       else if (!__ignore_failure)
           __throw_future_error(int(future_errc::promise_already_satisfied));
       }
 
@@ -471,7 +473,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
           lock_guard<mutex> __lock(_M_mutex);
           _M_result.swap(__res);
         }
-        _M_cond.notify_all();
         __set = true;
       }
 
@@ -983,22 +984,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       void
       set_value(const _Res& __r)
       {
+       auto __future = _M_future;
         auto __setter = _State::__setter(this, __r);
-        _M_future->_M_set_result(std::move(__setter));
+        __future->_M_set_result(std::move(__setter));
       }
 
       void
       set_value(_Res&& __r)
       {
+       auto __future = _M_future;
         auto __setter = _State::__setter(this, std::move(__r));
-        _M_future->_M_set_result(std::move(__setter));
+        __future->_M_set_result(std::move(__setter));
       }
 
       void
       set_exception(exception_ptr __p)
       {
+       auto __future = _M_future;
         auto __setter = _State::__setter(__p, this);
-        _M_future->_M_set_result(std::move(__setter));
+        __future->_M_set_result(std::move(__setter));
       }
     };
 
@@ -1081,15 +1085,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       void
       set_value(_Res& __r)
       {
+       auto __future = _M_future;
         auto __setter = _State::__setter(this, __r);
-        _M_future->_M_set_result(std::move(__setter));
+        __future->_M_set_result(std::move(__setter));
       }
 
       void
       set_exception(exception_ptr __p)
       {
+       auto __future = _M_future;
         auto __setter = _State::__setter(__p, this);
-        _M_future->_M_set_result(std::move(__setter));
+        __future->_M_set_result(std::move(__setter));
       }
     };
 
@@ -1166,8 +1172,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       void
       set_exception(exception_ptr __p)
       {
+       auto __future = _M_future;
         auto __setter = _State::__setter(__p, this);
-        _M_future->_M_set_result(std::move(__setter));
+        __future->_M_set_result(std::move(__setter));
       }
     };
 
@@ -1193,8 +1200,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   inline void
   promise<void>::set_value()
   {
+    auto __future = _M_future;
     auto __setter = _State::__setter(this);
-    _M_future->_M_set_result(std::move(__setter));
+    __future->_M_set_result(std::move(__setter));
   }
 
 

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