Patchwork [v3] proposed fix for libstdc++/49204 causes abi_check failure

login
register
mail settings
Submitter Jonathan Wakely
Date Feb. 1, 2012, 12:23 a.m.
Message ID <CAH6eHdShWw7QW92yOrvmSc7+3yBfB6FOpPHe1c6DZTp_p7Axxg@mail.gmail.com>
Download mbox | patch
Permalink /patch/138886/
State New
Headers show

Comments

Jonathan Wakely - Feb. 1, 2012, 12:23 a.m.
A bit later than I originally planned, but this is now committed, as attached.

2012-02-01  Jonathan Wakely  <jwakely.gcc@gmail.com>

        PR libstdc++/49204
        * include/std/future (__future_base::_State_base::wait()): Use lambda
        expression for predicate and remove redundant test.
        (__future_base::_State_base::wait_for()): Return future_status and
        use lambda expression for predicate.
        (__future_base::_State_base::wait_until()): Likewise.
        (__basic_future::wait_for(), __basic_future::wait_until()): Likewise.
        (__future_base::_Async_state): Replace with _Async_state_common
        class for non-dependent functionality and _Async_state_impl class
        template for dependent functionality.
        (__future_base::_Async_state_common::_M_join): Serialize attempts to
        join thread.
        (__future_base::_Async_state_common::_M_run_deferred): Join.
        (__future_base::_Async_state::_M_do_run): Replace with lambda.
        * src/c++11/future.cc (__future_base::_Async_state_common): Define
        destructor, so key function is in the library.
        * config/abi/pre/gnu.ver: Add exports for ~_Async_state_common.
        * testsuite/30_threads/packaged_task/members/get_future.cc: Expect
        future_status return instead of bool.
        * testsuite/30_threads/shared_future/members/wait_until.cc: Likewise.
        * testsuite/30_threads/shared_future/members/wait_for.cc: Likewise.
        * testsuite/30_threads/future/members/wait_until.cc: Likewise.
        * testsuite/30_threads/future/members/wait_for.cc: Likewise.
        * testsuite/30_threads/promise/members/set_value2.cc: Likewise.
        * testsuite/30_threads/promise/members/set_value3.cc: Likewise.
        * testsuite/30_threads/promise/members/swap.cc: Likewise.

Tested x86_64-linux.

Patch

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index ce56120..41d38a7 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1307,6 +1307,14 @@  GLIBCXX_3.4.17 {
     # std::wstring::pop_back()
     _ZNSbIwSt11char_traitsIwESaIwEE8pop_backEv;
 
+    # std::_Async_state_common::~_Async_state_common
+    _ZTINSt13__future_base19_Async_state_commonE;
+    _ZTSNSt13__future_base19_Async_state_commonE;
+    _ZTVNSt13__future_base19_Async_state_commonE;
+    _ZNSt13__future_base19_Async_state_commonD0Ev;
+    _ZNSt13__future_base19_Async_state_commonD1Ev;
+    _ZNSt13__future_base19_Async_state_commonD2Ev;
+
 } GLIBCXX_3.4.16;
 
 # Symbols in the support library (libsupc++) have their own tag.
diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index d3180e9..1093e3f 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -1,6 +1,6 @@ 
 // <future> -*- C++ -*-
 
-// Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011, 2012 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
@@ -328,27 +328,28 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
 	_M_run_deferred();
 	unique_lock<mutex> __lock(_M_mutex);
-	if (!_M_ready())
-	  _M_cond.wait(__lock, std::bind<bool>(&_State_base::_M_ready, this));
+	_M_cond.wait(__lock, [&] { return _M_ready(); });
 	return *_M_result;
       }
 
       template<typename _Rep, typename _Period>
-        bool
+        future_status
         wait_for(const chrono::duration<_Rep, _Period>& __rel)
         {
 	  unique_lock<mutex> __lock(_M_mutex);
-	  auto __bound = std::bind<bool>(&_State_base::_M_ready, this);
-	  return _M_ready() || _M_cond.wait_for(__lock, __rel, __bound);
+	  if (_M_cond.wait_for(__lock, __rel, [&] { return _M_ready(); }))
+	    return future_status::ready;
+	  return future_status::timeout;
 	}
 
       template<typename _Clock, typename _Duration>
