Re: [Boost-users] mutexes : Is this thread-safe ?

From: Tom Widmer (tom_usenet_at_[hidden])
Date: 2006-02-20 10:30:58

Kevin Heifner wrote:
> Tom Widmer wrote:
> 
>>Dhanvi Kapila wrote:
>>
>>>class BoostMutexLock
>>>{
>>>   public :
>>>        void lock() {
>>>               lk = new boost::mutex::scoped_lock(global_mutex);
>>>       }
>>>        void unlock() {
>>>              delete lk;
>>>        }
>>>  BoostMutexLock() : lk( 0 ) { }
>>>  ~BoostMutexLock() { }
>>>
>>>private:
>>>      boost::mutex global_mutex;
>>>      boost::mutex::scoped_lock* lk;
>>>};
>>>------------------------------------------------------
> 
> 
> [snip]
> 
> 
>>It will be threadsafe though, yes, since the scoped lock constructor 
>>will have to lock global_mutex before assigning to lk. If two threads on 
>>two CPUs run lock at the same time, they will create separate, 
>>independent scoped_lock objects, and one will get the lock and unlock it 
>>before the other one gets to assign lk, so there's no problem. It's 
>>inefficient though due to the memory allocation - why do you want it?
> 
> 
> I don't think it is threadsafe on every platform.  It it my 
> understanding that the following line:
>    lk = new boost::mutex::scoped_lock(global_mutex);
> can be split by the compiler to be:
>    lk = malloc(sizeof(boost::mutex::scoped_lock)); // alloc and 
> assign
>    // another thread could come in here, causing
>    // leak of the first malloc and a later double
>    // delete
>    lk->boost::mutex::scoped_lock(global_mutex); // call constructor

That particular re-ordering is illegal. Remember, when a mutex is 
locked, no memory access or code that happens after the mutex is locked 
(in this case the assignment to lk) can be moved to before the mutex was 
locked (in other words, a mutex emits an appropriate memory barrier), so 
lk will not be assigned until after global_mutex is locked.

For an example of why the reordering isn't allowed in the C++ abstract 
machine in the single threaded case, have a look at this:

struct A;
A* a = 0;
struct A
{
   A()
   {
     assert(a == 0);
   }
};

int main()
{
   a = new A; //must not assert
   delete a;
}

This is different from DCL, where one of the paths does *not* lock the 
mutex - in the OP's code, all paths lock the mutex before accessing 
shared data, so there's no problem.

Tom