Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement MappedMutexGuard, MappedRwLockReadGuard, and MappedRwLockWriteGuard. #117107

Merged
merged 10 commits into from
Feb 25, 2024
4 changes: 4 additions & 0 deletions library/std/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,17 @@ pub use core::sync::Exclusive;
pub use self::barrier::{Barrier, BarrierWaitResult};
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::condvar::{Condvar, WaitTimeoutResult};
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub use self::mutex::MappedMutexGuard;
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::mutex::{Mutex, MutexGuard};
#[stable(feature = "rust1", since = "1.0.0")]
#[allow(deprecated)]
pub use self::once::{Once, OnceState, ONCE_INIT};
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::poison::{LockResult, PoisonError, TryLockError, TryLockResult};
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub use self::rwlock::{MappedRwLockReadGuard, MappedRwLockWriteGuard};
#[stable(feature = "rust1", since = "1.0.0")]
pub use self::rwlock::{RwLock, RwLockReadGuard, RwLockWriteGuard};

Expand Down
219 changes: 219 additions & 0 deletions library/std/src/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ mod tests;

use crate::cell::UnsafeCell;
use crate::fmt;
use crate::marker::PhantomData;
use crate::mem::ManuallyDrop;
use crate::ops::{Deref, DerefMut};
use crate::ptr::NonNull;
use crate::sync::{poison, LockResult, TryLockError, TryLockResult};
use crate::sys::locks as sys;

Expand Down Expand Up @@ -213,6 +216,47 @@ impl<T: ?Sized> !Send for MutexGuard<'_, T> {}
#[stable(feature = "mutexguard", since = "1.19.0")]
unsafe impl<T: ?Sized + Sync> Sync for MutexGuard<'_, T> {}

/// An RAII mutex guard returned by `MutexGuard::map`, which can point to a
/// subfield of the protected data. When this structure is dropped (falls out
/// of scope), the lock will be unlocked.
///
/// The main difference between `MappedMutexGuard` and [`MutexGuard`] is that the
/// former cannot be used with [`Condvar`], since that
/// could introduce soundness issues if the locked object is modified by another
/// thread while the `Mutex` is unlocked.
///
/// The data protected by the mutex can be accessed through this guard via its
/// [`Deref`] and [`DerefMut`] implementations.
///
/// This structure is created by the [`map`] and [`try_map`] methods on
/// [`MutexGuard`].
///
/// [`map`]: MutexGuard::map
/// [`try_map`]: MutexGuard::try_map
/// [`Condvar`]: crate::sync::Condvar
#[must_use = "if unused the Mutex will immediately unlock"]
#[must_not_suspend = "holding a MappedMutexGuard across suspend \
points can cause deadlocks, delays, \
and cause Futures to not implement `Send`"]
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
#[clippy::has_significant_drop]
pub struct MappedMutexGuard<'a, T: ?Sized + 'a> {
// NB: we use a pointer instead of `&'a mut T` to avoid `noalias` violations, because a
// `MappedMutexGuard` argument doesn't hold uniqueness for its whole scope, only until it drops.
// `NonNull` is covariant over `T`, so we add a `PhantomData<&'a mut T>` field
// below for the correct variance over `T` (invariance).
data: NonNull<T>,
inner: &'a sys::Mutex,
poison_flag: &'a poison::Flag,
poison: poison::Guard,
_variance: PhantomData<&'a mut T>,
}

#[unstable(feature = "mapped_lock_guards", issue = "117108")]
impl<T: ?Sized> !Send for MappedMutexGuard<'_, T> {}
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
unsafe impl<T: ?Sized + Sync> Sync for MappedMutexGuard<'_, T> {}

impl<T> Mutex<T> {
/// Creates a new mutex in an unlocked state ready for use.
///
Expand Down Expand Up @@ -552,3 +596,178 @@ pub fn guard_lock<'a, T: ?Sized>(guard: &MutexGuard<'a, T>) -> &'a sys::Mutex {
pub fn guard_poison<'a, T: ?Sized>(guard: &MutexGuard<'a, T>) -> &'a poison::Flag {
&guard.lock.poison
}

