diff mbox

libstdc++/60966 fix packaged_task also

Message ID 20150109144050.GR3360@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Jan. 9, 2015, 2:40 p.m. UTC 
The race conditions fixed in PR 60966 can also happen when using
std::packaged_task (on the release branches only, the fix applied to
the trunk covers all cases).

The problem is that the mutex protecting the result in the shared
state is unlocked before the _M_cond.notify_all() call. This leaves a
small window where threads waiting on the future can see the result
(and so return from a waiting function) before the notify_all(). If
the waiting thread destroys the future *and* the
packaged_task as soon as the waiting function returns, then _M_cond
will be destroyed before the notify_all() call on it, resulting in
undefined behaviour (typically this means blocking forever in the
pthread_cond_broadcast() call).

This patch uses the same approach as done for std::promise on the
release branches: increasing the ref-count on the shared state until
the function setting the result has completed. This ensures that the
shared state (and its condition_variable member) will not be destroyed 
until after the _M_cond.notify_all() call.

Thanks to Barry Revzin for finding the problem and Michael Karcher for
debugging it.

The original fixes for 60966 solved the problem for std::promise, this
solves it for std::packaged_task. The only other types of asynchronous
result providers are those created by std::async, but for those types
the waiting functions already block in _M_complete_async() so cannot
return before the notify_all() call happens.

Tested x86_64-linux, committed to the 4.9 and 4.8 branches.

(No new testcase, because the hang only shows up occasionally after
thousands of iterations, or when using valgrind.)
diff mbox

Patch

commit 053f27a4ffbb7cadd780c0b28507aaff98a38824
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jan 9 13:35:42 2015 +0000

    	PR libstdc++/60966
    	* include/std/future (packaged_task::operator()): Increment the
    	reference count on the shared state until the function returns.

diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index d446b9d..6523cea 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -1450,7 +1450,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator()(_ArgTypes... __args)
       {
 	__future_base::_State_base::_S_check(_M_state);
-	_M_state->_M_run(std::forward<_ArgTypes>(__args)...);
+	auto __state = _M_state;
+	__state->_M_run(std::forward<_ArgTypes>(__args)...);
       }
 
       void