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

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.