Re: [boost] Correct Mutext Destroy Behaviour, Pthreads and Boost

Subject: Re: [boost] Correct Mutext Destroy Behaviour, Pthreads and Boost
From: Robert Bell (skyoyster_at_[hidden])
Date: 2015-06-01 17:46:30

Thank you for everyone's responses, I appreciate it. Yes, the wording is very detailed and perhaps it was not clear in the manner I thought.

The question then comes as to when it “is” safe to destroy a mutex when that mutex is effectively shared across threads.

######

instanceA

Thread 1
refInstanceA1->retain();

// RefCnt = 1
// Spawns Thread 2, hands reference to instanceA but does not retain it. …. (a)
.
.
.
refInstanceA1->release();

------

Thread 2
refInstanceA2->retain(); // Blocks because Thread 1 happens to be releasing…

// Because Thread 1 reduces retain count to 0 before Thread 2 acquires the lock, Thread 1 proceeds to unlock and then tries to destroy.
// Now Thread 2 acquires the lock and does so before the mutex delete call (as you pointed out if adjacency is not the criteria).

######

Unless Thread 1 actively retains the instance prior to handing it to Thread 2 (in (a) above), it’s not going to be safe. That means programmers have to actively remember to transfer ownership before moving to another thread.

The standard does NOT (sorry for the caps) seem to indicate that if the thread which owns the lock calls destroy it’s ok. That would be nice as it would simply cause other threads waiting on it to error out in a nice way.

> On 1 Jun 2015, at 13:45, Gottlob Frege <gottlobfrege_at_[hidden]> wrote:
> 
> On Mon, Jun 1, 2015 at 3:08 PM, Robert Bell <skyoyster_at_[hidden] <mailto:skyoyster_at_[hidden]>> wrote:
>> If an object is implemented to support reference counting, and has an internal raw pthread mutex, the open group is pretty clear in the pthread_mutex_destroy doc on what different implementations must ensure (http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_destroy.html <http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_destroy.html>).
>> 
> 
> I think you are reading the spec wrong.  See below.
> 
>> Note the following statement at the bottom:
>> 
>> "A mutex can be destroyed immediately after it is unlocked. For example, consider the following code:"
>> 
> 
> Note the word "can" not "shall" or "must".  Specs are very specific
> about the use of these words.
> 
>>   obj_done(struct obj *op)
>>   {
>>       pthread_mutex_lock(&op->om);
>>       if (--op->refcnt == 0) {
>>           pthread_mutex_unlock(&op->om);
>>   (A)     pthread_mutex_destroy(&op->om);
>>   (B)     free(op);
>>       } else
>>   (C)     pthread_mutex_unlock(&op->om);
>>   }
>> 
>> In this case obj is reference counted and obj_done() is called whenever a reference to the object is dropped. Implementations are required to allow an object to be destroyed and freed and potentially unmapped (for example, lines A and B) immediately after the object is unlocked (line C)."
>> 
> 
> The point (I think) about the example, is that an implementation of
> pthread_mutex is NOT allowed to postpone work done in
> pthread_mutex_unlock.  ie an implementation can not return
> immediately, and then do some of the work asynchronously, because the
> mutex could be gone by the time the async work was being done.
> 
> So pthread_mutex_destroy cannot be "lazy”.
> 
>> On moving some of my underlying libraries to Boost, I took a look at Boost's Mutex object. The implementation of the Mutex object's destructor call does:
>> 
>> 
>>   ~mutex()
>>           {
>>             int const res = posix::pthread_mutex_destroy(&m);
>>             boost::ignore_unused(res);
>>             BOOST_ASSERT(!res);
>>           }
>> 
>> My question centres on whether Boost's implementation violates the open group's requirement. The calls to posix::pthread_mutex_destroy (Boost's wrapper), usually perform a straight-through call to ::pthread_mutex_destroy, but it occurs inside the destructor, and so does this call occur "immediately after" an unlock by the group's definition?
>> 
> 
> There is no violation.  The call doesn't have to be "immediately
> after", but it can.
> 
>> In straight pthreads, if I want to make sure I can rely on various implementations of the standard, I can simply do exactly what the open group says, unlock, and the next line destroy. But in the case of Boost, is it safe to do something like:
>> 
>>   mutex->unlock();
>>   delete mutex_;
>> 
>> 
>> One thing I did note of interest is that Apple's implementation seems to lock the mutex from within the destroy call, which is curious:
>> 
>>   int
>>   pthread_mutex_destroy(pthread_mutex_t *mutex)
>>   {
>>        int res;
>> 
>>        LOCK(mutex->lock);
>>        if (mutex->sig == _PTHREAD_MUTEX_SIG)
>>        {
>>                if (mutex->owner == (pthread_t)NULL &&
>>                    mutex->busy == (pthread_cond_t *)NULL)
>>                {
>>                        mutex->sig = _PTHREAD_NO_SIG;
>>                        res = ESUCCESS;
>>                }
>>                else
>>                        res = EBUSY;
>>        }
>>        else
>>                res = EINVAL;
>>        UNLOCK(mutex->lock);
>>        return (res);
>>   }
>> 
>> 
>> There seems, generally, to be a good deal of confusion surrounding when it is safe to destroy the resources associated with a mutex, and most people offer widely inaccurate comments based largely on opinion. My question points to whether code adjacency is a requirement of correct behaviour in some implementations, and if not, why not?
>> 
> 
> There is no such thing as code adjacency nor "immediacy".  The next
> line of code could happen an hour from the previous line (if your
> schedule decides that makes sense).
> 
>> If I have missed an obvious explanation somewhere, my apologies. I read through the different patterns of Boost threading listed in the documentation, but was not convinced the question was addressed.
>> 
>> Thanks in advance…
>> 
>> 
>> _______________________________________________
>> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost <http://lists.boost.org/mailman/listinfo.cgi/boost>
> 
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost <http://lists.boost.org/mailman/listinfo.cgi/boost>