-        bool
+        future_status
         wait_until(const chrono::time_point<_Clock, _Duration>& __abs)
         {
 	  unique_lock<mutex> __lock(_M_mutex);
-	  auto __bound = std::bind<bool>(&_State_base::_M_ready, this);
-	  return _M_ready() || _M_cond.wait_until(__lock, __abs, __bound);
+	  if (_M_cond.wait_until(__lock, __abs, [&] { return _M_ready(); }))
+	    return future_status::ready;
+	  return future_status::timeout;
 	}
 
       void
@@ -480,14 +481,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       bool _M_ready() const noexcept { return static_cast<bool>(_M_result); }
 
+      // Misnamed: waits for completion of async function.
       virtual void _M_run_deferred() { }
     };
 
     template<typename _BoundFn, typename = typename _BoundFn::result_type>
       class _Deferred_state;
 
+    class _Async_state_common;
+
     template<typename _BoundFn, typename = typename _BoundFn::result_type>
-      class _Async_state;
+      class _Async_state_impl;
 
     template<typename _Signature>
       class _Task_state;
@@ -573,7 +577,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
 
       template<typename _Rep, typename _Period>
-        bool
+        future_status
         wait_for(const chrono::duration<_Rep, _Period>& __rel) const
         {
           _State_base::_S_check(_M_state);
@@ -581,7 +585,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
         }
 
       template<typename _Clock, typename _Duration>
-        bool
+        future_status
         wait_until(const chrono::time_point<_Clock, _Duration>& __abs) const
         {
           _State_base::_S_check(_M_state);
@@ -1418,29 +1422,39 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
     };
 
+  class __future_base::_Async_state_common : public __future_base::_State_base
+  {
+  protected:
+    ~_Async_state_common();
+
+    // Allow non-timed waiting functions to block until the thread completes,
+    // as if joined.
+    virtual void _M_run_deferred() { _M_join(); }
+
+    void _M_join() { std::call_once(_M_once, &thread::join, ref(_M_thread)); }
+
+    thread _M_thread;
+    once_flag _M_once;
+  };
+
   template<typename _BoundFn, typename _Res>
-    class __future_base::_Async_state final
-    : public __future_base::_State_base
+    class __future_base::_Async_state_impl final
+    : public __future_base::_Async_state_common
     {
     public:
       explicit
-      _Async_state(_BoundFn&& __fn)
-      : _M_result(new _Result<_Res>()), _M_fn(std::move(__fn)),
-	_M_thread(mem_fn(&_Async_state::_M_do_run), this)
-      { }
-
-      ~_Async_state() { _M_thread.join(); }
-
-    private:
-      void _M_do_run()
+      _Async_state_impl(_BoundFn&& __fn)
+      : _M_result(new _Result<_Res>()), _M_fn(std::move(__fn))
       {
-        _M_set_result(_S_task_setter(_M_result, _M_fn));
+	_M_thread = std::thread{ [this] {
+	  _M_set_result(_S_task_setter(_M_result, _M_fn));
+        } };
       }
 
+    private:
       typedef __future_base::_Ptr<_Result<_Res>> _Ptr_type;
       _Ptr_type _M_result;
       _BoundFn _M_fn;
-      thread _M_thread;
     };
 
   template<typename _BoundFn>
@@ -1457,7 +1471,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     __future_base::_S_make_async_state(_BoundFn&& __fn)
     {
       typedef typename remove_reference<_BoundFn>::type __fn_type;
-      typedef _Async_state<__fn_type> __state_type;
+      typedef _Async_state_impl<__fn_type> __state_type;
       return std::make_shared<__state_type>(std::move(__fn));
     }
 
diff --git a/libstdc++-v3/src/c++11/future.cc b/libstdc++-v3/src/c++11/future.cc
index e68642c3..dab0774 100644
--- a/libstdc++-v3/src/c++11/future.cc
+++ b/libstdc++-v3/src/c++11/future.cc
@@ -1,6 +1,6 @@ 
 // future -*- C++ -*-
 
-// Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011, 2012 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
@@ -84,6 +84,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __future_base::_Result_base::~_Result_base() = default;
 
   __future_base::_State_base::~_State_base() = default;
+
+  __future_base::_Async_state_common::~_Async_state_common() { _M_join(); }
+
+  // Explicit instantiation due to -fno-implicit-instantiation.
+  template void call_once(once_flag&, void (thread::*&&)(), reference_wrapper<thread>&&);
 #endif
 
 _GLIBCXX_END_NAMESPACE_VERSION
