From patchwork Wed Feb 1 00:23:14 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 138886 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id E7825B6EF1 for ; Wed, 1 Feb 2012 11:23:51 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1328660633; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=u/G3lHitCEbtxA/e+ldPj3+ZpGc=; b=YroEwcKba9Yb2Hf Wv6C509Xq5Kb7/k065/XMDhaONp91AK6t1KpkEbCdrJ+B3yNW5Z/BdAm24eEuUV2 jz5bbw1iMs2Cqxh9FGX2GuBt/suZ2zmcH/3IiGJWTQgkWioiblhrx90P3G+tFAuj LJroCEz6KT6sHJOjI+9i9zrQjoKc= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=PhTpt0qkboCmW79qyObHyLbKFq7z6POMmL9v1HCskFfzVoG++3i8CqVSIca3LK WC4UW6vRtCjdqIqEexzmqn+MnUCIvx403flXDi0wzBtrCkiLrIKqL0fw6jS9UPvZ m0R9UPQwAbhBkt02Iu1RoS6hLlwXBb1d8Ypj+nJep3liQ=; Received: (qmail 15085 invoked by alias); 1 Feb 2012 00:23:35 -0000 Received: (qmail 14964 invoked by uid 22791); 1 Feb 2012 00:23:32 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-lpp01m010-f47.google.com (HELO mail-lpp01m010-f47.google.com) (209.85.215.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 01 Feb 2012 00:23:17 +0000 Received: by lahc1 with SMTP id c1so384213lah.20 for ; Tue, 31 Jan 2012 16:23:15 -0800 (PST) MIME-Version: 1.0 Received: by 10.152.123.10 with SMTP id lw10mr5731913lab.35.1328055794942; Tue, 31 Jan 2012 16:23:14 -0800 (PST) Received: by 10.112.39.42 with HTTP; Tue, 31 Jan 2012 16:23:14 -0800 (PST) In-Reply-To: References: <4F194E99.7030005@oracle.com> Date: Wed, 1 Feb 2012 00:23:14 +0000 Message-ID: Subject: Re: [v3] proposed fix for libstdc++/49204 causes abi_check failure From: Jonathan Wakely To: "libstdc++" , gcc-patches Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org A bit later than I originally planned, but this is now committed, as attached. 2012-02-01 Jonathan Wakely 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. 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 @@ // -*- 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 __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_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_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(_M_result); } + // Misnamed: waits for completion of async function. virtual void _M_run_deferred() { } }; template class _Deferred_state; + class _Async_state_common; + template - class _Async_state; + class _Async_state_impl; template class _Task_state; @@ -573,7 +577,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 +585,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); @@ -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 - 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 @@ -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&&); #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 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 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; 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 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() 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 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 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 ); }