Re: [boost] [thread] Can Boost.Thread use Boost.Atomic without falling on a compatibility issue?

Subject: Re: [boost] [thread] Can Boost.Thread use Boost.Atomic without falling on a compatibility issue?
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-01-13 17:24:10

Le 13/01/13 21:32, Andrey Semashev a écrit :
> On Mon, Jan 14, 2013 at 12:04 AM, Andrey Semashev
> <andrey.semashev_at_[hidden]> wrote:
>> On Sun, Jan 13, 2013 at 11:44 PM, Andrey Semashev
>> <andrey.semashev_at_[hidden]> wrote:
>>> On Sun, Jan 13, 2013 at 11:15 PM, Vicente J. Botet Escriba
>>> <vicente.botet_at_[hidden]> wrote:
>>>> I believed it also but the mutex shouldn't be locked while doing
>>>> notifications, only when you change the condition and when you wait for the
>>>> condition to be notified.
>>> Looking at the code now it seems there is a potential race condition.
>>> The commit or rollback can set the flag and issue the broadcast just
>>> before the enter method acquires the lock and blocks in the cv.
>>>
>>> I suppose there should be a second check for the flag value under the
>>> lock in the enter_once_region method. The locks in commit and rollback
>>> functions can be removed altogether then.
>> I attached the updated once.cpp.
> No, that code is not correct either. The lock in commit and rollback
> functions *is* needed and it *must* cover both the store and broadcast
> exactly to prevent missed notifications. Otherwise thread A can check
> for the flag value but not yet enter pthread_cond_wait and thread B
> can store the new flag value and issue pthread_cond_broadcast.
>
>
Wouldn't be useful to use double check here

     BOOST_THREAD_DECL bool enter_once_region(once_flag& flag) 
BOOST_NOEXCEPT
     {
       atomic_type& f = reinterpret_cast< atomic_type& >(flag.storage);
       if (f.load(memory_order_acquire) != initialized)
       {
         pthread::pthread_mutex_scoped_lock lk(&once_mutex);
         if (f.load(memory_order_acquire) != initialized)
         {
           while (true)
           {
             atomic_int_type expected = uninitialized;
             if (f.compare_exchange_strong(expected, in_progress, 
memory_order_acq_rel, memory_order_acquire))
             {
               // We have set the flag to in_progress
               return true;
             }
             else if (expected == initialized)
             {
               // Another thread managed to complete the initialization
               return false;
             }
             else
             {
               // Wait until the initialization is complete
               BOOST_VERIFY(!pthread_cond_wait(&once_cv, &once_mutex));
             }
           }
         }
       }
       return false;
     }

     BOOST_THREAD_DECL void commit_once_region(once_flag& flag) 
BOOST_NOEXCEPT
     {
       atomic_type& f = reinterpret_cast< atomic_type& >(flag.storage);
       {
         pthread::pthread_mutex_scoped_lock lk(&once_mutex);
         f.store(initialized, memory_order_release);
       }
       BOOST_VERIFY(!pthread_cond_broadcast(&once_cv));
     }

     BOOST_THREAD_DECL void rollback_once_region(once_flag& flag) 
BOOST_NOEXCEPT
     {
       atomic_type& f = reinterpret_cast< atomic_type& >(flag.storage);
       {
         pthread::pthread_mutex_scoped_lock lk(&once_mutex);
         f.store(uninitialized, memory_order_release);
       }
       BOOST_VERIFY(!pthread_cond_broadcast(&once_cv));
     }

Vicente