diff --git a/libstdc++-v3/testsuite/30_threads/future/members/wait_for.cc b/libstdc++-v3/testsuite/30_threads/future/members/wait_for.cc
index 737b604..476bfd9 100644
--- a/libstdc++-v3/testsuite/30_threads/future/members/wait_for.cc
+++ b/libstdc++-v3/testsuite/30_threads/future/members/wait_for.cc
@@ -37,12 +37,12 @@  void test01()
 
   std::chrono::milliseconds delay(100);
 
-  VERIFY( !f1.wait_for(delay) );
+  VERIFY( f1.wait_for(delay) == std::future_status::timeout );
 
   p1.set_value(1);
 
   auto before = std::chrono::system_clock::now();
-  VERIFY( f1.wait_for(delay) );
+  VERIFY( f1.wait_for(delay) == std::future_status::ready );
   VERIFY( std::chrono::system_clock::now() < (before + delay) );
 }
 
diff --git a/libstdc++-v3/testsuite/30_threads/future/members/wait_until.cc b/libstdc++-v3/testsuite/30_threads/future/members/wait_until.cc
index aa7dff6..c588be8 100644
--- a/libstdc++-v3/testsuite/30_threads/future/members/wait_until.cc
+++ b/libstdc++-v3/testsuite/30_threads/future/members/wait_until.cc
@@ -41,13 +41,13 @@  void test01()
   std::future<int> f1(p1.get_future());
 
   auto when = make_time(10);
-  VERIFY( !f1.wait_until(when) );
+  VERIFY( f1.wait_until(when) == std::future_status::timeout );
   VERIFY( std::chrono::system_clock::now() >= when );
 
   p1.set_value(1);
 
   when = make_time(100);
-  VERIFY( f1.wait_until(when) );
+  VERIFY( f1.wait_until(when) == std::future_status::ready );
   VERIFY( std::chrono::system_clock::now() < when );
 }
 
diff --git a/libstdc++-v3/testsuite/30_threads/packaged_task/members/get_future.cc b/libstdc++-v3/testsuite/30_threads/packaged_task/members/get_future.cc
index 0eca8ab..79d4c1f 100644
--- a/libstdc++-v3/testsuite/30_threads/packaged_task/members/get_future.cc
+++ b/libstdc++-v3/testsuite/30_threads/packaged_task/members/get_future.cc
@@ -36,8 +36,9 @@  void test01()
   std::packaged_task<int&(int&)> p1(inc);
   std::future<int&> f1 = p1.get_future();
 
+  std::chrono::milliseconds delay(1);
   VERIFY( f1.valid() );
-  VERIFY( !f1.wait_for(std::chrono::milliseconds(1)) );
+  VERIFY( f1.wait_for(delay) == std::future_status::timeout );
 
   int i1 = 0;
 
diff --git a/libstdc++-v3/testsuite/30_threads/promise/members/set_value2.cc b/libstdc++-v3/testsuite/30_threads/promise/members/set_value2.cc
index 0841d66..7dbf079 100644
--- a/libstdc++-v3/testsuite/30_threads/promise/members/set_value2.cc
+++ b/libstdc++-v3/testsuite/30_threads/promise/members/set_value2.cc
@@ -48,7 +48,8 @@  void test01()
     test = true;
   }
 
-  VERIFY( f1.wait_for(std::chrono::milliseconds(1)) );
+  std::chrono::milliseconds delay(1);
+  VERIFY( f1.wait_for(delay) == std::future_status::ready );
   VERIFY( f1.get() == 1 );
   VERIFY( test );
 }
@@ -74,7 +75,8 @@  void test02()
     test = true;
   }
 
-  VERIFY( f1.wait_for(std::chrono::milliseconds(1)) );
+  std::chrono::milliseconds delay(1);
+  VERIFY( f1.wait_for(delay) == std::future_status::ready );
   VERIFY( f1.get() == 3 );
   VERIFY( test );
 }
diff --git a/libstdc++-v3/testsuite/30_threads/promise/members/set_value3.cc b/libstdc++-v3/testsuite/30_threads/promise/members/set_value3.cc
index 63f0836..268d162 100644
--- a/libstdc++-v3/testsuite/30_threads/promise/members/set_value3.cc
+++ b/libstdc++-v3/testsuite/30_threads/promise/members/set_value3.cc
@@ -41,24 +41,26 @@  struct tester
 std::promise<tester> pglobal;
 std::future<tester> fglobal = pglobal.get_future();
 
