[libvirt] [PATCH 1/14] Adding recursive locks

[Eric Blake]

On 03/18/2010 11:04 AM, Eric Blake wrote:
> But what does a true non-recursive mutex buy you?  The only difference
> between recursive and true non-recursive is whether you declare that an
> attempt to relock a mutex that you already own is a fatal deadlock
> error, rather than incrementing a counter for matching unlocks.  It's
> just that non-recursive mutexes typically have faster implementations.

Actually, it DOES buy something.  virCondWait DEPENDS on getting a true
non-recursive function (PTHREAD_MUTEX_NORMAL or
PTHREAD_MUTEX_ERRORCHECK, although the latter has better guaranteed
behavior in the case of deadlock), because POSIX is clear that:


    It is advised that an application should not use a
PTHREAD_MUTEX_RECURSIVE mutex with condition variables because the
implicit unlock performed for a pthread_cond_timedwait() or
pthread_cond_wait() may not actually release the mutex (if it had been
locked multiple times). If this happens, no other thread can satisfy the
condition of the predicate.

> 
> For that matter, do we even need the distinction?  Maybe ALL our code
> should be using recursive mutexes by default, by changing virMutexInit
> to be recursive no matter what, and not worry about introducing
> virMutexInitRecursive.  Looking more closely at virMutexInit in the
> pthreads version, we use pthread_mutex_init(,NULL), which requests
> PTHREAD_MUTEX_DEFAULT.

> That is, our current implementation of virMutexInit is NOT a true
> non-recursive mutex, so much as a mutex that is unspecified whether it
> is recursive or not.
>

And that means we have a bug in threads-pthread.c - we should be
explicitly requesting a pthread_mutexattr with PTHREAD_MUTEX_ERRORCHECK
rather than relying on NULL.

On the other hand, the win32 virCondWait is coded to correctly deal with
the fact that Windows mutexes are always recursive (that is, windows
events do not suffer from the pthread deadlock where a cond_wait on a
recursively-held mutex fails to release the mutex).  So, in a way, while
the pthread virMutexInit must be non-recursive, I don't see any problem
with the win32 being recursive.  But we do need the distinction between
virMutexInit and virMutexInitRecursive for pthreads, even though the
win32 implementation can be the same for both.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 323 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100318/7f8f9117/attachment-0001.sig>