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

[Daniel P. Berrange]

On Thu, Mar 18, 2010 at 11:16:09AM -0600, Eric Blake wrote:
> 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.

No, we should set  PTHREAD_MUTEX_NORMAL - we don't want it returning an
error code on failure, because all our code assumes pthread_mutex_lock
will not fail. Deadlock is what we want.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|