Re: [webkit-dev] Clang Thread Safety Analysis

[Chris Dumez via webkit-dev]

Hi,

Just a quick follow-up to let you know that both CheckedLock/CheckedCondition 
and UncheckedLock/UncheckedCondition have been removed from the tree.
The whole codebase has been ported to Lock/Condition, which have the thread 
safety analysis annotations.

--
 Chris Dumez




> On May 23, 2021, at 10:40 PM, Chris Dumez  wrote:
> 
> Clang Thread Safety Analysis
> WTF::Lock now supports Clang Thread Safety Analysis 
> . It is a C++ language 
> extension which warns about potential race conditions in code, at compile 
> time. It is the same great Lock, but with some extra help from clang to make 
> our multi-threaded code safer by giving out compilation errors when thread 
> unsafe code is detected.
> 
> Annotations
> Just by using WTF::Lock, Clang will already be able to do some basic 
> validation. However, to take full advantage of it, there are a few 
> annotations you should know about. Note that you'll see those annotations 
> throughout the code base already as a few of us have been busy adopting them. 
> All these annotations are declared in wtf/ThreadSafetyAnalysis.h 
> .
> 
> WTF_GUARDED_BY_LOCK()
> 
> WTF_GUARDED_BY_LOCK is an attribute on data members, which declares that the 
> data member is protected by the given lock. Read operations on the data 
> require shared access, while write operations require exclusive access.
> 
> WTF_POINTEE_GUARDED_BY_LOCK is similar, but is intended for use on pointers 
> and smart pointers. There is no constraint on the data member itself, but the 
> data that it points to is protected by the given lock.
> 
> class Foo {
> Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
> Lock m_stringsLock;
> };
> You will get a compiler error if you try to access or modify m_strings 
> without grabbing the m_stringsLock lock first.
> 
> WTF_REQUIRES_LOCK()
> 
> WTF_REQUIRES_LOCK is an attribute on functions or methods, which declares 
> that the calling thread must have exclusive access to the given locks. More 
> than one lock may be specified. The locks must be held on entry to the 
> function, and must still be held on exit.
> 
> class Foo {
> void addString(String&&) WTF_REQUIRES_LOCK(m_stringsLock);
> 
> Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
> Lock m_stringsLock;
> };
> You will get a compiler error if you try to call addString() without grabbing 
> the m_stringsLock lock first. This also allows to compiler to know that the 
> addString() implementation can modify the m_strings data member without 
> grabbing the lock first.
> 
> Another good use case:
> 
> static Lock globalCacheLock;
> static HashMap& globalCache() 
> WTF_REQUIRES_LOCK(globalCacheLock)
> {
> static NeverDestroyed> globalCache;
> return globalCache;
> }
> This will force all call sites to grab the globalCacheLock lock before 
> calling globalCache().
> 
> Previously, we may have passed a LockHolder& as parameter to try and convey 
> that. We have been updating code to stop passing such parameters though as it 
> is no longer useful.
> 
> WTF_ACQUIRES_LOCK() / WTF_RELEASES_LOCK()
> 
> WTF_ACQUIRES_LOCK is an attribute on functions or methods declaring that the 
> function acquires a lock, but does not release it. The given lock must not be 
> held on entry, and will be held on exit.
> 
> WTF_RELEASES_LOCK declares that the function releases the given lock. The 
> lock must be held on entry, and will no longer be held on exit.
> 
> class Foo {
> public:
> void suspend() WTF_ACQUIRES_LOCK(m_suspensionLock)
> {
> m_suspensionLock.lock();
> m_isSuspended = true;
> }
> void resume() WTF_RELEASE_LOCK(m_suspensionLock)
> {
> m_isSuspended = false;
> m_suspensionLock.unlock();
> }
> 
> private:
> Lock m_suspensionLock;
> bool m_isSuspended;
> };
> Without these annotations, you'd get a compiler error stating that you failed 
> to unlock m_suspensionLock before returning in suspend() and that you 
> unlocked m_suspensionLock in resume() even though it was not previously 
> locked.
> 
> WTF_RETURNS_LOCK()
> 
> WTF_RETURNS_LOCK is an attribute on functions or methods, which declares that 
> the function returns a reference to the given lock.
> 
> class Foo {
> public:
> Lock& lock() WTF_RETURNS_LOCK(m_lock) { return m_lock; }
> private:
> Lock m_lock;
> };
> WTF_EXCLUDES_LOCK()
> 
> WTF_EXCLUDES_LOCK is an attribute on functions or methods, which declares 
> that the caller must not hold the given lock. This annotation is used to 
> prevent deadlock. Many mutex implementations are not re-entrant, so deadlock 
> can occur if the function acquires the mutex a second time.
> 
> class Foo {
> void addString(String&& string) WTF_EXCLUDES_LOCK(m_stringsLock)
> {
> Locker locker { m_stringsLock };
> m_string.append(WTFMove(string));
>

[webkit-dev] Clang Thread Safety Analysis

[Chris Dumez via webkit-dev]

Clang Thread Safety Analysis
WTF::Lock now supports Clang Thread Safety Analysis 
. It is a C++ language 
extension which warns about potential race conditions in code, at compile time. 
It is the same great Lock, but with some extra help from clang to make our 
multi-threaded code safer by giving out compilation errors when thread unsafe 
code is detected.

Annotations
Just by using WTF::Lock, Clang will already be able to do some basic 
validation. However, to take full advantage of it, there are a few annotations 
you should know about. Note that you'll see those annotations throughout the 
code base already as a few of us have been busy adopting them. All these 
annotations are declared in wtf/ThreadSafetyAnalysis.h 
.

WTF_GUARDED_BY_LOCK()

WTF_GUARDED_BY_LOCK is an attribute on data members, which declares that the 
data member is protected by the given lock. Read operations on the data require 
shared access, while write operations require exclusive access.

WTF_POINTEE_GUARDED_BY_LOCK is similar, but is intended for use on pointers and 
smart pointers. There is no constraint on the data member itself, but the data 
that it points to is protected by the given lock.

class Foo {
Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
Lock m_stringsLock;
};
You will get a compiler error if you try to access or modify m_strings without 
grabbing the m_stringsLock lock first.

WTF_REQUIRES_LOCK()

WTF_REQUIRES_LOCK is an attribute on functions or methods, which declares that 
the calling thread must have exclusive access to the given locks. More than one 
lock may be specified. The locks must be held on entry to the function, and 
must still be held on exit.

class Foo {
void addString(String&&) WTF_REQUIRES_LOCK(m_stringsLock);

Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
Lock m_stringsLock;
};
You will get a compiler error if you try to call addString() without grabbing 
the m_stringsLock lock first. This also allows to compiler to know that the 
addString() implementation can modify the m_strings data member without 
grabbing the lock first.

Another good use case:

static Lock globalCacheLock;
static HashMap& globalCache() WTF_REQUIRES_LOCK(globalCacheLock)
{
static NeverDestroyed> globalCache;
return globalCache;
}
This will force all call sites to grab the globalCacheLock lock before calling 
globalCache().

Previously, we may have passed a LockHolder& as parameter to try and convey 
that. We have been updating code to stop passing such parameters though as it 
is no longer useful.

WTF_ACQUIRES_LOCK() / WTF_RELEASES_LOCK()

WTF_ACQUIRES_LOCK is an attribute on functions or methods declaring that the 
function acquires a lock, but does not release it. The given lock must not be 
held on entry, and will be held on exit.

WTF_RELEASES_LOCK declares that the function releases the given lock. The lock 
must be held on entry, and will no longer be held on exit.

class Foo {
public:
void suspend() WTF_ACQUIRES_LOCK(m_suspensionLock)
{
m_suspensionLock.lock();
m_isSuspended = true;
}
void resume() WTF_RELEASE_LOCK(m_suspensionLock)
{
m_isSuspended = false;
m_suspensionLock.unlock();
}

private:
Lock m_suspensionLock;
bool m_isSuspended;
};
Without these annotations, you'd get a compiler error stating that you failed 
to unlock m_suspensionLock before returning in suspend() and that you unlocked 
m_suspensionLock in resume() even though it was not previously locked.

WTF_RETURNS_LOCK()

WTF_RETURNS_LOCK is an attribute on functions or methods, which declares that 
the function returns a reference to the given lock.

class Foo {
public:
Lock& lock() WTF_RETURNS_LOCK(m_lock) { return m_lock; }
private:
Lock m_lock;
};
WTF_EXCLUDES_LOCK()

WTF_EXCLUDES_LOCK is an attribute on functions or methods, which declares that 
the caller must not hold the given lock. This annotation is used to prevent 
deadlock. Many mutex implementations are not re-entrant, so deadlock can occur 
if the function acquires the mutex a second time.

class Foo {
void addString(String&& string) WTF_EXCLUDES_LOCK(m_stringsLock)
{
Locker locker { m_stringsLock };
m_string.append(WTFMove(string));
}
Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
Lock m_stringsLock;
};
WTF_IGNORES_THREAD_SAFETY_ANALYSIS

WTF_IGNORES_THREAD_SAFETY_ANALYSIS is an attribute on functions or methods, 
which turns off thread safety checking for that method. It provides an escape 
hatch for functions which are either (1) deliberately thread-unsafe, or (2) are 
thread-safe, but too complicated for the analysis to understand. Reasons for 
(2) are be described in the Known Limitations 
.

class Foo {
void unsafeI