Data race in std::unique_lock

[TmLev]

Bugzilla Link	50365
Version	unspecified
OS	All
CC	@dwblaikie,@mclow

Extended Description

There are two lines in unique_lock::unlock that should appear in reverse order:

llvm-project/libcxx/include/__mutex_base

Line 255 in d480f96

__m_->unlock();

llvm-project/libcxx/include/__mutex_base

Line 256 in d480f96

__owns_ = false;

MRE would require a thread pool with fibers running over it (that's how TSan detected the race condition in the first place) so that one thread has unlocked __m_ but not written false yet, and another thread is destroying the lock already.

[mclow]

I concur; that's a bug.

[Quuxplusone]

Are you saying the following...?

• You've got two threads A and B, both able to access the same shared unique_lock object u, and u wraps a mutex m.
• Thread A is going to run u.lock(); u.unlock();
• Simultaneously, Thread B is going to run m.lock(); u.~unique_lock();
• If A wins the initial contention, then B will block inside m.lock(). B will unblock as soon as m becomes available, while A is still in the middle of running u.unlock() (that is, u is still in use by Thread A). This means that A and B have a data race because they're both trying to modify u at the same time (A is trying to finish up u.unlock(); B is trying to destroy u.)

But that's not unique_lock's fault, is it? Isn't that the fault of A and B for both trying to touch object u without synchronization? u wraps m, but u is not protected by m. I'd call this Not A (libc++) Bug, but rather a data race (UB) in the user's own code.

Swapping the two lines in libc++ might suppress the symptoms in this particular user's code, but has a non-zero risk of causing other hard-to-reason-about effects on other (non-UB) code.