+auto delay = std::chrono::milliseconds(1);
+
 tester::tester(int)
 {
   bool test __attribute__((unused)) = true;
-  VERIFY (!fglobal.wait_for(std::chrono::milliseconds(1)));
+  VERIFY (fglobal.wait_for(delay) == std::future_status::timeout);
 }
 
 tester::tester(const tester&)
 {
   bool test __attribute__((unused)) = true;
   // if this copy happens while a mutex is locked next line could deadlock:
-  VERIFY (!fglobal.wait_for(std::chrono::milliseconds(1)));
+  VERIFY (fglobal.wait_for(delay) == std::future_status::timeout);
 }
 
 tester& tester::operator=(const tester&)
 {
   bool test __attribute__((unused)) = true;
   // if this copy happens while a mutex is locked next line could deadlock:
-  VERIFY (!fglobal.wait_for(std::chrono::milliseconds(1)));
+  VERIFY (fglobal.wait_for(delay) == std::future_status::timeout);
   return *this;
 }
 
@@ -68,7 +70,7 @@  void test01()
 
   pglobal.set_value( tester(1) );
 
-  VERIFY( fglobal.wait_for(std::chrono::milliseconds(1)) );
+  VERIFY (fglobal.wait_for(delay) == std::future_status::ready);
 }
 
 int main()
diff --git a/libstdc++-v3/testsuite/30_threads/promise/members/swap.cc b/libstdc++-v3/testsuite/30_threads/promise/members/swap.cc
index 39e11c6..93a0026 100644
--- a/libstdc++-v3/testsuite/30_threads/promise/members/swap.cc
+++ b/libstdc++-v3/testsuite/30_threads/promise/members/swap.cc
@@ -35,8 +35,9 @@  void test01()
   std::promise<int> p2;
   p1.set_value(1);
   p1.swap(p2);
-  VERIFY( !p1.get_future().wait_for(std::chrono::milliseconds(1)) );
-  VERIFY( p2.get_future().wait_for(std::chrono::milliseconds(1)) );
+  auto delay = std::chrono::milliseconds(1);
+  VERIFY( p1.get_future().wait_for(delay) == std::future_status::timeout );
+  VERIFY( p2.get_future().wait_for(delay) == std::future_status::ready );
 }
 
 int main()
diff --git a/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_for.cc b/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_for.cc
index 3b57af5..2c4ec1d 100644
--- a/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_for.cc
+++ b/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_for.cc
@@ -38,14 +38,14 @@  void test01()
 
   std::chrono::milliseconds delay(100);
 
-  VERIFY( !f1.wait_for(delay) );
-  VERIFY( !f2.wait_for(delay) );
+  VERIFY( f1.wait_for(delay) == std::future_status::timeout );
+  VERIFY( f2.wait_for(delay) == std::future_status::timeout );
 
   p1.set_value(1);
 
   auto before = std::chrono::system_clock::now();
-  VERIFY( f1.wait_for(delay) );
-  VERIFY( f2.wait_for(delay) );
+  VERIFY( f1.wait_for(delay) == std::future_status::ready );
+  VERIFY( f2.wait_for(delay) == std::future_status::ready );
   VERIFY( std::chrono::system_clock::now() < (before + 2*delay) );
 }
 
diff --git a/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_until.cc b/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_until.cc
index e626f63..c6fcb93 100644
--- a/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_until.cc
+++ b/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_until.cc
@@ -42,18 +42,18 @@  void test01()
   std::shared_future<int> f2(f1);
 
   auto when = make_time(10);
-  VERIFY( !f1.wait_until(make_time(10)) );
+  VERIFY( f1.wait_until(make_time(10)) == std::future_status::timeout );
   VERIFY( std::chrono::system_clock::now() >= when );
 
   when = make_time(10);
-  VERIFY( !f2.wait_until(make_time(10)) );
+  VERIFY( f2.wait_until(make_time(10)) == std::future_status::timeout );
   VERIFY( std::chrono::system_clock::now() >= when );
 
   p1.set_value(1);
 
   when = make_time(100);
-  VERIFY( f1.wait_until(when) );
-  VERIFY( f2.wait_until(when) );
+  VERIFY( f1.wait_until(when) == std::future_status::ready );
+  VERIFY( f2.wait_until(when) == std::future_status::ready );
   VERIFY( std::chrono::system_clock::now() < when );
 }