Re: [boost] [smart_ptr] [thread] (was Re: A summary of testing with sanitizers)

Subject: Re: [boost] [smart_ptr] [thread] (was Re: A summary of testing with sanitizers)
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2015-04-25 09:52:50

Le 24/04/15 21:17, Ben Pope a écrit :
> On Tuesday, April 21, 2015 06:42 PM, Ben Pope wrote:
>>       89 SUMMARY: ThreadSanitizer: data race
>> /home/ben/development/boost/test/build/boost_root/status/../boost/smart_ptr/detail/shared_count.hpp:449:49 
>>
>> in boost::detail::shared_count::shared_count(boost::detail::shared_count
>> const&)
>
> I've just been looking at this one, essentially it seems to boil down to:
>
> #include <atomic>
> #include <thread>
> #include <cstdint>
>
> typedef _Atomic( std::int_least32_t ) atomic_int_least32_t;
>
> inline void atomic_increment( atomic_int_least32_t * pw )
> {
>     __c11_atomic_fetch_add( pw, 1, __ATOMIC_RELAXED );
> }
>
> int main()
> {
>    atomic_int_least32_t use_count_;
>    std::thread t ([&](){ atomic_increment(&use_count_); });
>    __c11_atomic_init( &use_count_, 1 );
>    t.join();
> }
>

The Boost.Thread code is different

             static void* thread_proxy(void* param)
             {
                 boost::detail::thread_data_ptr thread_info = 
static_cast<boost::detail::thread_data_base*>(param)->self; // (2)

     bool thread::start_thread_noexcept()
     {
         thread_info->self=thread_info; // (1)
         int const res = pthread_create(&thread_info->thread_handle, 0, 
&thread_proxy, thread_info.get());
         if (res != 0)
         {
             thread_info->self.reset(); // (3)
             return false;
         }

The thread context thread_info stores a smart_ptr to itself in member 
self (1).
if the thread creation succeeds, self is used (2)
Only if the thread creation fails, the self member is reset (3).

I don't see where the data race is.

I have changed (2) by

                 boost::detail::thread_data_ptr thread_info = 
static_cast<boost::detail::thread_data_base*>(param)->shared_from_this();
                 thread_info->self.reset();

Lets see what ThreadSanitizer reports after this change.

https://github.com/boostorg/thread/commit/0218136ed7df26b66ec64cec1f7195b83e3f90db

Best,
Vicente