From 40b3dc0c2c3d781ac07433064ffc466fa96bbe2b Mon Sep 17 00:00:00 2001 From: Gaurav Date: Tue, 3 Nov 2015 14:28:37 +0530 Subject: [PATCH 01/13] Remove duplicate conditional check. thread_info already checked against Null at line no 86, no need to check again. --- src/pthread/thread.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/pthread/thread.cpp b/src/pthread/thread.cpp index 4743d2db..e9d7a45c 100644 --- a/src/pthread/thread.cpp +++ b/src/pthread/thread.cpp @@ -110,10 +110,7 @@ namespace boost thread_info->tss_data.erase(current); } } - if (thread_info) // fixme: should we test this? - { - thread_info->self.reset(); - } + thread_info->self.reset(); } } } From eb6d819218587966334644f9500ddaf2f69fd9b4 Mon Sep 17 00:00:00 2001 From: "Vicente J. Botet Escriba" Date: Sun, 8 Nov 2015 10:58:36 +0100 Subject: [PATCH 02/13] remove unused typedef. --- include/boost/thread/future.hpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/boost/thread/future.hpp b/include/boost/thread/future.hpp index 3729de68..9bde1637 100644 --- a/include/boost/thread/future.hpp +++ b/include/boost/thread/future.hpp @@ -3664,9 +3664,6 @@ namespace detail typename decay::type... )>::type> async(launch policy, BOOST_THREAD_FWD_REF(F) f, BOOST_THREAD_FWD_REF(ArgTypes)... args) { - typedef typename boost::result_of::type( - typename decay::type... - )>::type R; typedef detail::invoker::type, typename decay::type...> BF; typedef typename BF::result_type Rp; From c0fe04ecc99f3ab71eaed6bcb3c5ffa77fbaa7ef Mon Sep 17 00:00:00 2001 From: "Vicente J. Botet Escriba" Date: Sun, 8 Nov 2015 11:04:40 +0100 Subject: [PATCH 03/13] ensure the void* in the cleanup is shared during the call. --- src/pthread/thread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pthread/thread.cpp b/src/pthread/thread.cpp index e9d7a45c..969e58cc 100644 --- a/src/pthread/thread.cpp +++ b/src/pthread/thread.cpp @@ -80,8 +80,8 @@ namespace boost { static void tls_destructor(void* data) { - boost::detail::thread_data_base* thread_info=static_cast(data); - //boost::detail::thread_data_ptr thread_info = static_cast(data)->shared_from_this(); + //boost::detail::thread_data_base* thread_info=static_cast(data); + boost::detail::thread_data_ptr thread_info = static_cast(data)->shared_from_this(); if(thread_info) { From 3f7f34b634c128a7b362cbcb98e047455a84e0c4 Mon Sep 17 00:00:00 2001 From: "Vicente J. Botet Escriba" Date: Sun, 8 Nov 2015 17:30:29 +0100 Subject: [PATCH 04/13] minor cleanup on condition_variable. --- .../thread/pthread/condition_variable.hpp | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/include/boost/thread/pthread/condition_variable.hpp b/include/boost/thread/pthread/condition_variable.hpp index 2aa6cd6b..a6033515 100644 --- a/include/boost/thread/pthread/condition_variable.hpp +++ b/include/boost/thread/pthread/condition_variable.hpp @@ -68,17 +68,14 @@ namespace boost #if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS thread_cv_detail::lock_on_exit > guard; detail::interruption_checker check_for_interruption(&internal_mutex,&cond); + pthread_mutex_t* the_mutex = &internal_mutex; guard.activate(m); - do { - res = pthread_cond_wait(&cond,&internal_mutex); - } while (res == EINTR); #else - //boost::pthread::pthread_mutex_scoped_lock check_for_interruption(&internal_mutex); pthread_mutex_t* the_mutex = m.mutex()->native_handle(); +#endif do { res = pthread_cond_wait(&cond,the_mutex); } while (res == EINTR); -#endif } #if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS this_thread::interruption_point(); @@ -99,18 +96,17 @@ namespace boost boost::throw_exception(condition_error(EPERM, "boost::condition_variable::do_wait_until() failed precondition mutex not owned")); } #endif - thread_cv_detail::lock_on_exit > guard; int cond_res; { #if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS + thread_cv_detail::lock_on_exit > guard; detail::interruption_checker check_for_interruption(&internal_mutex,&cond); + pthread_mutex_t* the_mutex = &internal_mutex; guard.activate(m); - cond_res=pthread_cond_timedwait(&cond,&internal_mutex,&timeout); #else - //boost::pthread::pthread_mutex_scoped_lock check_for_interruption(&internal_mutex); pthread_mutex_t* the_mutex = m.mutex()->native_handle(); - cond_res=pthread_cond_timedwait(&cond,the_mutex,&timeout); #endif + cond_res=pthread_cond_timedwait(&cond,the_mutex,&timeout); } #if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS this_thread::interruption_point(); @@ -178,7 +174,7 @@ namespace boost #if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS detail::interruption_checker check_for_interruption(&internal_mutex,&cond); #else - boost::pthread::pthread_mutex_scoped_lock check_for_interruption(&internal_mutex); + boost::pthread::pthread_mutex_scoped_lock check_for_interruption(&internal_mutex); #endif guard.activate(m); res=pthread_cond_wait(&cond,&internal_mutex); @@ -405,7 +401,7 @@ namespace boost #if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS detail::interruption_checker check_for_interruption(&internal_mutex,&cond); #else - boost::pthread::pthread_mutex_scoped_lock check_for_interruption(&internal_mutex); + boost::pthread::pthread_mutex_scoped_lock check_for_interruption(&internal_mutex); #endif guard.activate(m); res=pthread_cond_timedwait(&cond,&internal_mutex,&timeout); @@ -423,8 +419,6 @@ namespace boost } return true; } - - }; } From 7b67789f98e13ea82afea32b520bf40f65b817e1 Mon Sep 17 00:00:00 2001 From: "Vicente J. Botet Escriba" Date: Mon, 9 Nov 2015 23:58:15 +0100 Subject: [PATCH 05/13] Avoid ambiguity with C++17 std::invoke. --- include/boost/thread/detail/invoker.hpp | 24 ++++++++++++------------ include/boost/thread/detail/thread.hpp | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/include/boost/thread/detail/invoker.hpp b/include/boost/thread/detail/invoker.hpp index 60dc2c6c..9f38e979 100644 --- a/include/boost/thread/detail/invoker.hpp +++ b/include/boost/thread/detail/invoker.hpp @@ -99,7 +99,7 @@ namespace boost result_type execute(tuple_indices) { - return invoke(boost::move(csbl::get<0>(f_)), boost::move(csbl::get(f_))...); + return detail::invoke(boost::move(csbl::get<0>(f_)), boost::move(csbl::get(f_))...); } }; @@ -136,7 +136,7 @@ namespace boost result_type execute(tuple_indices) { - return invoke(boost::move(csbl::get<0>(f_)), boost::move(csbl::get(f_))...); + return detail::invoke(boost::move(csbl::get<0>(f_)), boost::move(csbl::get(f_))...); } }; //BOOST_THREAD_DCL_MOVABLE_BEG(X) invoker BOOST_THREAD_DCL_MOVABLE_END @@ -190,7 +190,7 @@ namespace boost {} \ \ result_type operator()() { \ - return invoke(boost::move(fp_) \ + return detail::invoke(boost::move(fp_) \ BOOST_PP_REPEAT(n, BOOST_THREAD_MOVE_DCL, ~) \ ); \ } \ @@ -315,7 +315,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) , boost::move(v2_) @@ -381,7 +381,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) , boost::move(v2_) @@ -442,7 +442,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) , boost::move(v2_) @@ -498,7 +498,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) , boost::move(v2_) @@ -549,7 +549,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) , boost::move(v2_) @@ -595,7 +595,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) , boost::move(v2_) @@ -636,7 +636,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) , boost::move(v2_) @@ -672,7 +672,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) , boost::move(v1_) ); @@ -703,7 +703,7 @@ namespace boost result_type operator()() { - return invoke(boost::move(fp_) + return detail::invoke(boost::move(fp_) , boost::move(v0_) ); } diff --git a/include/boost/thread/detail/thread.hpp b/include/boost/thread/detail/thread.hpp index 2e97e5cb..3c702db0 100644 --- a/include/boost/thread/detail/thread.hpp +++ b/include/boost/thread/detail/thread.hpp @@ -72,7 +72,7 @@ namespace boost void run2(tuple_indices) { - invoke(std::move(std::get<0>(fp)), std::move(std::get(fp))...); + detail::invoke(std::move(std::get<0>(fp)), std::move(std::get(fp))...); } void run() { From 3edbf67ef00da10dce82a5e6176f58d0ca0e211c Mon Sep 17 00:00:00 2001 From: "Vicente J. Botet Escriba" Date: Tue, 10 Nov 2015 07:40:31 +0100 Subject: [PATCH 06/13] add unwrap/wait/get test. --- example/future_unwrap.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/example/future_unwrap.cpp b/example/future_unwrap.cpp index 017ffaf0..2d455b95 100644 --- a/example/future_unwrap.cpp +++ b/example/future_unwrap.cpp @@ -40,12 +40,22 @@ int main() BOOST_THREAD_LOG << " > outer_future = boost::async(boost::launch::async, &p2); + boost::future inner_future = outer_future.unwrap(); + inner_future.wait(); + int ii = inner_future.get(); + BOOST_THREAD_LOG << "ii= "<< ii << "" << BOOST_THREAD_END_LOG; + } { boost::future > outer_future = boost::async(boost::launch::async, &p2); boost::future inner_future = outer_future.unwrap(); int ii = inner_future.get(); BOOST_THREAD_LOG << "ii= "<< ii << "" << BOOST_THREAD_END_LOG; } + } catch (std::exception& ex) { std::cout << "ERRORRRRR "< Date: Wed, 11 Nov 2015 00:29:15 +0100 Subject: [PATCH 07/13] add implicit unwrap test. --- example/future_unwrap.cpp | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/example/future_unwrap.cpp b/example/future_unwrap.cpp index 2d455b95..4f252805 100644 --- a/example/future_unwrap.cpp +++ b/example/future_unwrap.cpp @@ -42,19 +42,27 @@ int main() try { - { - boost::future > outer_future = boost::async(boost::launch::async, &p2); - boost::future inner_future = outer_future.unwrap(); - inner_future.wait(); - int ii = inner_future.get(); - BOOST_THREAD_LOG << "ii= "<< ii << "" << BOOST_THREAD_END_LOG; - } - { - boost::future > outer_future = boost::async(boost::launch::async, &p2); - boost::future inner_future = outer_future.unwrap(); - int ii = inner_future.get(); - BOOST_THREAD_LOG << "ii= "<< ii << "" << BOOST_THREAD_END_LOG; - } +#ifndef BOOST_NO_CXX11_RVALUE_REFERENCES + { + boost::future inner_future = boost::async(boost::launch::async, &p2); + inner_future.wait(); + int ii = inner_future.get(); + BOOST_THREAD_LOG << "ii= "<< ii << "" << BOOST_THREAD_END_LOG; + } +#endif + { + boost::future > outer_future = boost::async(boost::launch::async, &p2); + boost::future inner_future = outer_future.unwrap(); + inner_future.wait(); + int ii = inner_future.get(); + BOOST_THREAD_LOG << "ii= "<< ii << "" << BOOST_THREAD_END_LOG; + } + { + boost::future > outer_future = boost::async(boost::launch::async, &p2); + boost::future inner_future = outer_future.unwrap(); + int ii = inner_future.get(); + BOOST_THREAD_LOG << "ii= "<< ii << "" << BOOST_THREAD_END_LOG; + } } catch (std::exception& ex) { From aa608685af96a446885f42a0a18fb65b78cfb5ea Mon Sep 17 00:00:00 2001 From: "Vicente J. Botet Escriba" Date: Wed, 11 Nov 2015 00:31:59 +0100 Subject: [PATCH 08/13] rename centinel by sentinel. reset it as soon as the non deferred continuation is launched so that resources are released. --- include/boost/thread/future.hpp | 14 ++++++++++---- test/Jamfile.v2 | 3 ++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/include/boost/thread/future.hpp b/include/boost/thread/future.hpp index 9bde1637..431ceebc 100644 --- a/include/boost/thread/future.hpp +++ b/include/boost/thread/future.hpp @@ -4226,13 +4226,13 @@ namespace detail { F parent; Fp continuation; - shared_ptr centinel; + shared_ptr sentinel; public: continuation_shared_state(BOOST_THREAD_RV_REF(F) f, BOOST_THREAD_FWD_REF(Fp) c) : parent(boost::move(f)), continuation(boost::move(c)), - centinel(parent.future_) + sentinel(parent.future_) { } @@ -4243,6 +4243,7 @@ namespace detail void call() { try { + sentinel.reset(); this->mark_finished_with_result(this->continuation(boost::move(this->parent))); } catch(...) { this->mark_exceptional_finish(); @@ -4263,13 +4264,13 @@ namespace detail { F parent; Fp continuation; - shared_ptr centinel; + shared_ptr sentinel; public: continuation_shared_state(BOOST_THREAD_RV_REF(F) f, BOOST_THREAD_FWD_REF(Fp) c) : parent(boost::move(f)), continuation(boost::move(c)), - centinel(parent.future_) + sentinel(parent.future_) { } @@ -4281,6 +4282,7 @@ namespace detail void call() { try { + sentinel.reset(); this->continuation(boost::move(this->parent)); this->mark_finished_with_result(); } catch(...) { @@ -4491,6 +4493,7 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + //this->sentinel.reset(); } }; @@ -4527,6 +4530,7 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + //this->sentinel.reset(); } }; @@ -4564,6 +4568,7 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + //this->sentinel.reset(); } }; @@ -4597,6 +4602,7 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + //this->sentinel.reset(); } }; diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 1650dcfb..7edae976 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -793,7 +793,7 @@ rule thread-compile ( sources : reqs * : name ) #[ thread-run ../example/vhh_shared_mutex.cpp ] [ thread-run2 ../example/make_future.cpp : ex_make_future ] [ thread-run2 ../example/future_then.cpp : ex_future_then ] - [ thread-run2 ../example/future_fallback_to.cpp : ex_future_fallback_to ] + #[ thread-run2 ../example/future_fallback_to.cpp : ex_future_fallback_to ] [ thread-run2 ../example/future_unwrap.cpp : ex_future_unwrap ] [ thread-run2-noit ../example/synchronized_value.cpp : ex_synchronized_value ] [ thread-run2-noit ../example/synchronized_person.cpp : ex_synchronized_person ] @@ -963,6 +963,7 @@ rule thread-compile ( sources : reqs * : name ) #[ thread-run test_11256.cpp ] #[ thread-run test_11499.cpp ] #[ thread-run test_11611.cpp ] + [ thread-run2 ../example/future_fallback_to.cpp : ex_future_fallback_to2 ] ; From 4ba8415b08b04f30b918a2dd2f01c6c8f251e0f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BChler?= Date: Fri, 6 Nov 2015 13:27:12 +0100 Subject: [PATCH 09/13] drop sentinel in continuation/unwrap states - instead keep the inner state alive in places where we move ourself after we locked the inner state --- include/boost/thread/future.hpp | 50 ++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/include/boost/thread/future.hpp b/include/boost/thread/future.hpp index 431ceebc..db2c9978 100644 --- a/include/boost/thread/future.hpp +++ b/include/boost/thread/future.hpp @@ -4226,13 +4226,11 @@ namespace detail { F parent; Fp continuation; - shared_ptr sentinel; public: continuation_shared_state(BOOST_THREAD_RV_REF(F) f, BOOST_THREAD_FWD_REF(Fp) c) : parent(boost::move(f)), - continuation(boost::move(c)), - sentinel(parent.future_) + continuation(boost::move(c)) { } @@ -4243,7 +4241,6 @@ namespace detail void call() { try { - sentinel.reset(); this->mark_finished_with_result(this->continuation(boost::move(this->parent))); } catch(...) { this->mark_exceptional_finish(); @@ -4264,13 +4261,11 @@ namespace detail { F parent; Fp continuation; - shared_ptr sentinel; public: continuation_shared_state(BOOST_THREAD_RV_REF(F) f, BOOST_THREAD_FWD_REF(Fp) c) : parent(boost::move(f)), - continuation(boost::move(c)), - sentinel(parent.future_) + continuation(boost::move(c)) { } @@ -4282,7 +4277,6 @@ namespace detail void call() { try { - sentinel.reset(); this->continuation(boost::move(this->parent)); this->mark_finished_with_result(); } catch(...) { @@ -4493,7 +4487,6 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } - //this->sentinel.reset(); } }; @@ -4530,7 +4523,6 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } - //this->sentinel.reset(); } }; @@ -4568,7 +4560,6 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } - //this->sentinel.reset(); } }; @@ -4602,7 +4593,6 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } - //this->sentinel.reset(); } }; @@ -4716,7 +4706,10 @@ namespace detail { typedef typename boost::result_of)>::type future_type; BOOST_THREAD_ASSERT_PRECONDITION(this->future_!=0, future_uninitialized()); - boost::unique_lock lock(this->future_->mutex); + // keep state alive as we move ourself but hold the lock + shared_ptr sentinel(this->future_); + boost::unique_lock lock(sentinel->mutex); + if (underlying_cast(policy) & int(launch::async)) { return BOOST_THREAD_MAKE_RV_REF((boost::detail::make_future_async_continuation_shared_state, future_type>( lock, boost::move(*this), boost::forward(func) @@ -4779,7 +4772,10 @@ namespace detail { typedef typename boost::result_of)>::type future_type; BOOST_THREAD_ASSERT_PRECONDITION(this->future_!=0, future_uninitialized()); - boost::unique_lock lock(this->future_->mutex); + // keep state alive as we move ourself but hold the lock + shared_ptr sentinel(this->future_); + boost::unique_lock lock(sentinel->mutex); + return BOOST_THREAD_MAKE_RV_REF((boost::detail::make_future_executor_continuation_shared_state, future_type>(ex, lock, boost::move(*this), boost::forward(func) ))); @@ -4800,7 +4796,10 @@ namespace detail { typedef typename boost::result_of)>::type future_type; BOOST_THREAD_ASSERT_PRECONDITION(this->future_!=0, future_uninitialized()); - boost::unique_lock lock(this->future_->mutex); + // keep state alive as we move ourself but hold the lock + shared_ptr sentinel(this->future_); + boost::unique_lock lock(sentinel->mutex); + launch policy = this->launch_policy(lock); if (underlying_cast(policy) & int(launch::deferred)) { this->future_->wait_internal(lock); @@ -4828,7 +4827,10 @@ namespace detail { typedef typename boost::result_of)>::type future_type; BOOST_THREAD_ASSERT_PRECONDITION(this->future_!=0, future_uninitialized()); - boost::unique_lock lock(this->future_->mutex); + // keep state alive as we move ourself but hold the lock + shared_ptr sentinel(this->future_); + boost::unique_lock lock(sentinel->mutex); + if (underlying_cast(policy) & int(launch::async)) { return BOOST_THREAD_MAKE_RV_REF((boost::detail::make_future_async_continuation_shared_state, future_type>( lock, boost::move(*this), boost::forward(func) @@ -4891,7 +4893,10 @@ namespace detail { typedef typename boost::result_of)>::type future_type; BOOST_THREAD_ASSERT_PRECONDITION(this->future_!=0, future_uninitialized()); - boost::unique_lock lock(this->future_->mutex); + // keep state alive as we move ourself but hold the lock + shared_ptr sentinel(this->future_); + boost::unique_lock lock(sentinel->mutex); + return BOOST_THREAD_MAKE_RV_REF((boost::detail::make_future_executor_continuation_shared_state, future_type>(ex, lock, boost::move(*this), boost::forward(func) ))); @@ -4914,7 +4919,10 @@ namespace detail { typedef typename boost::result_of)>::type future_type; BOOST_THREAD_ASSERT_PRECONDITION(this->future_!=0, future_uninitialized()); - boost::unique_lock lock(this->future_->mutex); + // keep state alive as we move ourself but hold the lock + shared_ptr sentinel(this->future_); + boost::unique_lock lock(sentinel->mutex); + launch policy = this->launch_policy(lock); if (underlying_cast(policy) & int(launch::deferred)) { @@ -5197,7 +5205,11 @@ namespace detail BOOST_THREAD_FUTURE >::unwrap() { BOOST_THREAD_ASSERT_PRECONDITION(this->future_!=0, future_uninitialized()); - boost::unique_lock lock(this->future_->mutex); + + // keep state alive as we move ourself but hold the lock + shared_ptr sentinel(this->future_); + boost::unique_lock lock(sentinel->mutex); + return boost::detail::make_future_unwrap_shared_state >, R2>(lock, boost::move(*this)); } #endif From f36857ffefe24eb126ffc036f4f7fd8454d630de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BChler?= Date: Fri, 13 Nov 2015 12:02:34 +0100 Subject: [PATCH 10/13] explicitly clear continuation parent state --- include/boost/thread/future.hpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/boost/thread/future.hpp b/include/boost/thread/future.hpp index db2c9978..411a6cb5 100644 --- a/include/boost/thread/future.hpp +++ b/include/boost/thread/future.hpp @@ -4245,6 +4245,8 @@ namespace detail } catch(...) { this->mark_exceptional_finish(); } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); } static void run(shared_ptr that_) @@ -4282,6 +4284,8 @@ namespace detail } catch(...) { this->mark_exceptional_finish(); } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); } static void run(shared_ptr that_) @@ -4487,6 +4491,8 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); } }; @@ -4523,6 +4529,8 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); } }; @@ -4560,6 +4568,8 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); } }; @@ -4593,6 +4603,8 @@ namespace detail { } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); } }; From feab8add3f29fcc8cbc31f601db40c86d6c05a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BChler?= Date: Fri, 13 Nov 2015 12:00:41 +0100 Subject: [PATCH 11/13] refactor deferred execute calls --- include/boost/thread/future.hpp | 137 ++++++++------------------------ 1 file changed, 32 insertions(+), 105 deletions(-) diff --git a/include/boost/thread/future.hpp b/include/boost/thread/future.hpp index 411a6cb5..5f2db856 100644 --- a/include/boost/thread/future.hpp +++ b/include/boost/thread/future.hpp @@ -4249,6 +4249,21 @@ namespace detail this->parent = F(); } + void call(boost::unique_lock& lck) { + try { + Fp local_fuct = boost::move(this->continuation); + F local_parent = boost::move(this->parent); + relocker relock(lck); + Rp res = local_fuct(boost::move(local_parent)); + relock.lock(); + this->mark_finished_with_result_internal(boost::move(res), lck); + } catch (...) { + this->mark_exceptional_finish_internal(current_exception(), lck); + } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); + } + static void run(shared_ptr that_) { continuation_shared_state* that = static_cast(that_.get()); @@ -4288,6 +4303,21 @@ namespace detail this->parent = F(); } + void call(boost::unique_lock& lck) { + try { + Fp local_fuct = boost::move(this->continuation); + F local_parent = boost::move(this->parent); + relocker relock(lck); + local_fuct(boost::move(local_parent)); + relock.lock(); + this->mark_finished_with_result_internal(lck); + } catch (...) { + this->mark_exceptional_finish_internal(current_exception(), lck); + } + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); + } + static void run(shared_ptr that_) { continuation_shared_state* that = static_cast(that_.get()); @@ -4476,62 +4506,9 @@ namespace detail { boost::unique_lock lk(this->mutex); if (this->is_deferred_) { this->is_deferred_=false; - this->execute(lk); + this->call(lk); } } - - virtual void execute(boost::unique_lock& lck) { - try { - Fp local_fuct=boost::move(this->continuation); - F ftmp = boost::move(this->parent); - relocker relock(lck); - Rp res = local_fuct(boost::move(ftmp)); - relock.lock(); - this->mark_finished_with_result_internal(boost::move(res), lck); - } catch (...) { - this->mark_exceptional_finish_internal(current_exception(), lck); - } - // make sure parent is really cleared to prevent memory "leaks" - this->parent = F(); - } - }; - - template - struct future_deferred_continuation_shared_state: continuation_shared_state - { - typedef continuation_shared_state base_type; - - public: - future_deferred_continuation_shared_state(BOOST_THREAD_RV_REF(F) f, BOOST_THREAD_FWD_REF(Fp) c) - : base_type(boost::move(f), boost::forward(c)) - { - this->set_deferred(); - } - - ~future_deferred_continuation_shared_state() { - } - virtual void launch_continuation() { - boost::unique_lock lk(this->mutex); - if (this->is_deferred_) { - this->is_deferred_=false; - this->execute(lk); - } - } - - virtual void execute(boost::unique_lock& lck) { - try { - Fp local_fuct=boost::move(this->continuation); - F ftmp = boost::move(this->parent); - relocker relock(lck); - local_fuct(boost::move(ftmp)); - relock.lock(); - this->mark_finished_with_result_internal(lck); - } catch (...) { - this->mark_exceptional_finish_internal(current_exception(), lck); - } - // make sure parent is really cleared to prevent memory "leaks" - this->parent = F(); - } }; ////////////////////////// @@ -4553,59 +4530,9 @@ namespace detail { boost::unique_lock lk(this->mutex); if (this->is_deferred_) { this->is_deferred_=false; - this->execute(lk); + this->call(lk); } } - - virtual void execute(boost::unique_lock& lck) { - try { - Fp local_fuct=boost::move(this->continuation); - F ftmp = this->parent; - relocker relock(lck); - Rp res = local_fuct(ftmp); - relock.lock(); - this->mark_finished_with_result_internal(boost::move(res), lck); - } catch (...) { - this->mark_exceptional_finish_internal(current_exception(), lck); - } - // make sure parent is really cleared to prevent memory "leaks" - this->parent = F(); - } - }; - - template - struct shared_future_deferred_continuation_shared_state: continuation_shared_state - { - typedef continuation_shared_state base_type; - public: - shared_future_deferred_continuation_shared_state(F f, BOOST_THREAD_FWD_REF(Fp) c) - : base_type(boost::move(f), boost::forward(c)) - { - this->set_deferred(); - } - - virtual void launch_continuation() { - boost::unique_lock lk(this->mutex); - if (this->is_deferred_) { - this->is_deferred_=false; - this->execute(lk); - } - } - - virtual void execute(boost::unique_lock& lck) { - try { - Fp local_fuct=boost::move(this->continuation); - F ftmp = this->parent; - relocker relock(lck); - local_fuct(ftmp); - relock.lock(); - this->mark_finished_with_result_internal(lck); - } catch (...) { - this->mark_exceptional_finish_internal(current_exception(), lck); - } - // make sure parent is really cleared to prevent memory "leaks" - this->parent = F(); - } }; //////////////////////////////// From 8cba434c59d97b80d94faf4ac12a1058adf9ce1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BChler?= Date: Fri, 13 Nov 2015 12:15:39 +0100 Subject: [PATCH 12/13] neither continuation nor parent need mutex protection --- include/boost/thread/future.hpp | 38 ++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/include/boost/thread/future.hpp b/include/boost/thread/future.hpp index 5f2db856..1f9fd57b 100644 --- a/include/boost/thread/future.hpp +++ b/include/boost/thread/future.hpp @@ -4251,17 +4251,25 @@ namespace detail void call(boost::unique_lock& lck) { try { - Fp local_fuct = boost::move(this->continuation); - F local_parent = boost::move(this->parent); relocker relock(lck); - Rp res = local_fuct(boost::move(local_parent)); + + // neither continuation nor parent are protected by the lock - call() must only + // be called once, and no one else must modify it. + Rp res = this->continuation(boost::move(this->parent)); + + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); + relock.lock(); + this->mark_finished_with_result_internal(boost::move(res), lck); } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); + + // make sure parent is really cleared to prevent memory "leaks" + relocker relock(lck); + this->parent = F(); } - // make sure parent is really cleared to prevent memory "leaks" - this->parent = F(); } static void run(shared_ptr that_) @@ -4305,17 +4313,23 @@ namespace detail void call(boost::unique_lock& lck) { try { - Fp local_fuct = boost::move(this->continuation); - F local_parent = boost::move(this->parent); - relocker relock(lck); - local_fuct(boost::move(local_parent)); - relock.lock(); + { + relocker relock(lck); + // neither continuation nor parent are protected by the lock - call() must only + // be called once, and no one else must modify it. + this->continuation(boost::move(this->parent)); + + // make sure parent is really cleared to prevent memory "leaks" + this->parent = F(); + } this->mark_finished_with_result_internal(lck); } catch (...) { this->mark_exceptional_finish_internal(current_exception(), lck); + + // make sure parent is really cleared to prevent memory "leaks" + relocker relock(lck); + this->parent = F(); } - // make sure parent is really cleared to prevent memory "leaks" - this->parent = F(); } static void run(shared_ptr that_) From dbf0160b1e86a695915ff173b585a538cb4c8188 Mon Sep 17 00:00:00 2001 From: "Vicente J. Botet Escriba" Date: Sat, 14 Nov 2015 12:38:37 +0100 Subject: [PATCH 13/13] uncomment previous failing fallback_to test. --- test/Jamfile.v2 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 7edae976..1650dcfb 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -793,7 +793,7 @@ rule thread-compile ( sources : reqs * : name ) #[ thread-run ../example/vhh_shared_mutex.cpp ] [ thread-run2 ../example/make_future.cpp : ex_make_future ] [ thread-run2 ../example/future_then.cpp : ex_future_then ] - #[ thread-run2 ../example/future_fallback_to.cpp : ex_future_fallback_to ] + [ thread-run2 ../example/future_fallback_to.cpp : ex_future_fallback_to ] [ thread-run2 ../example/future_unwrap.cpp : ex_future_unwrap ] [ thread-run2-noit ../example/synchronized_value.cpp : ex_synchronized_value ] [ thread-run2-noit ../example/synchronized_person.cpp : ex_synchronized_person ] @@ -963,7 +963,6 @@ rule thread-compile ( sources : reqs * : name ) #[ thread-run test_11256.cpp ] #[ thread-run test_11499.cpp ] #[ thread-run test_11611.cpp ] - [ thread-run2 ../example/future_fallback_to.cpp : ex_future_fallback_to2 ] ;