Auto merge of rust-lang#117107 - zachs18:mapped-mutex-guard, r=Amanieu

Implement `MappedMutexGuard`, `MappedRwLockReadGuard`, and `MappedRwLockWriteGuard`.

ACP: rust-lang/libs-team#260
Tracking issue: rust-lang#117108

<details> <summary> (Outdated) </summary>

`MutexState`/`RwLockState` structs

~~Having `sys::(Mutex|RwLock)` and `poison::Flag` as separate fields in the `Mutex`/`RwLock` would require `MappedMutexGuard`/`MappedRwLockWriteGuard` to hold an additional pointer, so I combined the two fields into a `MutexState`/`RwLockState` struct. This should not noticeably affect perf or layout, but requires an additional field projection when accessing the former `.inner` or `.poison` fields (now `.state.inner` and `.state.poison`).~~ If this is not desired, then `MappedMutexGuard`/`MappedRwLockWriteGuard` can instead hold separate pointers to the two fields.

</details>

The doc-comments are mostly copied from the existing `*Guard` doc-comments, with some parts from `lock_api::Mapped*Guard`'s doc-comments.

Unresolved question: Are more tests needed?

library/std/src/sync/mod.rs

@@ -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};
 

library/std/src/sync/mutex.rs

@@ -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;
 
@@ -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.
     ///
@@ -550,3 +594,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() }));
+        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),
+        }
+    }
+}

library/std/src/sync/mutex/tests.rs

@@ -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)>);
@@ -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
@@ -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);
+}

library/std/src/sync/poison.rs

@@ -78,6 +78,7 @@ impl Flag {
     }
 }
 
+#[derive(Clone)]
 pub struct Guard {
     #[cfg(panic = "unwind")]
     panicking: bool,