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

login
register
mail settings
Submitter Jonathan Wakely
Date Dec. 31, 2011, 5:33 p.m.
Message ID <CAH6eHdQVFG2Xv8rb8WNtQrCHLfJBoc1VMbX_bjqoxVwLc8nUFg@mail.gmail.com>
Download mbox | patch
Permalink /patch/133744/
State New
Headers show

Comments

Jonathan Wakely - Dec. 31, 2011, 5:33 p.m.
I want to commit the attached patch:

        PR libstdc++/49204
        * include/std/future (__future_base::_State_base::wait()): Call
        _M_complete_async instead of _M_run_deferred.
        (__future_base::_State_base::wait_for()): Call _M_has_deferred and
        return future_status.
        (__future_base::_State_base::wait_until()): Likewise.
        (__future_base::_State_base::_M_run_deferred()): Rename to ...
        (__future_base::_State_base::_M_complete_async()): This.
        (__future_base::_State_base::_M_has_deferred()): New.
        (__basic_future::wait_for(), __basic_future::wait_until()): Return
        future_status.
        (__future_base::_Deferred_state::_M_run_deferred()): Rename to ...
        (__future_base::_Deferred_state::_M_complete_async()): This.
        (__future_base::_Async_state::_M_join()): New.
        (__future_base::_Async_state::~_Async_state()): Call _M_join.
        (__future_base::_Async_state::_M_complete_async()): Likewise.
        * 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.
        * testsuite/30_threads/async/sync.cc: Likewise, and test 'deferred'
        status.

But it causes this abi_check error:

1 incompatible symbols
0
_ZTVNSt13__future_base11_State_baseE
vtable for std::__future_base::_State_base
version status: unversioned
type: object
type size: 48
status: <default>

        incompatible sizes
        40
        48

I've renamed _M_run_deferred to _M_complete_async but it has the same
signature, semantics and position in the vtable.  I've also added
_M_has_deferred() at the end of the vtable, for an internal
implementation type that users use directly or refer to. Is that OK
and the testsuite can be adjusted to know about the new vtable size?

Patch

Index: include/std/future
===================================================================
--- include/std/future	(revision 182658)
+++ include/std/future	(working copy)
@@ -326,29 +326,38 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Result_base&
       wait()
       {
-	_M_run_deferred();
+	_M_complete_async();
 	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_ready())
+	    return future_status::ready;
+	  if (_M_has_deferred())
+	    return future_status::deferred;
+	  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_ready())
+	    return future_status::ready;
+	  if (_M_has_deferred())
+	    return future_status::deferred;
+	  if (_M_cond.wait_until(__lock, __abs, [&] { return _M_ready(); }))
+	    return future_status::ready;
+	  return future_status::timeout;
 	}
 
       void
@@ -480,7 +489,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       bool _M_ready() const noexcept { return static_cast<bool>(_M_result); }
 
-      virtual void _M_run_deferred() { }
+      // Wait for completion of async function.
+      virtual void _M_complete_async() { }
+
+      // Return true if state contains a deferred function.
+      virtual bool _M_has_deferred() { return false; }
     };
 
     template<typename _BoundFn, typename = typename _BoundFn::result_type>
@@ -573,7 +586,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 +594,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);
@@ -1411,11 +1424,13 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _BoundFn _M_fn;
 
       virtual void
-      _M_run_deferred()
+      _M_complete_async()
       {
         // safe to call multiple times so ignore failure
         _M_set_result(_S_task_setter(_M_result, _M_fn), true);
       }
+
+      virtual bool _M_has_deferred() { return static_cast<bool>(_M_result); }
     };
 
   template<typename _BoundFn, typename _Res>
@@ -1429,18 +1444,22 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_thread(mem_fn(&_Async_state::_M_do_run), this)
       { }
 
-      ~_Async_state() { _M_thread.join(); }
+      ~_Async_state() { _M_join(); }
 
     private:
