Skip to content

Commit b858b7f

Browse files
committed
Auto merge of #25015 - alexcrichton:rwlock-check-ret, r=aturon
Apparently implementations are allowed to return EDEADLK instead of blocking forever, in which case this can lead to unsafety in the `RwLock` primitive exposed by the standard library. A debug-build of the standard library would have caught this error (due to the debug assert), but we don't ship debug builds right now. This commit adds explicit checks for the EDEADLK error code and triggers a panic to ensure the call does not succeed. Closes #25012
2 parents c42c1e7 + 5c8ca26 commit b858b7f

File tree

1 file changed

+29
-12
lines changed

1 file changed

+29
-12
lines changed

src/libstd/sys/unix/rwlock.rs

+29-12
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use prelude::v1::*;
1212

13+
use libc;
1314
use cell::UnsafeCell;
1415
use sys::sync as ffi;
1516

@@ -26,7 +27,23 @@ impl RWLock {
2627
#[inline]
2728
pub unsafe fn read(&self) {
2829
let r = ffi::pthread_rwlock_rdlock(self.inner.get());
29-
debug_assert_eq!(r, 0);
30+
31+
// According to the pthread_rwlock_rdlock spec, this function **may**
32+
// fail with EDEADLK if a deadlock is detected. On the other hand
33+
// pthread mutexes will *never* return EDEADLK if they are initialized
34+
// as the "fast" kind (which ours always are). As a result, a deadlock
35+
// situation may actually return from the call to pthread_rwlock_rdlock
36+
// instead of blocking forever (as mutexes and Windows rwlocks do). Note
37+
// that not all unix implementations, however, will return EDEADLK for
38+
// their rwlocks.
39+
//
40+
// We roughly maintain the deadlocking behavior by panicking to ensure
41+
// that this lock acquisition does not succeed.
42+
if r == libc::EDEADLK {
43+
panic!("rwlock read lock would result in deadlock");
44+
} else {
45+
debug_assert_eq!(r, 0);
46+
}
3047
}
3148
#[inline]
3249
pub unsafe fn try_read(&self) -> bool {
@@ -35,7 +52,12 @@ impl RWLock {
3552
#[inline]
3653
pub unsafe fn write(&self) {
3754
let r = ffi::pthread_rwlock_wrlock(self.inner.get());
38-
debug_assert_eq!(r, 0);
55+
// see comments above for why we check for EDEADLK
56+
if r == libc::EDEADLK {
57+
panic!("rwlock write lock would result in deadlock");
58+
} else {
59+
debug_assert_eq!(r, 0);
60+
}
3961
}
4062
#[inline]
4163
pub unsafe fn try_write(&self) -> bool {
@@ -49,21 +71,16 @@ impl RWLock {
4971
#[inline]
5072
pub unsafe fn write_unlock(&self) { self.read_unlock() }
5173
#[inline]
52-
#[cfg(not(target_os = "dragonfly"))]
53-
pub unsafe fn destroy(&self) {
54-
let r = ffi::pthread_rwlock_destroy(self.inner.get());
55-
debug_assert_eq!(r, 0);
56-
}
57-
58-
#[inline]
59-
#[cfg(target_os = "dragonfly")]
6074
pub unsafe fn destroy(&self) {
61-
use libc;
6275
let r = ffi::pthread_rwlock_destroy(self.inner.get());
6376
// On DragonFly pthread_rwlock_destroy() returns EINVAL if called on a
6477
// rwlock that was just initialized with
6578
// ffi::PTHREAD_RWLOCK_INITIALIZER. Once it is used (locked/unlocked)
6679
// or pthread_rwlock_init() is called, this behaviour no longer occurs.
67-
debug_assert!(r == 0 || r == libc::EINVAL);
80+
if cfg!(target_os = "dragonfly") {
81+
debug_assert!(r == 0 || r == libc::EINVAL);
82+
} else {
83+
debug_assert_eq!(r, 0);
84+
}
6885
}
6986
}

0 commit comments

Comments
 (0)