Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions library/std/src/sync/nonpoison/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ unsafe impl<T: ?Sized + Send> Sync for Mutex<T> {}
#[cfg_attr(not(test), rustc_diagnostic_item = "NonPoisonMutexGuard")]
pub struct MutexGuard<'a, T: ?Sized + 'a> {
lock: &'a Mutex<T>,
/// The unlocked state is used to prevent double unlocking of guards upon panicking in
/// unlocked scopes.
unlocked: bool,
}

/// A [`MutexGuard`] is not `Send` to maximize platform portability.
Expand Down Expand Up @@ -447,7 +450,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>) -> MutexGuard<'mutex, T> {
return MutexGuard { lock };
MutexGuard { lock, unlocked: false }
}
}

Expand All @@ -471,8 +474,10 @@ impl<T: ?Sized> DerefMut for MutexGuard<'_, T> {
impl<T: ?Sized> Drop for MutexGuard<'_, T> {
#[inline]
fn drop(&mut self) {
unsafe {
self.lock.inner.unlock();
if !self.unlocked {
unsafe {
self.lock.inner.unlock();
}
}
}
}
Expand All @@ -496,6 +501,48 @@ pub(super) fn guard_lock<'a, T: ?Sized>(guard: &MutexGuard<'a, T>) -> &'a sys::M
&guard.lock.inner
}

impl<'mutex, T: ?Sized> MutexGuard<'mutex, T> {
/// Unlocks the [`MutexGuard`] for the scope of `func` and acquires it again after.
/// Panics won't lock the guard again.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How important is this guarantee? I find it unfortunate that this necessitates adding a field and an additional check in the Drop implementation, which affects all code using MutexGuards, not just code that calls unlocked.

///
/// # Examples
///
/// ```
/// #[feature(unlockable_guards)]
///
/// use std::sync::nonpoison::Mutex;
/// use std::sync::nonpoison::MutexGuard;
/// use std::sync::nonpoison::TryLockResult;
///
/// let mutex = Mutex::new(1usize);
/// let mut guard = mutex.lock();
///
/// // guard is locked and can be used here
/// *guard = 5;
///
/// MutexGuard::unlocked(&mut guard, || {
/// // guard is locked and can be acquired potentially from another thread
/// assert!(matches!(mutex.try_lock(), TryLockResult::Ok(_)));
/// });
///
/// // guard is locked again
/// assert_eq!(*guard, 5);
/// ```
#[unstable(feature = "unlockable_guards", issue = "148568")]
pub fn unlocked<F>(self: &mut Self, func: F) -> ()
where
F: FnOnce() -> (),
{
self.unlocked = true;
unsafe { self.lock.inner.unlock() };

func();

self.lock.inner.lock();
Comment on lines +537 to +541
Copy link
Copy Markdown
Member

@y21 y21 Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fundamental design issue, but the implementation looks unsound in the presence of panics since it leaves the Mutex unlocked when exiting the function

let mutex = Mutex::new(1);
let mut guard = mutex.lock().unwrap();
panic::catch_unwind(AssertUnwindSafe(|| MutexGuard::unlocked(&mut guard, || panic!())));

let ref1 = &mut *guard;
let ref2 = &mut *mutex.lock();
dbg!(ref1, ref2);

lock_api doesn't have this issue since it calls lock() in a local's Drop impl that always executes, should probably do the same here.

edit: Ah, I overlooked the new unlocked: bool field, the comment for it also talks about panicking in the closure, but from a cursory look I'm not sure it prevents this issue?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, that's unsound. I don't think the "no relock during panic" guarantee can be soundly provided, at least if unlocked only takes the mutex guard by reference.

self.unlocked = false;
}
}

