Skip to content

Commit

Permalink
Auto merge of rust-lang#97924 - cuviper:unguarded-poison, r=Mark-Simu…
Browse files Browse the repository at this point in the history
…lacrum

Avoid `thread::panicking()` in non-poisoning methods of `Mutex` and `RwLock`

`Mutex::lock()` and `RwLock::write()` are poison-guarded against panics,
in that they set the poison flag if a panic occurs while they're locked.
But if we're already in a panic (`thread::panicking()`), they leave the
poison flag alone.

That check is a bit of a waste for methods that never set the poison
flag though, namely `get_mut()`, `into_inner()`, and `RwLock::read()`.
These use-cases are now split to avoid that unnecessary call.
  • Loading branch information
bors committed Jun 18, 2022
2 parents 2cec687 + 34895de commit ec21d7e
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
6 changes: 3 additions & 3 deletions library/std/src/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ impl<T: ?Sized> Mutex<T> {
T: Sized,
{
let data = self.data.into_inner();
poison::map_result(self.poison.borrow(), |_| data)
poison::map_result(self.poison.borrow(), |()| data)
}

/// Returns a mutable reference to the underlying data.
Expand All @@ -448,7 +448,7 @@ impl<T: ?Sized> Mutex<T> {
#[stable(feature = "mutex_get_mut", since = "1.6.0")]
pub fn get_mut(&mut self) -> LockResult<&mut T> {
let data = self.data.get_mut();
poison::map_result(self.poison.borrow(), |_| data)
poison::map_result(self.poison.borrow(), |()| data)
}
}

Expand Down Expand Up @@ -497,7 +497,7 @@ impl<T: ?Sized + fmt::Debug> fmt::Debug for Mutex<T> {

impl<'mutex, T: ?Sized> MutexGuard<'mutex, T> {
unsafe fn new(lock: &'mutex Mutex<T>) -> LockResult<MutexGuard<'mutex, T>> {
poison::map_result(lock.poison.borrow(), |guard| MutexGuard { lock, poison: guard })
poison::map_result(lock.poison.guard(), |guard| MutexGuard { lock, poison: guard })
}
}

Expand Down
9 changes: 8 additions & 1 deletion library/std/src/sync/poison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,15 @@ impl Flag {
Flag { failed: AtomicBool::new(false) }
}

/// Check the flag for an unguarded borrow, where we only care about existing poison.
#[inline]
pub fn borrow(&self) -> LockResult<Guard> {
pub fn borrow(&self) -> LockResult<()> {
if self.get() { Err(PoisonError::new(())) } else { Ok(()) }
}

/// Check the flag for a guarded borrow, where we may also set poison when `done`.
#[inline]
pub fn guard(&self) -> LockResult<Guard> {
let ret = Guard { panicking: thread::panicking() };
if self.get() { Err(PoisonError::new(ret)) } else { Ok(ret) }
}
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sync/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl<T: ?Sized> RwLock<T> {
T: Sized,
{
let data = self.data.into_inner();
poison::map_result(self.poison.borrow(), |_| data)
poison::map_result(self.poison.borrow(), |()| data)
}

/// Returns a mutable reference to the underlying data.
Expand All @@ -461,7 +461,7 @@ impl<T: ?Sized> RwLock<T> {
#[stable(feature = "rwlock_get_mut", since = "1.6.0")]
pub fn get_mut(&mut self) -> LockResult<&mut T> {
let data = self.data.get_mut();
poison::map_result(self.poison.borrow(), |_| data)
poison::map_result(self.poison.borrow(), |()| data)
}
}

Expand Down Expand Up @@ -510,13 +510,13 @@ impl<T> From<T> for RwLock<T> {

impl<'rwlock, T: ?Sized> RwLockReadGuard<'rwlock, T> {
unsafe fn new(lock: &'rwlock RwLock<T>) -> LockResult<RwLockReadGuard<'rwlock, T>> {
poison::map_result(lock.poison.borrow(), |_| RwLockReadGuard { lock })
poison::map_result(lock.poison.borrow(), |()| RwLockReadGuard { lock })
}
}

impl<'rwlock, T: ?Sized> RwLockWriteGuard<'rwlock, T> {
unsafe fn new(lock: &'rwlock RwLock<T>) -> LockResult<RwLockWriteGuard<'rwlock, T>> {
poison::map_result(lock.poison.borrow(), |guard| RwLockWriteGuard { lock, poison: guard })
poison::map_result(lock.poison.guard(), |guard| RwLockWriteGuard { lock, poison: guard })
}
}

Expand Down

0 comments on commit ec21d7e

Please sign in to comment.