-      void _M_do_run()
-      {
-        _M_set_result(_S_task_setter(_M_result, _M_fn));
-      }
+      void _M_do_run() { _M_set_result(_S_task_setter(_M_result, _M_fn)); }
+
+      virtual void _M_complete_async() { _M_join(); }
+
+      void
+      _M_join()
+      { std::call_once(_M_once, &thread::join, ref(_M_thread)); }
 
       typedef __future_base::_Ptr<_Result<_Res>> _Ptr_type;
       _Ptr_type _M_result;
       _BoundFn _M_fn;
       thread _M_thread;
+      once_flag _M_once;
     };
 
   template<typename _BoundFn>
Index: testsuite/30_threads/packaged_task/members/get_future.cc
===================================================================
--- testsuite/30_threads/packaged_task/members/get_future.cc	(revision 182640)
+++ testsuite/30_threads/packaged_task/members/get_future.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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;
 
Index: testsuite/30_threads/shared_future/members/wait_until.cc
===================================================================
--- testsuite/30_threads/shared_future/members/wait_until.cc	(revision 182640)
+++ testsuite/30_threads/shared_future/members/wait_until.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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 );
 }
 
Index: testsuite/30_threads/shared_future/members/wait_for.cc
===================================================================
--- testsuite/30_threads/shared_future/members/wait_for.cc	(revision 182640)
+++ testsuite/30_threads/shared_future/members/wait_for.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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) );
 }
 
Index: testsuite/30_threads/future/members/wait_until.cc
===================================================================
--- testsuite/30_threads/future/members/wait_until.cc	(revision 182640)
+++ testsuite/30_threads/future/members/wait_until.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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 );
 }
 
Index: testsuite/30_threads/future/members/wait_for.cc
===================================================================
--- testsuite/30_threads/future/members/wait_for.cc	(revision 182640)
+++ testsuite/30_threads/future/members/wait_for.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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) );
 }
 
Index: testsuite/30_threads/promise/members/set_value2.cc
===================================================================
--- testsuite/30_threads/promise/members/set_value2.cc	(revision 182640)
+++ testsuite/30_threads/promise/members/set_value2.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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 );
 }
Index: testsuite/30_threads/promise/members/set_value3.cc
===================================================================
--- testsuite/30_threads/promise/members/set_value3.cc	(revision 182640)
+++ testsuite/30_threads/promise/members/set_value3.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009, 2010 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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()
Index: testsuite/30_threads/promise/members/swap.cc
===================================================================
--- testsuite/30_threads/promise/members/swap.cc	(revision 182640)
+++ testsuite/30_threads/promise/members/swap.cc	(working copy)
@@ -6,7 +6,7 @@ 
 // { dg-require-gthreads "" }
 // { dg-require-atomic-builtins "" }
 
-// Copyright (C) 2009 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2011 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
@@ -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()
Index: testsuite/30_threads/async/sync.cc
===================================================================
--- testsuite/30_threads/async/sync.cc	(revision 182640)
+++ testsuite/30_threads/async/sync.cc	(working copy)
@@ -39,12 +39,32 @@  void test01()
   using namespace std;
 
   int a = 1;
-  int b = 10;
-  int c = 100;
+  int b = 1;
+  int c = 1;
   future<int> f1 = async(launch::deferred, sum(), a, ref(b), cref(c));
+  a = 0;
+  b = 10;
+  c = 100;
+
+  const std::chrono::seconds delay(10);
+  const auto then = std::chrono::system_clock::now() + delay;
 
   VERIFY( f1.valid() );
+  // timed waiting functions should return 'deferred' immediately
+  VERIFY( f1.wait_until(then) == std::future_status::deferred );
+  VERIFY( f1.wait_for(delay) == std::future_status::deferred );
+  VERIFY( std::chrono::system_clock::now() < then );
+
+  f1.wait();
+
+  VERIFY( f1.valid() );
+  // timed waiting functions should return 'ready' immediately
+  VERIFY( f1.wait_until(then) == std::future_status::ready );
+  VERIFY( f1.wait_for(delay) == std::future_status::ready );
+  VERIFY( std::chrono::system_clock::now() < then );
+
   VERIFY( f1.get() == 111 );
+  VERIFY( !f1.valid() );
 }
 
 int main()