impl<'a, T: ?Sized> MutexGuard<'a, T> {
/// Makes a [`MappedMutexGuard`] for a component of the borrowed data, e.g.
/// an enum variant.
Expand Down
109 changes: 102 additions & 7 deletions library/std/src/sync/nonpoison/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ pub struct RwLockReadGuard<'rwlock, T: ?Sized + 'rwlock> {
data: NonNull<T>,
/// A reference to the internal [`sys::RwLock`] that we have read-locked.
inner_lock: &'rwlock sys::RwLock,
/// The unlocked state is used to prevent double unlocking of guards upon panicking in
/// unlocked scopes.
unlocked: bool,
}

#[unstable(feature = "nonpoison_rwlock", issue = "134645")]
Expand All @@ -107,6 +110,9 @@ unsafe impl<T: ?Sized + Sync> Sync for RwLockReadGuard<'_, T> {}
pub struct RwLockWriteGuard<'rwlock, T: ?Sized + 'rwlock> {
/// A reference to the [`RwLock`] that we have write-locked.
lock: &'rwlock RwLock<T>,
/// The unlocked state is used to prevent double unlocking of guards upon panicking in
/// unlocked scopes.
unlocked: bool,
}

#[unstable(feature = "nonpoison_rwlock", issue = "134645")]
Expand Down Expand Up @@ -607,6 +613,7 @@ impl<'rwlock, T: ?Sized> RwLockReadGuard<'rwlock, T> {
RwLockReadGuard {
data: unsafe { NonNull::new_unchecked(lock.data.get()) },
inner_lock: &lock.inner,
unlocked: false,
}
}

Expand Down Expand Up @@ -684,7 +691,7 @@ impl<'rwlock, T: ?Sized> RwLockWriteGuard<'rwlock, T> {
/// `lock.inner.write()`, `lock.inner.try_write()`, or `lock.inner.try_upgrade` before
/// instantiating this object.
unsafe fn new(lock: &'rwlock RwLock<T>) -> RwLockWriteGuard<'rwlock, T> {
RwLockWriteGuard { lock }
RwLockWriteGuard { lock, unlocked: false }
}

/// Downgrades a write-locked `RwLockWriteGuard` into a read-locked [`RwLockReadGuard`].
Expand Down Expand Up @@ -976,19 +983,23 @@ impl<'rwlock, T: ?Sized> MappedRwLockWriteGuard<'rwlock, T> {
#[unstable(feature = "nonpoison_rwlock", issue = "134645")]
impl<T: ?Sized> Drop for RwLockReadGuard<'_, T> {
fn drop(&mut self) {
// SAFETY: the conditions of `RwLockReadGuard::new` were satisfied when created.
unsafe {
self.inner_lock.read_unlock();
if !self.unlocked {
// SAFETY: the conditions of `RwLockReadGuard::new` were satisfied when created.
unsafe {
self.inner_lock.read_unlock();
}
}
}
}

#[unstable(feature = "nonpoison_rwlock", issue = "134645")]
impl<T: ?Sized> Drop for RwLockWriteGuard<'_, T> {
fn drop(&mut self) {
// SAFETY: the conditions of `RwLockWriteGuard::new` were satisfied when created.
unsafe {
self.lock.inner.write_unlock();
if !self.unlocked {
// SAFETY: the conditions of `RwLockWriteGuard::new` were satisfied when created.
unsafe {
self.lock.inner.write_unlock();
}
}
}
}
Expand Down Expand Up @@ -1138,3 +1149,87 @@ impl<T: ?Sized + fmt::Display> fmt::Display for MappedRwLockWriteGuard<'_, T> {
(**self).fmt(f)
}
}

impl<'rw_lock, T: ?Sized> RwLockReadGuard<'rw_lock, T> {
/// Unlocks the [`RwLockReadGuard`] for the scope of `func` and acquires it again after.
/// Panics won't lock the guard again.
///
/// # Examples
///
/// ```
/// #[feature(unlockable_guards)]
///
/// use std::sync::nonpoison::RwLock;
/// use std::sync::nonpoison::RwLockReadGuard;
/// use std::sync::nonpoison::TryLockResult;
///
/// let rw_lock = RwLock::new(1usize);
/// let mut read_guard = rw_lock.read();
///
/// // guard is locked and can be used here
/// assert_eq!(*read_guard, 1);
///
/// RwLockReadGuard::unlocked(&mut read_guard, || {
/// // guard is locked and can be acquired potentially from another thread
/// assert!(matches!(rw_lock.try_write(), TryLockResult::Ok(_)));
/// });
///
/// // guard is locked again
/// assert_eq!(*read_guard, 1);
/// ```
#[unstable(feature = "unlockable_guards", issue = "148568")]
pub fn unlocked<F>(self: &mut Self, func: F) -> ()
where
F: FnOnce() -> (),
{
self.unlocked = true;
unsafe { self.inner_lock.read_unlock() };

func();

self.inner_lock.read();
self.unlocked = false;
}
}

impl<'rw_lock, T: ?Sized> RwLockWriteGuard<'rw_lock, T> {
/// Unlocks the [`RwLockWriteGuard`] for the scope of `func` and acquires it again after.
/// Panics won't lock the guard again.
///
/// # Examples
///
/// ```
/// #[feature(unlockable_guards)]
///
/// use std::sync::nonpoison::RwLock;
/// use std::sync::nonpoison::RwLockWriteGuard;
/// use std::sync::nonpoison::TryLockResult;
///
/// let rw_lock = RwLock::new(1usize);
/// let mut write_guard = rw_lock.write();
///
/// // guard is locked and can be used here
/// *write_guard = 5;
///
/// RwLockWriteGuard::unlocked(&mut write_guard, || {
/// // guard is locked and can be acquired potentially from another thread
/// assert!(matches!(rw_lock.try_write(), TryLockResult::Ok(_)));
/// });
///
/// // guard is locked again
/// assert_eq!(*write_guard, 5);
/// ```
#[unstable(feature = "unlockable_guards", issue = "148568")]
pub fn unlocked<F>(self: &mut Self, func: F) -> ()
where
F: FnOnce() -> (),
{
self.unlocked = true;
unsafe { self.lock.inner.write_unlock() };

func();

self.lock.inner.write();
self.unlocked = false;
}
}
18 changes: 18 additions & 0 deletions library/std/tests/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,3 +531,21 @@ fn test_mutex_with_mut() {
assert_eq!(*mutex.lock(), 5);
assert_eq!(result, 10);
}

////////////////////////////////////////////////////////////////////////////////////////////////////
// Nonpoison Tests
////////////////////////////////////////////////////////////////////////////////////////////////////

#[test]
fn test_mutex_guard_unlocked() {
let mutex = std::sync::nonpoison::Mutex::new(1usize);
let mut guard = mutex.lock();

assert!(matches!(mutex.try_lock(), std::sync::nonpoison::TryLockResult::Err(_)));

MutexGuard::unlocked(&mut guard, || {
assert!(matches!(mutex.try_lock(), std::sync::nonpoison::TryLockResult::Ok(_)));
});

assert!(matches!(mutex.try_lock(), std::sync::nonpoison::TryLockResult::Err(_)));
}
32 changes: 32 additions & 0 deletions library/std/tests/sync/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -883,3 +883,35 @@ fn test_rwlock_with_mut() {
assert_eq!(*rwlock.read(), 5);
assert_eq!(result, 10);
}

////////////////////////////////////////////////////////////////////////////////////////////////////
// Non-poison Tests
////////////////////////////////////////////////////////////////////////////////////////////////////

#[test]
fn test_read_guard_unlocked() {
let rw_lock = std::sync::nonpoison::RwLock::new(1usize);
let mut read_guard = rw_lock.read();

assert!(matches!(rw_lock.try_write(), std::sync::nonpoison::TryLockResult::Err(_)));

RwLockReadGuard::unlocked(&mut read_guard, || {
assert!(matches!(rw_lock.try_write(), std::sync::nonpoison::TryLockResult::Ok(_)));
});

assert!(matches!(rw_lock.try_write(), std::sync::nonpoison::TryLockResult::Err(_)));
}

#[test]
fn test_write_guard_unlocked() {
let rw_lock = std::sync::nonpoison::RwLock::new(1usize);
let mut write_guard = rw_lock.write();

assert!(matches!(rw_lock.try_write(), std::sync::nonpoison::TryLockResult::Err(_)));

RwLockWriteGuard::unlocked(&mut write_guard, || {
assert!(matches!(rw_lock.try_write(), std::sync::nonpoison::TryLockResult::Ok(_)));
});

assert!(matches!(rw_lock.try_write(), std::sync::nonpoison::TryLockResult::Err(_)));
}
Loading