From patchwork Sat Dec 31 17:33:23 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [v3] proposed fix for libstdc++/49204 causes abi_check failure Date: Sat, 31 Dec 2011 07:33:23 -0000 From: Jonathan Wakely X-Patchwork-Id: 133744 Message-Id: To: "libstdc++" , gcc-patches 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: 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? 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 __lock(_M_mutex); - if (!_M_ready()) - _M_cond.wait(__lock, std::bind(&_State_base::_M_ready, this)); + _M_cond.wait(__lock, [&] { return _M_ready(); }); return *_M_result; } template - bool + future_status wait_for(const chrono::duration<_Rep, _Period>& __rel) { unique_lock __lock(_M_mutex); - auto __bound = std::bind(&_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 - bool + future_status wait_until(const chrono::time_point<_Clock, _Duration>& __abs) { unique_lock __lock(_M_mutex); - auto __bound = std::bind(&_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(_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 @@ -573,7 +586,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } template - 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 - 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(_M_result); } }; template @@ -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 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 p1(inc); std::future 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 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 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 pglobal; std::future 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 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 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()