
Thread Safety Analysis in Clang - ndesaulniers
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
======
eridius
The title currently reads "Thread Safety Analysis Is Clang". I suspect it was
meant to read "Thread Safety Analysis In Clang".

~~~
chrisseaton
I thought it was a joke about race conditions.

------
SCHiM
Isn't manually locking and unlocking (as seen in the examples) frowned upon
anyway?

In any development I do, and with every developer I've ever developed a
project internally, the use of std::lock_guard<> (which is a scope lock) was
required when working with mutexes. This code looks like that classic "C with
arrays" type of C++ code, that is prone to errors that have long been solved.

As a rule of thumb, when prototyping new multi threaded code and I have a
class that needs to be thread safe, I add a scoped lock in every member
function that is not const. Problem solved, your class is now thread safe,
although not very performant...

~~~
aseipp
These analysis passes also work just fine with scoped locks, using the
SCOPE_CAPABILITY annotation (see the included mutex.h example).

Scoped locks by themselves cannot statically rule out the same kinds of errors
that this analysis can, in any case. Even if you _only_ use scoped locks -- an
obvious (but trivial) example is when you have a lock held but then attempt to
take a lock again in some control path (perhaps through a recursive or deep
call stack, or, in your case, calling another member function which also takes
the lock unconditionally.) This is a trivial case for this analysis to detect
but std::lock_guard<> alone will not save you.

The annotations also serve as useful commentary and 'guiding tools' for
refactoring as well, in my experience. If you annotate a function as yielding
or taking a lock, and later change this -- the compiler will warn you that
your annotation is out of sync with the implementation. In practice I find
this quite useful to keep code clean, but it also obviously helps tell you if
your change is correct, too.

I normally just turn this analysis on in Clang, because IME it has very little
negative effect and is very useful in practice.

~~~
duneroadrunner
> when you have a lock held but then attempt to take a lock again in some
> control path (perhaps through a recursive or deep call stack, or, in your
> case, calling another member function

The standard library provides std::recursive_mutex which supports being locked
multiple times by the same thread. Unfortunately, it doesn't provide an
std::recursive_shared_timed_mutex for the general case where you might want to
support multiple simultaneous read operations. The SaferCPlusPlus library does
implement such a mutex, and, like I mentioned in another comment, provides
data types that automatically control access to shared objects. I think for
most cases, it's the safest and easiest way to share objects asynchronously
(in C++). (The data types are kind of analogous to Arc<> and Arc< Mutex<> >
for those familiar with Rust.)

Btw, the implementation is self-contained, so you can use the safe sharing
data types without having to include the rest of the library. Or you can just
copy the source code and make your own safe sharing data types.

~~~
aseipp
There are many people who would largely consider recursive mutexes a complete
mistake, for the most part. I'm one of those people. :)

But I admit I sort of willfully skipped over this point precisely _because_ I
dislike recursive mutexes. But they are there for this case, you are 100%
right.

------
Matthias247
Looks interesting, especially as a way to introduce some extra thread safety
checking into legacy codebases. Or improve the quality of new ones if C++ is
the target (yes, I know that Rust can do similar things without extra tools).

If anyone is familiar with it I directly have a question regarding it: An
often used pattern besides locking is that data is owned by a particular
thread, and only that thread is allowed to mutate the data. E.g. in GUIs only
the main thread can mutate it. I guess that can also be expressed with the
tool, but examples would be welcome. I guess the owning thread could have the
capability, but a challenge could be that the start and end of the owning
thread might be encapsulated by a library (e.g. QT or Windows mainloop), and
be slightly hidden from the implementation of components. The goal should be
that if I do

    
    
        field = value;
    

inside an arbitrary thread I get a warning. And if I do

    
    
        owningThreadWithEventLoop.post([](){ field = value; });
    

everything is fine.

------
zvrba
I prefer to use language constructs to enforce correct usage where possible.

For example, if a set of values have to be synchronized by a mutex, I wrap
them in a struct and
[http://www.boost.org/doc/libs/1_63_0/doc/html/thread/sds.htm...](http://www.boost.org/doc/libs/1_63_0/doc/html/thread/sds.html#thread.sds.synchronized_valuesxxx)

If a method requires that it's called while a mutex is held, I make it
explicit by declaring the function as `void f(std::unique_lock<std::mutex>&)`
[requiring a reference ensures that you have to have a constructed object
before function entry]. Granted, this approach doesn't associate the mutex
with its data.

------
bla2
In my experience dynamic checkers like
[https://clang.llvm.org/docs/ThreadSanitizer.html](https://clang.llvm.org/docs/ThreadSanitizer.html)
tend to work better that static checks.

~~~
jsolson
We (Google) do both.

Having recently done some coding in Swift to implement a little toy
Hypervisor, I sorely missed these annotations. By and large the errors they
catch are ones I'd be embarrassed to have pointed out in a code review; I
don't mind nearly so much when the compiler catches me without my mutex
locked.

------
anonymousDan
Does anyone know of a similar tool for Java?

~~~
nurblieh
Google's ErrorProne, [https://github.com/google/error-
prone/tree/master/core/src/m...](https://github.com/google/error-
prone/tree/master/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety)

Bill Pugh's FindBugs,
[http://findbugs.sourceforge.net/bugDescriptions.html](http://findbugs.sourceforge.net/bugDescriptions.html)

------
nurettin
So is this a Rust killer?

~~~
rurban
Why Rust? Rust doesn't have concurrency capabilities. ponylang has.

But like Rust it needs mutexes and errors with wrong or missing mutexes. So in
this regard it's equal to Rust.

~~~
adrianN
> Rust doesn't have concurrency capabilities

[https://doc.rust-lang.org/book/concurrency.html](https://doc.rust-
lang.org/book/concurrency.html)

------
hellofunk
This is interesting, but I am curious why the effort has gone into this C++
tool when Google's own multi-threading-first language, Go, is based on Tony
Hoare's work from decades ago, except that it omits the most interesting
feature of CSP, the specialized calculus theorem that can prove the
correctness of all your multihreaded logic. It would seem that those
interesting ideas were ignored in favor of providing tooling for C++.

~~~
stymaar
For the record, Go has no static thread safety guaranty. It's easy to write a
data-race in Go (and it occurs frequently). Fortunately the go compiler has a
tool that help detecting data-races[1]. The only two languages I know that
offer data-race-free multi-threading are Pony[2] and Rust.

Additionally, Go isn't really a C++ competitor: you'll probably never see
Google Chrome using Go instead of C++. Go is a good language for back-end
micro-services and cli tools, not for building performance-sensitive native
desktop applications.

[1]:
[https://golang.org/doc/articles/race_detector.html](https://golang.org/doc/articles/race_detector.html)

[2]: [http://www.ponylang.org/](http://www.ponylang.org/)

~~~
masklinn
> The only two languages I know that offer data-race-free multi-threading are
> Pony[2] and Rust.

I believe Erlang also does for the most part, though some of its built-in
constructs are racey (the process registry if you query it, shared ETS tables,
and _dirty_ Mnesia operations).

