Data race in boost/thread/future.hpp

[Alexander Karzhenkov <karzhenkov@…>]

Non-static member function "mark_finished_with_result" of class template "future_async_shared_state" (inherited from "future_async_shared_state_base") is called from static member function "run" concurrently with constructor. Newly created thread uses the object being constructed by original thread. This is data race condition and results in undefined behavior [1.10.21, std-2011]. Moreover, the call of "mark_finished_with_result" may be completed even before constructor execution begins, which also leads to undefined behavior [12.7.1, std-2011].

Program below illustrates the problem:

#define BOOST_THREAD_VERSION 4

#include <iostream>
#include <boost/thread.hpp>

void func(int num)
{
	//boost::this_thread::yield();
	if (num == 0) return;
	std::cout << 'A' << num << std::endl;
	async(boost::launch::async, func, num - 1);
	std::cout << 'B' << num << std::endl;
}

int main()
{
	func(3);
}

Under QNX 6.5.0 SP1 with gcc 4.8.3 it gives the following:

A3
A2
A1
In function notify_all -- d:/boost_1_56_0/boost/thread/pthread/condition_variable.hpp:142 !pthread_cond_broadcast(&cond) -- assertion failed

After uncommenting the call of yield we get what’s expected:

A3
A2
A1
B1
B2
B3

Possible solution for C++11 is to create new thread in the body of "future_async_shared_state" constructor instead of using member initializer syntax for base:

--- future.hpp	Mon Aug  4 00:58:54 2014
+++ future-fixed.hpp	Thu Sep  4 13:00:54 2014
@@ -882,5 +882,5 @@
         public:
-          explicit future_async_shared_state(BOOST_THREAD_FWD_REF(Fp) f) :
-          base_type(thread(&future_async_shared_state::run, this, boost::forward<Fp>(f)))
+          explicit future_async_shared_state(BOOST_THREAD_FWD_REF(Fp) f)
           {
+            thr_ = thread(&future_async_shared_state::run, this, boost::forward<Fp>(f));
           }
@@ -912,5 +912,5 @@
         public:
-          explicit future_async_shared_state(BOOST_THREAD_FWD_REF(Fp) f) :
-          base_type(thread(&future_async_shared_state::run, this, boost::forward<Fp>(f)))
+          explicit future_async_shared_state(BOOST_THREAD_FWD_REF(Fp) f)
           {
+            thr_ = thread(&future_async_shared_state::run, this, boost::forward<Fp>(f));
           }

[viboes]

Thanks for catching this data race condition :)

Have you checked if your patch fixes the issue?

[Alexander Karzhenkov <karzhenkov@…>]

Yes, after applying the patch sample program runs normally with and without "yield". Maybe it's also reasonable to remove constructor of "future_async_shared_state_base" taking "thread" as parameter.

[viboes]

​https://github.com/boostorg/thread/commit/11e4c950acc8f86b7cf404c4df4e3d3d50d4e55e