Bug 49204 - [C++0x] remaining issues in <future>
Summary: [C++0x] remaining issues in <future>
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: 4.9.0
Assignee: Jonathan Wakely
URL:
Keywords:
Depends on: 51617
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-28 00:31 UTC by Jonathan Wakely
Modified: 2013-11-20 21:02 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-05-28 00:31:49


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2011-05-28 00:31:09 UTC
(this is a placeholder and reminder to myself)

We don't return future_status from the timed waiting functions.


The requirements for async say:

 If the implementation chooses the launch::async policy,
   — a call to a waiting function on an asynchronous return object that
     shares the shared state created by this async call shall block until
     the associated thread has completed, as if joined (30.3.1.5);

The current implementation doesn't meet this requirement,
_Async_state::_M_do_run could make the state ready and unblock waiters
before the thread completes.

Proposed fix: _State_base::_M_wait() already calls _M_run_deferred()
to give a deferred function a chance to run, we could add override it
as _Async_state::_M_run_deferred() and join the thread.


@@ -1325,7 +1325,7 @@
   _M_thread(mem_fn(&_Async_state::_M_do_run), this)
      { }

-      ~_Async_state() { _M_thread.join(); }
+      ~_Async_state() { _M_join(); }

    private:
      void _M_do_run()
@@ -1334,11 +1334,18 @@
        _M_set_result(std::move(__setter));
      }

+      void _M_run_deferred()
+      { _M_join(); }
+
+      void _M_join()
+      { std::call_once(_M_once, &thread::join, ref(_M_thread)); }
+
      template<typename, typename> friend class _Task_setter;
      typedef typename __future_base::_Ptr<_Result<_Res>>::type _Ptr_type;
      _Ptr_type _M_result;
      std::function<_Res()> _M_fn;
      thread _M_thread;
+      once_flag _M_once;
    };

  /// async


