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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list