[cfe-dev] Who is working on/has worked on Capability analysis (-Wthread-safety)?

[Aaron Puchert via cfe-dev]

Am 04.11.21 um 04:40 schrieb Randell Jesup:
> On 10/31/2021 3:36 PM, Aaron Puchert wrote:
>> Thanks for letting me know. (I barely follow the list.)
>>>> On Sat, Oct 30, 2021 at 9:28 PM Randell Jesup via cfe-dev 
>>>> <cfe-dev at lists.llvm.org> wrote:
>>>>> One way to possibly handle the reader/writer vs readers case 
>>>>> (where reads on the writing thread don't need to lock) would be to 
>>>>> be able to say "guarded by this or that", in this case something 
>>>>> like GUARDED_BY(mMutex, MainThread) (GUARDED_BY(mMutex || 
>>>>> MainThread) ??).
>>
>> There has been some work on logical expressions in capability 
>> attributes in 
>> https://reviews.llvm.org/rG7c192b452fa2b3c63ed547e0ff88a5e62765b59f, 
>> but I believe it's not functional yet. I thought about the semantics 
>> of this a bit, but don't have a good understanding yet.
>>
>> If you want accesses to a resource to be protected, it's in general 
>> not sufficient to have one of a set of capabilities. There can only 
>> be one. What happens in this scenario is that there is a certain 
>> period of time where the resource is exclusive to the main thread, 
>> and another where it's only accessible via mutex (even if the main 
>> thread were to access it). So maybe logical expressions aren't the 
>> right way to understand this situation, but rather there should be 
>> different types, one that has GUARDED_BY(MainThread) and another that 
>> has GUARDED_BY(mMutex), and at some point we convert between them. In 
>> some sense the protection regime changes throughout the lifetime, and 
>> for static analysis that means we need a new (static) type.
>
>
> Well, the example I gave doesn't match that, I believe.  This is a 
> value only written on Thread 1, and read on Thread 1 and on other 
> threads. The requirements for locking would be the lock around all 
> writes (on thread 1), and lock around all reads from threads *other* 
> than thread 1.  Reads from Thread 1 don't need to lock, since they're 
> on the only thread that ever writes to the value.   This is 
> conceptually GUARDED_BY(mMutex || OnThread1).
>
> The case you're referencing sounds more like "value is freely written 
> and read on thread 1, then at some point later access to it is 
> provided to other threads, and after that happens access is controlled 
> by a mutex" - where the protection regime changes over time (and may 
> also correlate with specific methods, like an Init() method).
>
You're absolutely right, I was misreading your case.

What you're describing sounds in fact like thread 1 always has a shared 
lock, which it sometimes promotes to exclusive lock, then demotes to a 
shared lock again. The other threads can only acquire shared locks, 
because thread 1 doesn't ever release its shared lock. Now there are 
multiple ways to write that down:

  * Use an actual “multiple readers / single writer” lock, also known as
    read/write lock. Thread 1 never releases it. Then we can always
    read, and we block other threads from ever acquiring a write lock.
    They can still get a read lock though if we don't have the write lock.
  * If that's not desirable, for example because multiple readers aren't
    needed, or a read/write lock would be overkill, build a “fake
    read/write lock” that looks like a read/write lock to the thread
    safety analysis, but is actually implemented via mutex. It would
    have different entry points for the different threads:
      o LockMain acquires the mutex and is annotated RELEASE_SHARED()
        ACQUIRE(), perhaps REQUIRES(main).
      o UnlockMain releases the mutex and is annotated RELEASE()
        ACQUIRE_SHARED(), perhaps REQUIRES(main).
      o LockOther acquires the mutex and is annotated ACQUIRE_SHARED().
      o UnlockOther releases the mutex and is annotated RELEASE_SHARED().

There might be more options, but you get the idea. Conceptually you 
don't have multiple capabilities, you have just one. The key is which 
modes (shared/exclusive) they are held in, and who can acquire that 
capability in which mode.

The problem with booleans is that they mess with exclusivity: mMutex || 
OnThread1 seems to imply that either of them is fine. But the other 
threads can't get write access even if they have the mutex. In fact the 
mutex gives different threads different levels of access. That's why I 
think you actually have a (possibly specialized) read/write lock here. 
And note that the second option should be a zero-cost abstraction around 
a mutex.

> We have another pattern that's interesting, where a method may use 
> Maybe<MutexAutoLock> lock(std::in_place, mMutex); (Maybe<> is 
> effectively std::optional).   This allows us to lock.reset() instead 
> of using MutexAutoUnlock lock(mLock), which will lead to an extra 
> lock/unlock pair compared to Maybe<> and lock.reset().
>
Supporting that would be hard, because we'd need to look into the Maybe 
type, and then the analysis isn't local anymore. However, the analysis 
supports “premature unlocking” of scopes, i.e. you can write 
lock.Unlock(). I've even added support for “relockable scopes”, where 
you can lock.Lock() again. (Have a look at the MutexLocker in 
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutex-h to see 
what is possible.)

The philosophy that we follow with scope types is that they should work 
as local variables whose lifetime is determined by a scope. Anything 
else is “out of scope”, which includes types like optional that invoke 
the destructor manually. Also the scopes (at least for now) need to be 
bound to a fixed set of mutexes for their entire lifetime.

> We also have an RAII MutexAutoTryLock class, which is like 
> MutexAutoLock but does a trylock operation, and also you can convert 
> it to bool to see if it obtained the lock.   I don't think this is 
> easily representable in the current system.  (I'll note that we use it 
> 4 times in our codebase, and 3 of those are in Mutext test code, so I 
> don't care that much ;-) .)
>
Same problem. We support a scope with deferred locking and then 
try-acquiring that. To stay in the language of mutex.h:

MutexLocker lock(&mu, defer_lock);
if (lock.TryLock())
     ...

If the try-acquire is hidden in the constructor, and we obtain the value 
only later, that seems hard to support. (For starters, we'd need 
additional attributes because TRY_ACQUIRE wants a return value.)

If you're worried about scope, you can with C++17 write

if (MutexLocker lock(&mu, defer_lock); lock.TryLock())
     ...

which I guess looks almost like what you can do with MutexAutoTryLock.

Aaron
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20211104/5fdd4d2c/attachment.html>