[PATCH] D29588: Use rw locks for sanitizer thread registry

[Francis Ricci via Phabricator via llvm-commits]

fjricci added a comment.

In https://reviews.llvm.org/D29588#668828, @dvyukov wrote:

> > Sharing a mutex access across threads like this is not supported by blocking mutex
>
> Why?


If a thread owns a mutex on the thread registry for writing (as with a blocking mutex), we can't really assume in another thread that the registry won't change. Since the thread accessing the data is actually in a separate process (the result of a clone() in StopTheWorld), I'm not sure that there's even a way to pass the blocking mutex around safely.

> I've checked BlockingMutex comments and linux/mac/win implementation. I don't see any indication that it's not supported.
>  How is RWMutex different?

An RWMutex acquired for read-only use can be safely used for reads by multiple threads, as it guarantees the protected data won't change. A blocking mutex provides no guarantees that the protected state won't change (and in fact implies that it probably will).

> How does the problem manifest? What is the scenario that demonstrated the problem?

There are two pieces to this. The primary bit is just that adding checks to ensure that `CheckLocked()` only succeeds if the calling thread is the thread that owns the mutex will fail, as demonstrated by a cherry-pick of https://reviews.llvm.org/D29594. The other bit is consistency. Since these checks already exist on darwin and windows, the existing code will fail if and when we decide to add support for lsan to those platforms. This gives essentially two options - one would be removing the validation checks from the darwin and windows blocking mutex implementations, so that they succeed in the same way that linux currently does, by no longer forcing CheckLocked() to fail if another thread owns the mutex. The other would be the current method of adding strict validation checks to the linux implementation, which requires moving away from a blocking mutex.


https://reviews.llvm.org/D29588