impl<'a, T: ?Sized> MutexGuard<'a, T> {
/// Makes a [`MappedMutexGuard`] for a component of the borrowed data, e.g.
/// an enum variant.
///
/// The `Mutex` is already locked, so this cannot fail.
///
/// This is an associated function that needs to be used as
/// `MutexGuard::map(...)`. A method would interfere with methods of the
/// same name on the contents of the `MutexGuard` used through `Deref`.
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub fn map<U, F>(orig: Self, f: F) -> MappedMutexGuard<'a, U>
where
F: FnOnce(&mut T) -> &mut U,
U: ?Sized,
{
// SAFETY: the conditions of `MutexGuard::new` were satisfied when the original guard
// was created, and have been upheld throughout `map` and/or `try_map`.
// The signature of the closure guarantees that it will not "leak" the lifetime of the reference
// passed to it. If the closure panics, the guard will be dropped.
let data = NonNull::from(f(unsafe { &mut *orig.lock.data.get() }));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(applies to all the (try_)map functions) I used the unsafe here instead of just &(mut) *orig to be clearer that the lifetime of the reference is longer than the lifetime of the orig variable itself. However I think the other version should still be sound, so I can change it back to that if it seems better.

let orig = ManuallyDrop::new(orig);
MappedMutexGuard {
data,
inner: &orig.lock.inner,
poison_flag: &orig.lock.poison,
poison: orig.poison.clone(),
_variance: PhantomData,
}
}

/// Makes a [`MappedMutexGuard`] for a component of the borrowed data. The
/// original guard is returned as an `Err(...)` if the closure returns
/// `None`.
///
/// The `Mutex` is already locked, so this cannot fail.
///
/// This is an associated function that needs to be used as
/// `MutexGuard::try_map(...)`. A method would interfere with methods of the
/// same name on the contents of the `MutexGuard` used through `Deref`.
#[doc(alias = "filter_map")]
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub fn try_map<U, F>(orig: Self, f: F) -> Result<MappedMutexGuard<'a, U>, Self>
where
F: FnOnce(&mut T) -> Option<&mut U>,
U: ?Sized,
{
// SAFETY: the conditions of `MutexGuard::new` were satisfied when the original guard
// was created, and have been upheld throughout `map` and/or `try_map`.
// The signature of the closure guarantees that it will not "leak" the lifetime of the reference
// passed to it. If the closure panics, the guard will be dropped.
match f(unsafe { &mut *orig.lock.data.get() }) {
Some(data) => {
let data = NonNull::from(data);
let orig = ManuallyDrop::new(orig);
Ok(MappedMutexGuard {
data,
inner: &orig.lock.inner,
poison_flag: &orig.lock.poison,
poison: orig.poison.clone(),
_variance: PhantomData,
})
}
None => Err(orig),
}
}
}

#[unstable(feature = "mapped_lock_guards", issue = "117108")]
impl<T: ?Sized> Deref for MappedMutexGuard<'_, T> {
type Target = T;

fn deref(&self) -> &T {
unsafe { self.data.as_ref() }
}
}

#[unstable(feature = "mapped_lock_guards", issue = "117108")]
impl<T: ?Sized> DerefMut for MappedMutexGuard<'_, T> {
fn deref_mut(&mut self) -> &mut T {
unsafe { self.data.as_mut() }
}
}

#[unstable(feature = "mapped_lock_guards", issue = "117108")]
impl<T: ?Sized> Drop for MappedMutexGuard<'_, T> {
#[inline]
fn drop(&mut self) {
unsafe {
self.poison_flag.done(&self.poison);
self.inner.unlock();
}
}
}

#[unstable(feature = "mapped_lock_guards", issue = "117108")]
impl<T: ?Sized + fmt::Debug> fmt::Debug for MappedMutexGuard<'_, T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Debug::fmt(&**self, f)
}
}

#[unstable(feature = "mapped_lock_guards", issue = "117108")]
impl<T: ?Sized + fmt::Display> fmt::Display for MappedMutexGuard<'_, T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
(**self).fmt(f)
}
}

impl<'a, T: ?Sized> MappedMutexGuard<'a, T> {
/// Makes a [`MappedMutexGuard`] for a component of the borrowed data, e.g.
/// an enum variant.
///
/// The `Mutex` is already locked, so this cannot fail.
///
/// This is an associated function that needs to be used as
/// `MappedMutexGuard::map(...)`. A method would interfere with methods of the
/// same name on the contents of the `MutexGuard` used through `Deref`.
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub fn map<U, F>(mut orig: Self, f: F) -> MappedMutexGuard<'a, U>
where
F: FnOnce(&mut T) -> &mut U,
U: ?Sized,
{
// SAFETY: the conditions of `MutexGuard::new` were satisfied when the original guard
// was created, and have been upheld throughout `map` and/or `try_map`.
// The signature of the closure guarantees that it will not "leak" the lifetime of the reference
// passed to it. If the closure panics, the guard will be dropped.
let data = NonNull::from(f(unsafe { orig.data.as_mut() }));
let orig = ManuallyDrop::new(orig);
MappedMutexGuard {
data,
inner: orig.inner,
poison_flag: orig.poison_flag,
poison: orig.poison.clone(),
_variance: PhantomData,
}
}

/// Makes a [`MappedMutexGuard`] for a component of the borrowed data. The
/// original guard is returned as an `Err(...)` if the closure returns
/// `None`.
///
/// The `Mutex` is already locked, so this cannot fail.
///
/// This is an associated function that needs to be used as
/// `MappedMutexGuard::try_map(...)`. A method would interfere with methods of the
/// same name on the contents of the `MutexGuard` used through `Deref`.
#[doc(alias = "filter_map")]
#[unstable(feature = "mapped_lock_guards", issue = "117108")]
pub fn try_map<U, F>(mut orig: Self, f: F) -> Result<MappedMutexGuard<'a, U>, Self>
where
F: FnOnce(&mut T) -> Option<&mut U>,
U: ?Sized,
{
// SAFETY: the conditions of `MutexGuard::new` were satisfied when the original guard
// was created, and have been upheld throughout `map` and/or `try_map`.
// The signature of the closure guarantees that it will not "leak" the lifetime of the reference
// passed to it. If the closure panics, the guard will be dropped.
match f(unsafe { orig.data.as_mut() }) {
Some(data) => {
let data = NonNull::from(data);
let orig = ManuallyDrop::new(orig);
Ok(MappedMutexGuard {
data,
inner: orig.inner,
poison_flag: orig.poison_flag,
poison: orig.poison.clone(),
_variance: PhantomData,
})
}
None => Err(orig),
}
}
}
91 changes: 90 additions & 1 deletion library/std/src/sync/mutex/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::sync::atomic::{AtomicUsize, Ordering};
use crate::sync::mpsc::channel;
use crate::sync::{Arc, Condvar, Mutex};
use crate::sync::{Arc, Condvar, MappedMutexGuard, Mutex, MutexGuard, TryLockError};
use crate::thread;