That would ensure non-timed waiting functions block as if joined.  It
doesn't handle non-timed waiting functions, but I'm not convinced they
should block.
Comment 1 Jonathan Wakely 2011-05-31 10:52:17 UTC
also http://lwg.github.com/issues/lwg-active.html#2056
Comment 2 Jonathan Wakely 2011-05-31 13:40:03 UTC
We should also make std::async check the system load when deciding whether to run asynchronously or deferred.  We should be able to reuse GNU Make's code for deciding whether to run a new job or not, see load_too_high() in job.c
Comment 3 Jonathan Wakely 2011-12-23 16:10:58 UTC
Author: redi
Date: Fri Dec 23 16:10:48 2011
New Revision: 182658

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182658
Log:
	PR libstdc++/49204
	* include/std/future (future_errc): Implement LWG 2056.


Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/future
Comment 4 Jonathan Wakely 2011-12-30 13:10:28 UTC
(In reply to comment #0)
> doesn't handle non-timed waiting functions, but I'm not convinced they
> should block.

Reported as http://cplusplus.github.com/LWG/lwg-active.html#2100
Comment 5 Jonathan Wakely 2012-01-02 17:56:19 UTC
(In reply to comment #2)
> We should also make std::async check the system load when deciding whether to
> run asynchronously or deferred.  We should be able to reuse GNU Make's code for
> deciding whether to run a new job or not, see load_too_high() in job.c

It turns out that calling get_nprocs() (via std::thread::hardware_concurrency()) and getloadavg() from several threads concurrently is a really bad idea, so I'm working on a simple implementation that just compares the number of active async jobs to the number of processors.
Comment 6 Jonathan Wakely 2012-02-01 00:20:27 UTC
Author: redi
Date: Wed Feb  1 00:20:08 2012
New Revision: 183788

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183788
Log:
	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.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/config/abi/pre/gnu.ver
    trunk/libstdc++-v3/include/std/future
    trunk/libstdc++-v3/src/c++11/future.cc
    trunk/libstdc++-v3/testsuite/30_threads/future/members/wait_for.cc
    trunk/libstdc++-v3/testsuite/30_threads/future/members/wait_until.cc
    trunk/libstdc++-v3/testsuite/30_threads/packaged_task/members/get_future.cc
    trunk/libstdc++-v3/testsuite/30_threads/promise/members/set_value2.cc
    trunk/libstdc++-v3/testsuite/30_threads/promise/members/set_value3.cc
    trunk/libstdc++-v3/testsuite/30_threads/promise/members/swap.cc
    trunk/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_for.cc
    trunk/libstdc++-v3/testsuite/30_threads/shared_future/members/wait_until.cc
Comment 7 Jonathan Wakely 2012-02-01 00:25:31 UTC
Still outstanding tasks:

timed waiting functions do nto return future_status::deferred if the shared state contains a deferred function (requires an additional virtual function to be added to the vtable)

check system load when deciding whether to use async or deferred policy
Comment 8 Jonathan Wakely 2012-02-03 09:22:18 UTC
Not planning more work before 4.7.0
Comment 9 Jonathan Wakely 2012-08-22 19:20:19 UTC
(In reply to comment #7)
> timed waiting functions do nto return future_status::deferred if the shared
> state contains a deferred function (requires an additional virtual function to
> be added to the vtable)

setting ABI keyword as this can't be properly fixed without an incompatible change
Comment 10 Jakub Jelinek 2013-03-22 14:41:50 UTC
GCC 4.8.0 is being released, adjusting target milestone.
Comment 11 Jonathan Wakely 2013-03-23 15:01:56 UTC
I didn't get this done for 4.8 and it would be too invasive for 4.8.1 so adjusting milestone
Comment 12 Jonathan Wakely 2013-11-20 20:59:22 UTC
Author: redi
Date: Wed Nov 20 20:59:19 2013
New Revision: 205144

URL: http://gcc.gnu.org/viewcvs?rev=205144&root=gcc&view=rev
Log:
	PR libstdc++/49204
	* include/std/future (__future_base::_State_base): Rename to
	__future_base::_State_baseV2.
	(__future_base::_State_baseV2::~_State_baseV2): Define as defaulted.
	(__future_base::_State_baseV2::_M_run_deferred): Rename to
	_M_complete_async.
	(__future_base::_State_baseV2::_M_has_deferred): Add new virtual.
	(__future_base::_State_baseV2::wait_for): Call _M_has_deferred() to
	test for a deferred function, or call _M_complete_async() to join an
	async thread that has made the shared state ready.
	(__future_base::_State_baseV2::wait_until): Likewise.
	(__future_base::_Async_state_common): Rename to _Async_state_commonV2.
	(__future_base::_Async_state_commonV2::_M_run_deferred): Rename to
	_M_complete_async.
	* src/c++11/compatibility-thread-c++0x.cc (__future_base::_State_base):
	Export old definition.
	(__future_base::_Async_state_common): Likewise.
	* src/c++11/future.cc (__future_base::_State_base::~_State_base):
	Remove.
	* doc/xml/manual/status_cxx2011.xml: Update status.
	* testsuite/30_threads/async/async.cc: Test future_status::timeout
	and future_status::ready.
	* testsuite/30_threads/async/sync.cc: Test future_status::deferred.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
    trunk/libstdc++-v3/include/std/future
    trunk/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
    trunk/libstdc++-v3/src/c++11/future.cc
    trunk/libstdc++-v3/testsuite/30_threads/async/async.cc
    trunk/libstdc++-v3/testsuite/30_threads/async/sync.cc
Comment 13 Jonathan Wakely 2013-11-20 21:02:33 UTC
(In reply to Jonathan Wakely from comment #7)
> Still outstanding tasks:
> 
> timed waiting functions do nto return future_status::deferred if the shared
> state contains a deferred function (requires an additional virtual function
> to be added to the vtable)

This part is fixed for 4.9.0

> check system load when deciding whether to use async or deferred policy

PR 51617 covers that.

We're also missing all the "_at_thread_exit" throughout Clause 30, but I'm going to resolve this PR as fixed.