[Patch] Clang: allow scoped_lockable to acquire multiple locks

[Howard Hinnant]

On Aug 31, 2014, at 4:00 PM, Richard Smith <richard at metafoo.co.uk> wrote:

>> On Sat, Aug 30, 2014 at 9:53 PM, Ed Schouten <ed at 80386.nl> wrote:
>> Hi DeLesley,
>> 
>> First of all, thanks for your work on the thread annotations for
>> Clang! Impressed by how well they work, I decided to patch up
>> FreeBSD's pthread library to use these annotations by default.
>> 
>> http://lists.freebsd.org/pipermail/freebsd-toolchain/2014-August/001183.html
>> 
>> While experimenting with using these annotations in a C++ codebase, I
>> discovered that it's currently not possible to use these for
>> interlocking, as scoped_lockable does not support acquiring multiple
>> mutexes. Example:
>> 
>> class SCOPED_LOCKABLE DoubleGuard {
>>   DoubleGuard(Mutex* m1, Mutex* m2) EXCLUSIVE_LOCK_FUNCTION(m1, m2) {
>>     if (m1 == m2) {
>>       m1_ = m1;
>>       m2_ = nullptr;
>>       m1_->Lock();
>>     } else {
>>       if (m1 < m2) {
>>         m1_ = m1;
>>         m2_ = m2;
>>       } else {
>>         m1_ = m2;
>>         m2_ = m1;
>>       }
>>       m1_->Lock();
>>       m2_->Lock();
>>     }
>>   }
>> 
>>   ~DoubleGuard() UNLOCK_FUNCTION() {
>>     m1_->Unlock();
>>     if (m2_ != nullptr)
>>       m2_->Unlock();
>>   }
>> 
>>  private:
>>   Mutex* m1_;
>>   Mutex* m2_;
>> };
>> 
>> This class cannot be used, as it will cause a warning that the guard
>> object itself is locked twice. Attached is a patch to fix this. It
>> changes the lock path to always create exactly one FactEntry for
>> scoped lockables. Instead of having a single UnderlyingMutex, it now
>> uses a CapExprSet.
>> 
>> What are your thoughts on this change? Is it all right for me to commit this?
>> 
> Does this handle the variadic template case:
> 
> class SCOPED_LOCKABLE MultiGuard {
> public:
>   template<typename ...T> MultiGuard(T *...mutexes) EXCLUSIVE_LOCK_FUNCTION(mutexes...)
>     : m{mutexes...} {
>     std::sort(m.begin(), m.end());
>     for (Mutex *mut : m) mut->Lock();
>   }
>   ~MutexGuard() UNLOCK_FUNCTION() {
>     std::reverse(m.begin(), m.end());
>     for (Mutex *mut : m) : mut->Unlock();
>   }
> private:
>   std::vector<Mutex*> m;
> };

On locking multiple locks at once:

http://htmlpreview.github.io/?https://github.com/HowardHinnant/papers/blob/master/dining_philosophers.html

One should probably use std::lock for this, and if std::lock doesn’t have sufficiently high performance for you, you should submit a bug report.  The linked paper above is a fairly in-depth study of the performance of various algorithms.  The ordered solution (what is proposed above) does not have impressive performance.

Howard