struct Packet<T>(Arc<(Mutex<T>, Condvar)>);
Expand Down Expand Up @@ -188,6 +188,21 @@ fn test_mutex_arc_poison() {
assert!(arc.is_poisoned());
}

#[test]
fn test_mutex_arc_poison_mapped() {
let arc = Arc::new(Mutex::new(1));
assert!(!arc.is_poisoned());
let arc2 = arc.clone();
let _ = thread::spawn(move || {
let lock = arc2.lock().unwrap();
let lock = MutexGuard::map(lock, |val| val);
assert_eq!(*lock, 2); // deliberate assertion failure to poison the mutex
})
.join();
assert!(arc.lock().is_err());
assert!(arc.is_poisoned());
}

#[test]
fn test_mutex_arc_nested() {
// Tests nested mutexes and access
Expand Down Expand Up @@ -236,3 +251,77 @@ fn test_mutex_unsized() {
let comp: &[i32] = &[4, 2, 5];
assert_eq!(&*mutex.lock().unwrap(), comp);
}

#[test]
fn test_mapping_mapped_guard() {
let arr = [0; 4];
let mut lock = Mutex::new(arr);
let guard = lock.lock().unwrap();
let guard = MutexGuard::map(guard, |arr| &mut arr[..2]);
let mut guard = MappedMutexGuard::map(guard, |slice| &mut slice[1..]);
assert_eq!(guard.len(), 1);
guard[0] = 42;
drop(guard);
assert_eq!(*lock.get_mut().unwrap(), [0, 42, 0, 0]);
}

#[test]
fn panic_while_mapping_unlocked_poison() {
let lock = Mutex::new(());

let _ = crate::panic::catch_unwind(|| {
let guard = lock.lock().unwrap();
let _guard = MutexGuard::map::<(), _>(guard, |_| panic!());
});

match lock.try_lock() {
Ok(_) => panic!("panicking in a MutexGuard::map closure should poison the Mutex"),
Err(TryLockError::WouldBlock) => {
panic!("panicking in a MutexGuard::map closure should unlock the mutex")
}
Err(TryLockError::Poisoned(_)) => {}
}

let _ = crate::panic::catch_unwind(|| {
let guard = lock.lock().unwrap();
let _guard = MutexGuard::try_map::<(), _>(guard, |_| panic!());
});

match lock.try_lock() {
Ok(_) => panic!("panicking in a MutexGuard::try_map closure should poison the Mutex"),
Err(TryLockError::WouldBlock) => {
panic!("panicking in a MutexGuard::try_map closure should unlock the mutex")
}
Err(TryLockError::Poisoned(_)) => {}
}

let _ = crate::panic::catch_unwind(|| {
let guard = lock.lock().unwrap();
let guard = MutexGuard::map::<(), _>(guard, |val| val);
let _guard = MappedMutexGuard::map::<(), _>(guard, |_| panic!());
});

match lock.try_lock() {
Ok(_) => panic!("panicking in a MappedMutexGuard::map closure should poison the Mutex"),
Err(TryLockError::WouldBlock) => {
panic!("panicking in a MappedMutexGuard::map closure should unlock the mutex")
}
Err(TryLockError::Poisoned(_)) => {}
}

let _ = crate::panic::catch_unwind(|| {
let guard = lock.lock().unwrap();
let guard = MutexGuard::map::<(), _>(guard, |val| val);
let _guard = MappedMutexGuard::try_map::<(), _>(guard, |_| panic!());
});

match lock.try_lock() {
Ok(_) => panic!("panicking in a MappedMutexGuard::try_map closure should poison the Mutex"),
Err(TryLockError::WouldBlock) => {
panic!("panicking in a MappedMutexGuard::try_map closure should unlock the mutex")
}
Err(TryLockError::Poisoned(_)) => {}
}

drop(lock);
}
1 change: 1 addition & 0 deletions library/std/src/sync/poison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ impl Flag {
}
}

#[derive(Clone)]
pub struct Guard {
panicking: bool,
}
Expand Down
Loading
Loading