rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

explain our rwlock implementation (and fix a potential data race)

Browse Source
This commit is contained in:
Ralf Jung
2020-05-04 19:37:55 +02:00
parent 61fdd3e2be
commit 40a6b8c339
1 changed files with 24 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
src/libstd/sys/unix/rwlock.rs
View File

@@ -22,27 +22,26 @@ impl RWLock {
pub unsafe fn read(&self) {
let r = libc::pthread_rwlock_rdlock(self.inner.get());
// According to the pthread_rwlock_rdlock spec, this function **may**
// fail with EDEADLK if a deadlock is detected. On the other hand
// pthread mutexes will *never* return EDEADLK if they are initialized
// as the "fast" kind (which ours always are). As a result, a deadlock
// situation may actually return from the call to pthread_rwlock_rdlock
// instead of blocking forever (as mutexes and Windows rwlocks do). Note
// that not all unix implementations, however, will return EDEADLK for
// their rwlocks.
// According to POSIX, when a thread tries to acquire this read lock
// while it already holds the write lock
// (or vice versa, or tries to acquire the write lock twice),
// "the call shall either deadlock or return [EDEADLK]"
// (https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_wrlock.html,
// https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_rdlock.html).
// So, in principle, all we have to do here is check `r == 0` to be sure we properly
// got the lock.
//
// We roughly maintain the deadlocking behavior by panicking to ensure
// that this lock acquisition does not succeed.
//
// We also check whether this lock is already write locked. This
// is only possible if it was write locked by the current thread and
// the implementation allows recursive locking. The POSIX standard
// doesn't require recursively locking a rwlock to deadlock, but we can't
// allow that because it could lead to aliasing issues.
// However, (at least) glibc before version 2.25 does not conform to this spec,
// and can return `r == 0` even when this thread already holds the write lock.
// We thus check for this situation ourselves and panic when detecting that a thread
// got the write lock more than once, or got a read and a write lock.
if r == libc::EAGAIN {
panic!("rwlock maximum reader count exceeded");
} else if r == libc::EDEADLK || (r == 0 && *self.write_locked.get()) {
// Above, we make sure to only access `write_locked` when `r == 0` to avoid
// data races.
if r == 0 {
// `pthread_rwlock_rdlock` succeeded when it should not have.
self.raw_unlock();
}
panic!("rwlock read lock would result in deadlock");
@@ -56,6 +55,7 @@ impl RWLock {
let r = libc::pthread_rwlock_tryrdlock(self.inner.get());
if r == 0 {
if *self.write_locked.get() {
// `pthread_rwlock_tryrdlock` succeeded when it should not have.
self.raw_unlock();
false
} else {
@@ -69,18 +69,21 @@ impl RWLock {
#[inline]
pub unsafe fn write(&self) {
let r = libc::pthread_rwlock_wrlock(self.inner.get());
// See comments above for why we check for EDEADLK and write_locked. We
// also need to check that num_readers is 0.
// See comments above for why we check for EDEADLK and write_locked. For the same reason,
// we also need to check that there are no readers (tracked in `num_readers`).
if r == libc::EDEADLK
|| *self.write_locked.get()
|| (r == 0 && *self.write_locked.get())
|| self.num_readers.load(Ordering::Relaxed) != 0
{
// Above, we make sure to only access `write_locked` when `r == 0` to avoid
// data races.
if r == 0 {
// `pthread_rwlock_wrlock` succeeded when it should not have.
self.raw_unlock();
}
panic!("rwlock write lock would result in deadlock");
} else {
debug_assert_eq!(r, 0);
assert_eq!(r, 0);
}
*self.write_locked.get() = true;
}
@@ -89,6 +92,7 @@ impl RWLock {
let r = libc::pthread_rwlock_trywrlock(self.inner.get());
if r == 0 {
if *self.write_locked.get() || self.num_readers.load(Ordering::Relaxed) != 0 {
// `pthread_rwlock_trywrlock` succeeded when it should not have.
self.raw_unlock();
false
} else {