From 90d299b499b527e81010c163e3c086933b2297d0 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Jul 2025 13:34:04 +0200 Subject: [PATCH 1/7] clean up existing poison files --- library/std/src/sync/poison.rs | 6 +++--- library/std/src/sync/poison/condvar.rs | 2 +- library/std/src/sync/poison/mutex.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/sync/poison.rs b/library/std/src/sync/poison.rs index 0c05f152ef84c..b901a5701a488 100644 --- a/library/std/src/sync/poison.rs +++ b/library/std/src/sync/poison.rs @@ -13,7 +13,9 @@ //! depend on the primitive. See [#Overview] below. //! //! For the alternative implementations that do not employ poisoning, -//! see `std::sync::nonpoisoning`. +//! see [`std::sync::nonpoison`]. +//! +//! [`std::sync::nonpoison`]: crate::sync::nonpoison //! //! # Overview //! @@ -56,8 +58,6 @@ //! while it is locked exclusively (write mode). If a panic occurs in any reader, //! then the lock will not be poisoned. -// FIXME(sync_nonpoison) add links to sync::nonpoison to the doc comment above. - #[stable(feature = "rust1", since = "1.0.0")] pub use self::condvar::{Condvar, WaitTimeoutResult}; #[unstable(feature = "mapped_lock_guards", issue = "117108")] diff --git a/library/std/src/sync/poison/condvar.rs b/library/std/src/sync/poison/condvar.rs index 7f0f3f652bcb7..0e9d4233c6577 100644 --- a/library/std/src/sync/poison/condvar.rs +++ b/library/std/src/sync/poison/condvar.rs @@ -13,7 +13,7 @@ use crate::time::{Duration, Instant}; #[stable(feature = "wait_timeout", since = "1.5.0")] pub struct WaitTimeoutResult(bool); -// FIXME(sync_nonpoison): `WaitTimeoutResult` is actually poisoning-agnostic, it seems. +// FIXME(nonpoison_condvar): `WaitTimeoutResult` is actually poisoning-agnostic, it seems. // Should we take advantage of this fact? impl WaitTimeoutResult { /// Returns `true` if the wait was known to have timed out. diff --git a/library/std/src/sync/poison/mutex.rs b/library/std/src/sync/poison/mutex.rs index 30325be685c32..64744f18c746e 100644 --- a/library/std/src/sync/poison/mutex.rs +++ b/library/std/src/sync/poison/mutex.rs @@ -650,7 +650,7 @@ impl fmt::Debug for Mutex { d.field("data", &&**err.get_ref()); } Err(TryLockError::WouldBlock) => { - d.field("data", &format_args!("")); + d.field("data", &""); } } d.field("poisoned", &self.poison.get()); From 29658d7dbdbfe17013a15866c619f71089232559 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Jul 2025 13:47:49 +0200 Subject: [PATCH 2/7] add `nonpoison::mutex` implementation Adds the equivalent `nonpoison` types to the `poison::mutex` module. These types and implementations are gated under the `nonpoison_mutex` feature gate. Co-authored-by: Aandreba --- library/std/src/sync/mod.rs | 2 + library/std/src/sync/nonpoison.rs | 37 ++ library/std/src/sync/nonpoison/mutex.rs | 611 ++++++++++++++++++++++++ 3 files changed, 650 insertions(+) create mode 100644 library/std/src/sync/nonpoison.rs create mode 100644 library/std/src/sync/nonpoison/mutex.rs diff --git a/library/std/src/sync/mod.rs b/library/std/src/sync/mod.rs index e67b4f6f22f5a..6ef3bf25cf67c 100644 --- a/library/std/src/sync/mod.rs +++ b/library/std/src/sync/mod.rs @@ -225,6 +225,8 @@ pub use self::poison::{MappedMutexGuard, MappedRwLockReadGuard, MappedRwLockWrit pub mod mpmc; pub mod mpsc; +#[unstable(feature = "sync_nonpoison", issue = "134645")] +pub mod nonpoison; #[unstable(feature = "sync_poison_mod", issue = "134646")] pub mod poison; diff --git a/library/std/src/sync/nonpoison.rs b/library/std/src/sync/nonpoison.rs new file mode 100644 index 0000000000000..2bbf226dc2cde --- /dev/null +++ b/library/std/src/sync/nonpoison.rs @@ -0,0 +1,37 @@ +//! Non-poisoning synchronous locks. +//! +//! The difference from the locks in the [`poison`] module is that the locks in this module will not +//! become poisoned when a thread panics while holding a guard. +//! +//! [`poison`]: super::poison + +use crate::fmt; + +/// A type alias for the result of a nonblocking locking method. +#[unstable(feature = "sync_nonpoison", issue = "134645")] +pub type TryLockResult = Result; + +/// A lock could not be acquired at this time because the operation would otherwise block. +#[unstable(feature = "sync_nonpoison", issue = "134645")] +pub struct WouldBlock; + +#[unstable(feature = "sync_nonpoison", issue = "134645")] +impl fmt::Debug for WouldBlock { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + "WouldBlock".fmt(f) + } +} + +#[unstable(feature = "sync_nonpoison", issue = "134645")] +impl fmt::Display for WouldBlock { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + "try_lock failed because the operation would block".fmt(f) + } +} + +#[unstable(feature = "mapped_lock_guards", issue = "117108")] +pub use self::mutex::MappedMutexGuard; +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +pub use self::mutex::{Mutex, MutexGuard}; + +mod mutex; diff --git a/library/std/src/sync/nonpoison/mutex.rs b/library/std/src/sync/nonpoison/mutex.rs new file mode 100644 index 0000000000000..b6861c78f001d --- /dev/null +++ b/library/std/src/sync/nonpoison/mutex.rs @@ -0,0 +1,611 @@ +use crate::cell::UnsafeCell; +use crate::fmt; +use crate::marker::PhantomData; +use crate::mem::{self, ManuallyDrop}; +use crate::ops::{Deref, DerefMut}; +use crate::ptr::NonNull; +use crate::sync::nonpoison::{TryLockResult, WouldBlock}; +use crate::sys::sync as sys; + +/// A mutual exclusion primitive useful for protecting shared data that does not keep track of +/// lock poisoning. +/// +/// For more information about mutexes, check out the documentation for the poisoning variant of +/// this lock at [`poison::Mutex`]. +/// +/// [`poison::Mutex`]: crate::sync::poison::Mutex +/// +/// # Examples +/// +/// Note that this `Mutex` does **not** propagate threads that panic while holding the lock via +/// poisoning. If you need this functionality, see [`poison::Mutex`]. +/// +/// ``` +/// #![feature(nonpoison_mutex)] +/// +/// use std::thread; +/// use std::sync::{Arc, nonpoison::Mutex}; +/// +/// let mutex = Arc::new(Mutex::new(0u32)); +/// let mut handles = Vec::new(); +/// +/// for n in 0..10 { +/// let m = Arc::clone(&mutex); +/// let handle = thread::spawn(move || { +/// let mut guard = m.lock(); +/// *guard += 1; +/// panic!("panic from thread {n} {guard}") +/// }); +/// handles.push(handle); +/// } +/// +/// for h in handles { +/// let _ = h.join(); +/// } +/// +/// println!("Finished, locked {} times", mutex.lock()); +/// ``` +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +#[cfg_attr(not(test), rustc_diagnostic_item = "NonPoisonMutex")] +pub struct Mutex { + inner: sys::Mutex, + data: UnsafeCell, +} + +/// `T` must be `Send` for a [`Mutex`] to be `Send` because it is possible to acquire +/// the owned `T` from the `Mutex` via [`into_inner`]. +/// +/// [`into_inner`]: Mutex::into_inner +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +unsafe impl Send for Mutex {} + +/// `T` must be `Send` for [`Mutex`] to be `Sync`. +/// This ensures that the protected data can be accessed safely from multiple threads +/// without causing data races or other unsafe behavior. +/// +/// [`Mutex`] provides mutable access to `T` to one thread at a time. However, it's essential +/// for `T` to be `Send` because it's not safe for non-`Send` structures to be accessed in +/// this manner. For instance, consider [`Rc`], a non-atomic reference counted smart pointer, +/// which is not `Send`. With `Rc`, we can have multiple copies pointing to the same heap +/// allocation with a non-atomic reference count. If we were to use `Mutex>`, it would +/// only protect one instance of `Rc` from shared access, leaving other copies vulnerable +/// to potential data races. +/// +/// Also note that it is not necessary for `T` to be `Sync` as `&T` is only made available +/// to one thread at a time if `T` is not `Sync`. +/// +/// [`Rc`]: crate::rc::Rc +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +unsafe impl Sync for Mutex {} + +/// An RAII implementation of a "scoped lock" of a mutex. When this structure is +/// dropped (falls out of scope), the lock will be 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 [`lock`] and [`try_lock`] methods on +/// [`Mutex`]. +/// +/// [`lock`]: Mutex::lock +/// [`try_lock`]: Mutex::try_lock +#[must_use = "if unused the Mutex will immediately unlock"] +#[must_not_suspend = "holding a MutexGuard across suspend \ + points can cause deadlocks, delays, \ + and cause Futures to not implement `Send`"] +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +#[clippy::has_significant_drop] +#[cfg_attr(not(test), rustc_diagnostic_item = "NonPoisonMutexGuard")] +pub struct MutexGuard<'a, T: ?Sized + 'a> { + lock: &'a Mutex, +} + +/// A [`MutexGuard`] is not `Send` to maximize platform portablity. +/// +/// On platforms that use POSIX threads (commonly referred to as pthreads) there is a requirement to +/// release mutex locks on the same thread they were acquired. +/// For this reason, [`MutexGuard`] must not implement `Send` to prevent it being dropped from +/// another thread. +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl !Send for MutexGuard<'_, T> {} + +/// `T` must be `Sync` for a [`MutexGuard`] to be `Sync` +/// because it is possible to get a `&T` from `&MutexGuard` (via `Deref`). +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +unsafe impl Sync for MutexGuard<'_, T> {} + +// FIXME(nonpoison_condvar): Use this link instead: [`Condvar`]: crate::sync::nonpoison::Condvar +/// 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 [`filter_map`] methods on +/// [`MutexGuard`]. +/// +/// [`map`]: MutexGuard::map +/// [`filter_map`]: MutexGuard::filter_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")] +// #[unstable(feature = "nonpoison_mutex", issue = "134645")] +#[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, + inner: &'a sys::Mutex, + _variance: PhantomData<&'a mut T>, +} + +#[unstable(feature = "mapped_lock_guards", issue = "117108")] +// #[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl !Send for MappedMutexGuard<'_, T> {} +#[unstable(feature = "mapped_lock_guards", issue = "117108")] +// #[unstable(feature = "nonpoison_mutex", issue = "134645")] +unsafe impl Sync for MappedMutexGuard<'_, T> {} + +impl Mutex { + /// Creates a new mutex in an unlocked state ready for use. + /// + /// # Examples + /// + /// ``` + /// #![feature(nonpoison_mutex)] + /// + /// use std::sync::nonpoison::Mutex; + /// + /// let mutex = Mutex::new(0); + /// ``` + #[unstable(feature = "nonpoison_mutex", issue = "134645")] + #[inline] + pub const fn new(t: T) -> Mutex { + Mutex { inner: sys::Mutex::new(), data: UnsafeCell::new(t) } + } + + /// Returns the contained value by cloning it. + /// + /// # Examples + /// + /// ``` + /// #![feature(nonpoison_mutex)] + /// #![feature(lock_value_accessors)] + /// + /// use std::sync::nonpoison::Mutex; + /// + /// let mut mutex = Mutex::new(7); + /// + /// assert_eq!(mutex.get_cloned(), 7); + /// ``` + #[unstable(feature = "lock_value_accessors", issue = "133407")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn get_cloned(&self) -> T + where + T: Clone, + { + self.lock().clone() + } + + /// Sets the contained value. + /// + /// # Examples + /// + /// ``` + /// #![feature(nonpoison_mutex)] + /// #![feature(lock_value_accessors)] + /// + /// use std::sync::nonpoison::Mutex; + /// + /// let mut mutex = Mutex::new(7); + /// + /// assert_eq!(mutex.get_cloned(), 7); + /// mutex.set(11); + /// assert_eq!(mutex.get_cloned(), 11); + /// ``` + #[unstable(feature = "lock_value_accessors", issue = "133407")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn set(&self, value: T) { + if mem::needs_drop::() { + // If the contained value has a non-trivial destructor, we + // call that destructor after the lock has been released. + drop(self.replace(value)) + } else { + *self.lock() = value; + } + } + + /// Replaces the contained value with `value`, and returns the old contained value. + /// + /// # Examples + /// + /// ``` + /// #![feature(nonpoison_mutex)] + /// #![feature(lock_value_accessors)] + /// + /// use std::sync::nonpoison::Mutex; + /// + /// let mut mutex = Mutex::new(7); + /// + /// assert_eq!(mutex.replace(11), 7); + /// assert_eq!(mutex.get_cloned(), 11); + /// ``` + #[unstable(feature = "lock_value_accessors", issue = "133407")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn replace(&self, value: T) -> T { + let mut guard = self.lock(); + mem::replace(&mut *guard, value) + } +} + +impl Mutex { + /// Acquires a mutex, blocking the current thread until it is able to do so. + /// + /// This function will block the local thread until it is available to acquire + /// the mutex. Upon returning, the thread is the only thread with the lock + /// held. An RAII guard is returned to allow scoped unlock of the lock. When + /// the guard goes out of scope, the mutex will be unlocked. + /// + /// The exact behavior on locking a mutex in the thread which already holds + /// the lock is left unspecified. However, this function will not return on + /// the second call (it might panic or deadlock, for example). + /// + /// # Panics + /// + /// This function might panic when called if the lock is already held by + /// the current thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(nonpoison_mutex)] + /// + /// use std::sync::{Arc, nonpoison::Mutex}; + /// use std::thread; + /// + /// let mutex = Arc::new(Mutex::new(0)); + /// let c_mutex = Arc::clone(&mutex); + /// + /// thread::spawn(move || { + /// *c_mutex.lock() = 10; + /// }).join().expect("thread::spawn failed"); + /// assert_eq!(*mutex.lock(), 10); + /// ``` + #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn lock(&self) -> MutexGuard<'_, T> { + unsafe { + self.inner.lock(); + MutexGuard::new(self) + } + } + + /// Attempts to acquire this lock. + /// + /// This function does not block. If the lock could not be acquired at this time, then + /// [`WouldBlock`] is returned. Otherwise, an RAII guard is returned. + /// + /// The lock will be unlocked when the guard is dropped. + /// + /// # Errors + /// + /// If the mutex could not be acquired because it is already locked, then this call will return + /// the [`WouldBlock`] error. + /// + /// # Examples + /// + /// ``` + /// use std::sync::{Arc, Mutex}; + /// use std::thread; + /// + /// let mutex = Arc::new(Mutex::new(0)); + /// let c_mutex = Arc::clone(&mutex); + /// + /// thread::spawn(move || { + /// let mut lock = c_mutex.try_lock(); + /// if let Ok(ref mut mutex) = lock { + /// **mutex = 10; + /// } else { + /// println!("try_lock failed"); + /// } + /// }).join().expect("thread::spawn failed"); + /// assert_eq!(*mutex.lock().unwrap(), 10); + /// ``` + #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn try_lock(&self) -> TryLockResult> { + unsafe { if self.inner.try_lock() { Ok(MutexGuard::new(self)) } else { Err(WouldBlock) } } + } + + /// Consumes this mutex, returning the underlying data. + /// + /// # Examples + /// + /// ``` + /// #![feature(nonpoison_mutex)] + /// + /// use std::sync::nonpoison::Mutex; + /// + /// let mutex = Mutex::new(0); + /// assert_eq!(mutex.into_inner(), 0); + /// ``` + #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn into_inner(self) -> T + where + T: Sized, + { + self.data.into_inner() + } + + /// Returns a mutable reference to the underlying data. + /// + /// Since this call borrows the `Mutex` mutably, no actual locking needs to + /// take place -- the mutable borrow statically guarantees no locks exist. + /// + /// # Examples + /// + /// ``` + /// #![feature(nonpoison_mutex)] + /// + /// use std::sync::nonpoison::Mutex; + /// + /// let mut mutex = Mutex::new(0); + /// *mutex.get_mut() = 10; + /// assert_eq!(*mutex.lock(), 10); + /// ``` + #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn get_mut(&mut self) -> &mut T { + self.data.get_mut() + } + + /// Returns a raw pointer to the underlying data. + /// + /// The returned pointer is always non-null and properly aligned, but it is + /// the user's responsibility to ensure that any reads and writes through it + /// are properly synchronized to avoid data races, and that it is not read + /// or written through after the mutex is dropped. + #[unstable(feature = "mutex_data_ptr", issue = "140368")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn data_ptr(&self) -> *mut T { + self.data.get() + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl From for Mutex { + /// Creates a new mutex in an unlocked state ready for use. + /// This is equivalent to [`Mutex::new`]. + fn from(t: T) -> Self { + Mutex::new(t) + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl Default for Mutex { + /// Creates a `Mutex`, with the `Default` value for T. + fn default() -> Mutex { + Mutex::new(Default::default()) + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl fmt::Debug for Mutex { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut d = f.debug_struct("Mutex"); + match self.try_lock() { + Ok(guard) => { + d.field("data", &&*guard); + } + Err(WouldBlock) => { + d.field("data", &""); + } + } + d.finish_non_exhaustive() + } +} + +impl<'mutex, T: ?Sized> MutexGuard<'mutex, T> { + unsafe fn new(lock: &'mutex Mutex) -> MutexGuard<'mutex, T> { + return MutexGuard { lock }; + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl Deref for MutexGuard<'_, T> { + type Target = T; + + fn deref(&self) -> &T { + unsafe { &*self.lock.data.get() } + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl DerefMut for MutexGuard<'_, T> { + fn deref_mut(&mut self) -> &mut T { + unsafe { &mut *self.lock.data.get() } + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl Drop for MutexGuard<'_, T> { + #[inline] + fn drop(&mut self) { + unsafe { + self.lock.inner.unlock(); + } + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl fmt::Debug for MutexGuard<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&**self, f) + } +} + +#[unstable(feature = "nonpoison_mutex", issue = "134645")] +impl fmt::Display for MutexGuard<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (**self).fmt(f) + } +} + +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")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn map(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 `filter_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, _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::filter_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")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn filter_map(orig: Self, f: F) -> Result, 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 `filter_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, _variance: PhantomData }) + } + None => Err(orig), + } + } +} + +#[unstable(feature = "mapped_lock_guards", issue = "117108")] +impl Deref for MappedMutexGuard<'_, T> { + type Target = T; + + fn deref(&self) -> &T { + unsafe { self.data.as_ref() } + } +} + +#[unstable(feature = "mapped_lock_guards", issue = "117108")] +impl DerefMut for MappedMutexGuard<'_, T> { + fn deref_mut(&mut self) -> &mut T { + unsafe { self.data.as_mut() } + } +} + +#[unstable(feature = "mapped_lock_guards", issue = "117108")] +impl Drop for MappedMutexGuard<'_, T> { + #[inline] + fn drop(&mut self) { + unsafe { + self.inner.unlock(); + } + } +} + +#[unstable(feature = "mapped_lock_guards", issue = "117108")] +impl 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 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")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn map(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 `filter_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, _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::filter_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")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn filter_map(mut orig: Self, f: F) -> Result, 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 `filter_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, _variance: PhantomData }) + } + None => Err(orig), + } + } +} From ff7835f32afef252ffeb00df0582e3e0e27fc74c Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Jul 2025 14:04:32 +0200 Subject: [PATCH 3/7] reorder mutex tests This commit simply helps discern the actual changes needed to test both poison and nonpoison locks. --- library/std/tests/sync/mutex.rs | 302 +++++++++++++++++--------------- 1 file changed, 158 insertions(+), 144 deletions(-) diff --git a/library/std/tests/sync/mutex.rs b/library/std/tests/sync/mutex.rs index ac82914d6de46..6f06d3385d1aa 100644 --- a/library/std/tests/sync/mutex.rs +++ b/library/std/tests/sync/mutex.rs @@ -6,28 +6,9 @@ use std::sync::mpsc::channel; use std::sync::{Arc, Condvar, MappedMutexGuard, Mutex, MutexGuard, TryLockError}; use std::{hint, mem, thread}; -struct Packet(Arc<(Mutex, Condvar)>); - -#[derive(Eq, PartialEq, Debug)] -struct NonCopy(i32); - -#[derive(Eq, PartialEq, Debug)] -struct NonCopyNeedsDrop(i32); - -impl Drop for NonCopyNeedsDrop { - fn drop(&mut self) { - hint::black_box(()); - } -} - -#[test] -fn test_needs_drop() { - assert!(!mem::needs_drop::()); - assert!(mem::needs_drop::()); -} - -#[derive(Clone, Eq, PartialEq, Debug)] -struct Cloneable(i32); +//////////////////////////////////////////////////////////////////////////////////////////////////// +// Nonpoison & Poison Tests +//////////////////////////////////////////////////////////////////////////////////////////////////// #[test] fn smoke() { @@ -78,19 +59,22 @@ fn try_lock() { *m.try_lock().unwrap() = (); } -fn new_poisoned_mutex(value: T) -> Mutex { - let mutex = Mutex::new(value); - - let catch_unwind_result = panic::catch_unwind(AssertUnwindSafe(|| { - let _guard = mutex.lock().unwrap(); +#[derive(Eq, PartialEq, Debug)] +struct NonCopy(i32); - panic!("test panic to poison mutex"); - })); +#[derive(Eq, PartialEq, Debug)] +struct NonCopyNeedsDrop(i32); - assert!(catch_unwind_result.is_err()); - assert!(mutex.is_poisoned()); +impl Drop for NonCopyNeedsDrop { + fn drop(&mut self) { + hint::black_box(()); + } +} - mutex +#[test] +fn test_needs_drop() { + assert!(!mem::needs_drop::()); + assert!(mem::needs_drop::()); } #[test] @@ -117,6 +101,143 @@ fn test_into_inner_drop() { assert_eq!(num_drops.load(Ordering::SeqCst), 1); } +#[test] +fn test_get_mut() { + let mut m = Mutex::new(NonCopy(10)); + *m.get_mut().unwrap() = NonCopy(20); + assert_eq!(m.into_inner().unwrap(), NonCopy(20)); +} + +#[test] +fn test_get_cloned() { + #[derive(Clone, Eq, PartialEq, Debug)] + struct Cloneable(i32); + + let m = Mutex::new(Cloneable(10)); + + assert_eq!(m.get_cloned().unwrap(), Cloneable(10)); +} + +#[test] +fn test_set() { + fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) + where + T: Debug + Eq, + { + let m = Mutex::new(init()); + + assert_eq!(*m.lock().unwrap(), init()); + m.set(value()).unwrap(); + assert_eq!(*m.lock().unwrap(), value()); + } + + inner(|| NonCopy(10), || NonCopy(20)); + inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); +} + +#[test] +fn test_replace() { + fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) + where + T: Debug + Eq, + { + let m = Mutex::new(init()); + + assert_eq!(*m.lock().unwrap(), init()); + assert_eq!(m.replace(value()).unwrap(), init()); + assert_eq!(*m.lock().unwrap(), value()); + } + + inner(|| NonCopy(10), || NonCopy(20)); + inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); +} + +#[test] +fn test_mutex_arc_condvar() { + struct Packet(Arc<(Mutex, Condvar)>); + + let packet = Packet(Arc::new((Mutex::new(false), Condvar::new()))); + let packet2 = Packet(packet.0.clone()); + let (tx, rx) = channel(); + let _t = thread::spawn(move || { + // wait until parent gets in + rx.recv().unwrap(); + let &(ref lock, ref cvar) = &*packet2.0; + let mut lock = lock.lock().unwrap(); + *lock = true; + cvar.notify_one(); + }); + + let &(ref lock, ref cvar) = &*packet.0; + let mut lock = lock.lock().unwrap(); + tx.send(()).unwrap(); + assert!(!*lock); + while !*lock { + lock = cvar.wait(lock).unwrap(); + } +} + +#[test] +fn test_mutex_arc_nested() { + // Tests nested mutexes and access + // to underlying data. + let arc = Arc::new(Mutex::new(1)); + let arc2 = Arc::new(Mutex::new(arc)); + let (tx, rx) = channel(); + let _t = thread::spawn(move || { + let lock = arc2.lock().unwrap(); + let lock2 = lock.lock().unwrap(); + assert_eq!(*lock2, 1); + tx.send(()).unwrap(); + }); + rx.recv().unwrap(); +} + +#[test] +fn test_mutex_unsized() { + let mutex: &Mutex<[i32]> = &Mutex::new([1, 2, 3]); + { + let b = &mut *mutex.lock().unwrap(); + b[0] = 4; + b[2] = 5; + } + 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]); +} + +//////////////////////////////////////////////////////////////////////////////////////////////////// +// Poison Tests +//////////////////////////////////////////////////////////////////////////////////////////////////// + +/// Creates a mutex that is immediately poisoned. +fn new_poisoned_mutex(value: T) -> Mutex { + let mutex = Mutex::new(value); + + let catch_unwind_result = panic::catch_unwind(AssertUnwindSafe(|| { + let _guard = mutex.lock().unwrap(); + + panic!("test panic to poison mutex"); + })); + + assert!(catch_unwind_result.is_err()); + assert!(mutex.is_poisoned()); + + mutex +} + #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_into_inner_poison() { @@ -128,16 +249,12 @@ fn test_into_inner_poison() { } } -#[test] -fn test_get_cloned() { - let m = Mutex::new(Cloneable(10)); - - assert_eq!(m.get_cloned().unwrap(), Cloneable(10)); -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_get_cloned_poison() { + #[derive(Clone, Eq, PartialEq, Debug)] + struct Cloneable(i32); + let m = new_poisoned_mutex(Cloneable(10)); match m.get_cloned() { @@ -146,13 +263,6 @@ fn test_get_cloned_poison() { } } -#[test] -fn test_get_mut() { - let mut m = Mutex::new(NonCopy(10)); - *m.get_mut().unwrap() = NonCopy(20); - assert_eq!(m.into_inner().unwrap(), NonCopy(20)); -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_get_mut_poison() { @@ -164,23 +274,6 @@ fn test_get_mut_poison() { } } -#[test] -fn test_set() { - fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) - where - T: Debug + Eq, - { - let m = Mutex::new(init()); - - assert_eq!(*m.lock().unwrap(), init()); - m.set(value()).unwrap(); - assert_eq!(*m.lock().unwrap(), value()); - } - - inner(|| NonCopy(10), || NonCopy(20)); - inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_set_poison() { @@ -203,23 +296,6 @@ fn test_set_poison() { inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); } -#[test] -fn test_replace() { - fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) - where - T: Debug + Eq, - { - let m = Mutex::new(init()); - - assert_eq!(*m.lock().unwrap(), init()); - assert_eq!(m.replace(value()).unwrap(), init()); - assert_eq!(*m.lock().unwrap(), value()); - } - - inner(|| NonCopy(10), || NonCopy(20)); - inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_replace_poison() { @@ -242,32 +318,11 @@ fn test_replace_poison() { inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); } -#[test] -fn test_mutex_arc_condvar() { - let packet = Packet(Arc::new((Mutex::new(false), Condvar::new()))); - let packet2 = Packet(packet.0.clone()); - let (tx, rx) = channel(); - let _t = thread::spawn(move || { - // wait until parent gets in - rx.recv().unwrap(); - let &(ref lock, ref cvar) = &*packet2.0; - let mut lock = lock.lock().unwrap(); - *lock = true; - cvar.notify_one(); - }); - - let &(ref lock, ref cvar) = &*packet.0; - let mut lock = lock.lock().unwrap(); - tx.send(()).unwrap(); - assert!(!*lock); - while !*lock { - lock = cvar.wait(lock).unwrap(); - } -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_arc_condvar_poison() { + struct Packet(Arc<(Mutex, Condvar)>); + let packet = Packet(Arc::new((Mutex::new(1), Condvar::new()))); let packet2 = Packet(packet.0.clone()); let (tx, rx) = channel(); @@ -326,22 +381,6 @@ fn test_mutex_arc_poison_mapped() { assert!(arc.is_poisoned()); } -#[test] -fn test_mutex_arc_nested() { - // Tests nested mutexes and access - // to underlying data. - let arc = Arc::new(Mutex::new(1)); - let arc2 = Arc::new(Mutex::new(arc)); - let (tx, rx) = channel(); - let _t = thread::spawn(move || { - let lock = arc2.lock().unwrap(); - let lock2 = lock.lock().unwrap(); - assert_eq!(*lock2, 1); - tx.send(()).unwrap(); - }); - rx.recv().unwrap(); -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_mutex_arc_access_in_unwind() { @@ -364,31 +403,6 @@ fn test_mutex_arc_access_in_unwind() { assert_eq!(*lock, 2); } -#[test] -fn test_mutex_unsized() { - let mutex: &Mutex<[i32]> = &Mutex::new([1, 2, 3]); - { - let b = &mut *mutex.lock().unwrap(); - b[0] = 4; - b[2] = 5; - } - 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] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn panic_while_mapping_unlocked_poison() { From 2fc79fd05799f82db7db24b3d39e8f63568798a8 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Jul 2025 14:09:27 +0200 Subject: [PATCH 4/7] add nonpoison and poison mutex tests Adds tests for the `nonpoison::Mutex` variant by using a macro to duplicate the existing `poison` tests. Note that all of the tests here are adapted from the existing `poison` tests. --- library/std/tests/sync/lib.rs | 55 +++++ library/std/tests/sync/mutex.rs | 351 +++++++++++++++++++------------- 2 files changed, 260 insertions(+), 146 deletions(-) diff --git a/library/std/tests/sync/lib.rs b/library/std/tests/sync/lib.rs index 51190f0894fb7..93433a3b858c3 100644 --- a/library/std/tests/sync/lib.rs +++ b/library/std/tests/sync/lib.rs @@ -6,7 +6,10 @@ #![feature(reentrant_lock)] #![feature(rwlock_downgrade)] #![feature(std_internals)] +#![feature(sync_nonpoison)] +#![feature(nonpoison_mutex)] #![allow(internal_features)] +#![feature(macro_metavar_expr_concat)] // For concatenating identifiers in macros. mod barrier; mod condvar; @@ -29,3 +32,55 @@ mod rwlock; #[path = "../common/mod.rs"] mod common; + +// A macro that generates two test cases for both the poison and nonpoison locks. +// +// To write a test that tests both `poison` and `nonpoison` locks, import any of the types +// under both `poison` and `nonpoison` using the module name `locks` instead. For example, write +// `use locks::Mutex;` instead of `use std::sync::poiosn::Mutex`. This will import the correct type +// for each test variant. +// +// Write a test as normal in the `test_body`, but instead of calling `unwrap` on `poison` methods +// that return a `LockResult` or similar, call the macro `maybe_unwrap!(...)` on the result. +// +// For example, call `maybe_unwrap!(mutex.lock())` instead of `mutex.lock().unwrap()` or +// `maybe_unwrap!(rwlock.read())` instead of `rwlock.read().unwrap()`. +// +// For the `poison` types, `maybe_unwrap!` will simply unwrap the `Result` (usually this is a form +// of `LockResult`, but it could also be other kinds of results). For the `nonpoison` types, it is a +// no-op. +// +// The `poison` test will have the same `name`, but with a suffix of `_unwrap_poisoned`. +#[macro_export] +macro_rules! nonpoison_and_poison_unwrap_test { + ( + name: $name:ident, + test_body: {$($test_body:tt)*} + ) => { + // Creates the nonpoison test. + #[test] + fn $name() { + use ::std::sync::nonpoison as locks; + + #[allow(unused_macros)] + macro_rules! maybe_unwrap { + ($e:expr) => { $e }; + } + + $($test_body)* + } + + // Creates the poison test with the suffix `_unwrap_poisoned`. + #[test] + fn ${concat($name, _unwrap_poisoned)}() { + use ::std::sync::poison as locks; + + #[allow(unused_macros)] + macro_rules! maybe_unwrap { + ($e:expr) => { ::std::result::Result::unwrap($e) }; + } + + $($test_body)* + } + } +} diff --git a/library/std/tests/sync/mutex.rs b/library/std/tests/sync/mutex.rs index 6f06d3385d1aa..69214fda508c0 100644 --- a/library/std/tests/sync/mutex.rs +++ b/library/std/tests/sync/mutex.rs @@ -10,54 +10,68 @@ use std::{hint, mem, thread}; // Nonpoison & Poison Tests //////////////////////////////////////////////////////////////////////////////////////////////////// -#[test] -fn smoke() { - let m = Mutex::new(()); - drop(m.lock().unwrap()); - drop(m.lock().unwrap()); -} +use super::nonpoison_and_poison_unwrap_test; -#[test] -fn lots_and_lots() { - const J: u32 = 1000; - const K: u32 = 3; +nonpoison_and_poison_unwrap_test!( + name: smoke, + test_body: { + use locks::Mutex; + + let m = Mutex::new(()); + drop(maybe_unwrap!(m.lock())); + drop(maybe_unwrap!(m.lock())); + } +); + +nonpoison_and_poison_unwrap_test!( + name: lots_and_lots, + test_body: { + use locks::Mutex; - let m = Arc::new(Mutex::new(0)); + const J: u32 = 1000; + const K: u32 = 3; - fn inc(m: &Mutex) { - for _ in 0..J { - *m.lock().unwrap() += 1; + let m = Arc::new(Mutex::new(0)); + + fn inc(m: &Mutex) { + for _ in 0..J { + *maybe_unwrap!(m.lock()) += 1; + } } - } - let (tx, rx) = channel(); - for _ in 0..K { - let tx2 = tx.clone(); - let m2 = m.clone(); - thread::spawn(move || { - inc(&m2); - tx2.send(()).unwrap(); - }); - let tx2 = tx.clone(); - let m2 = m.clone(); - thread::spawn(move || { - inc(&m2); - tx2.send(()).unwrap(); - }); - } + let (tx, rx) = channel(); + for _ in 0..K { + let tx2 = tx.clone(); + let m2 = m.clone(); + thread::spawn(move || { + inc(&m2); + tx2.send(()).unwrap(); + }); + let tx2 = tx.clone(); + let m2 = m.clone(); + thread::spawn(move || { + inc(&m2); + tx2.send(()).unwrap(); + }); + } - drop(tx); - for _ in 0..2 * K { - rx.recv().unwrap(); + drop(tx); + for _ in 0..2 * K { + rx.recv().unwrap(); + } + assert_eq!(*maybe_unwrap!(m.lock()), J * K * 2); } - assert_eq!(*m.lock().unwrap(), J * K * 2); -} +); -#[test] -fn try_lock() { - let m = Mutex::new(()); - *m.try_lock().unwrap() = (); -} +nonpoison_and_poison_unwrap_test!( + name: try_lock, + test_body: { + use locks::Mutex; + + let m = Mutex::new(()); + *m.try_lock().unwrap() = (); + } +); #[derive(Eq, PartialEq, Debug)] struct NonCopy(i32); @@ -77,146 +91,191 @@ fn test_needs_drop() { assert!(mem::needs_drop::()); } -#[test] -fn test_into_inner() { - let m = Mutex::new(NonCopy(10)); - assert_eq!(m.into_inner().unwrap(), NonCopy(10)); -} +nonpoison_and_poison_unwrap_test!( + name: test_into_inner, + test_body: { + use locks::Mutex; -#[test] -fn test_into_inner_drop() { - struct Foo(Arc); - impl Drop for Foo { - fn drop(&mut self) { - self.0.fetch_add(1, Ordering::SeqCst); - } + let m = Mutex::new(NonCopy(10)); + assert_eq!(maybe_unwrap!(m.into_inner()), NonCopy(10)); } - let num_drops = Arc::new(AtomicUsize::new(0)); - let m = Mutex::new(Foo(num_drops.clone())); - assert_eq!(num_drops.load(Ordering::SeqCst), 0); - { - let _inner = m.into_inner().unwrap(); +); + +nonpoison_and_poison_unwrap_test!( + name: test_into_inner_drop, + test_body: { + use locks::Mutex; + + struct Foo(Arc); + impl Drop for Foo { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + let num_drops = Arc::new(AtomicUsize::new(0)); + let m = Mutex::new(Foo(num_drops.clone())); assert_eq!(num_drops.load(Ordering::SeqCst), 0); + { + let _inner = maybe_unwrap!(m.into_inner()); + assert_eq!(num_drops.load(Ordering::SeqCst), 0); + } + assert_eq!(num_drops.load(Ordering::SeqCst), 1); } - assert_eq!(num_drops.load(Ordering::SeqCst), 1); -} +); -#[test] -fn test_get_mut() { - let mut m = Mutex::new(NonCopy(10)); - *m.get_mut().unwrap() = NonCopy(20); - assert_eq!(m.into_inner().unwrap(), NonCopy(20)); -} +nonpoison_and_poison_unwrap_test!( + name: test_get_mut, + test_body: { + use locks::Mutex; -#[test] -fn test_get_cloned() { - #[derive(Clone, Eq, PartialEq, Debug)] - struct Cloneable(i32); + let mut m = Mutex::new(NonCopy(10)); + *maybe_unwrap!(m.get_mut()) = NonCopy(20); + assert_eq!(maybe_unwrap!(m.into_inner()), NonCopy(20)); + } +); - let m = Mutex::new(Cloneable(10)); +nonpoison_and_poison_unwrap_test!( + name: test_get_cloned, + test_body: { + use locks::Mutex; - assert_eq!(m.get_cloned().unwrap(), Cloneable(10)); -} + #[derive(Clone, Eq, PartialEq, Debug)] + struct Cloneable(i32); -#[test] -fn test_set() { - fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) - where - T: Debug + Eq, - { - let m = Mutex::new(init()); + let m = Mutex::new(Cloneable(10)); - assert_eq!(*m.lock().unwrap(), init()); - m.set(value()).unwrap(); - assert_eq!(*m.lock().unwrap(), value()); + assert_eq!(maybe_unwrap!(m.get_cloned()), Cloneable(10)); } +); + +nonpoison_and_poison_unwrap_test!( + name: test_set, + test_body: { + use locks::Mutex; + + fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) + where + T: Debug + Eq, + { + let m = Mutex::new(init()); + + assert_eq!(*maybe_unwrap!(m.lock()), init()); + maybe_unwrap!(m.set(value())); + assert_eq!(*maybe_unwrap!(m.lock()), value()); + } - inner(|| NonCopy(10), || NonCopy(20)); - inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); -} - -#[test] -fn test_replace() { - fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) - where - T: Debug + Eq, - { - let m = Mutex::new(init()); - - assert_eq!(*m.lock().unwrap(), init()); - assert_eq!(m.replace(value()).unwrap(), init()); - assert_eq!(*m.lock().unwrap(), value()); + inner(|| NonCopy(10), || NonCopy(20)); + inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); } +); + +nonpoison_and_poison_unwrap_test!( + name: test_replace, + test_body: { + use locks::Mutex; + + fn inner(mut init: impl FnMut() -> T, mut value: impl FnMut() -> T) + where + T: Debug + Eq, + { + let m = Mutex::new(init()); + + assert_eq!(*maybe_unwrap!(m.lock()), init()); + assert_eq!(maybe_unwrap!(m.replace(value())), init()); + assert_eq!(*maybe_unwrap!(m.lock()), value()); + } - inner(|| NonCopy(10), || NonCopy(20)); - inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); -} + inner(|| NonCopy(10), || NonCopy(20)); + inner(|| NonCopyNeedsDrop(10), || NonCopyNeedsDrop(20)); + } +); +// FIXME(nonpoison_condvar): Move this to the `condvar.rs` test file once `nonpoison::condvar` gets +// implemented. #[test] fn test_mutex_arc_condvar() { struct Packet(Arc<(Mutex, Condvar)>); let packet = Packet(Arc::new((Mutex::new(false), Condvar::new()))); let packet2 = Packet(packet.0.clone()); + let (tx, rx) = channel(); + let _t = thread::spawn(move || { - // wait until parent gets in + // Wait until our parent has taken the lock. rx.recv().unwrap(); let &(ref lock, ref cvar) = &*packet2.0; - let mut lock = lock.lock().unwrap(); - *lock = true; + + // Set the data to `true` and wake up our parent. + let mut guard = lock.lock().unwrap(); + *guard = true; cvar.notify_one(); }); let &(ref lock, ref cvar) = &*packet.0; - let mut lock = lock.lock().unwrap(); + let mut guard = lock.lock().unwrap(); + // Wake up our child. tx.send(()).unwrap(); - assert!(!*lock); - while !*lock { - lock = cvar.wait(lock).unwrap(); - } -} - -#[test] -fn test_mutex_arc_nested() { - // Tests nested mutexes and access - // to underlying data. - let arc = Arc::new(Mutex::new(1)); - let arc2 = Arc::new(Mutex::new(arc)); - let (tx, rx) = channel(); - let _t = thread::spawn(move || { - let lock = arc2.lock().unwrap(); - let lock2 = lock.lock().unwrap(); - assert_eq!(*lock2, 1); - tx.send(()).unwrap(); - }); - rx.recv().unwrap(); -} -#[test] -fn test_mutex_unsized() { - let mutex: &Mutex<[i32]> = &Mutex::new([1, 2, 3]); - { - let b = &mut *mutex.lock().unwrap(); - b[0] = 4; - b[2] = 5; + // Wait until our child has set the data to `true`. + assert!(!*guard); + while !*guard { + guard = cvar.wait(guard).unwrap(); } - 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]); -} +nonpoison_and_poison_unwrap_test!( + name: test_mutex_arc_nested, + test_body: { + use locks::Mutex; + + // Tests nested mutexes and access + // to underlying data. + let arc = Arc::new(Mutex::new(1)); + let arc2 = Arc::new(Mutex::new(arc)); + let (tx, rx) = channel(); + let _t = thread::spawn(move || { + let lock = maybe_unwrap!(arc2.lock()); + let lock2 = maybe_unwrap!(lock.lock()); + assert_eq!(*lock2, 1); + tx.send(()).unwrap(); + }); + rx.recv().unwrap(); + } +); + +nonpoison_and_poison_unwrap_test!( + name: test_mutex_unsized, + test_body: { + use locks::Mutex; + + let mutex: &Mutex<[i32]> = &Mutex::new([1, 2, 3]); + { + let b = &mut *maybe_unwrap!(mutex.lock()); + b[0] = 4; + b[2] = 5; + } + let comp: &[i32] = &[4, 2, 5]; + assert_eq!(&*maybe_unwrap!(mutex.lock()), comp); + } +); + +nonpoison_and_poison_unwrap_test!( + name: test_mapping_mapped_guard, + test_body: { + use locks::{Mutex, MutexGuard, MappedMutexGuard}; + + let arr = [0; 4]; + let lock = Mutex::new(arr); + let guard = maybe_unwrap!(lock.lock()); + 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!(*maybe_unwrap!(lock.lock()), [0, 42, 0, 0]); + } +); //////////////////////////////////////////////////////////////////////////////////////////////////// // Poison Tests From 7b0a316e332abdaa96afec2210b4c89a79b82477 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Jul 2025 14:59:35 +0200 Subject: [PATCH 5/7] add extra drop and unwind tests --- library/std/tests/sync/mutex.rs | 103 +++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 23 deletions(-) diff --git a/library/std/tests/sync/mutex.rs b/library/std/tests/sync/mutex.rs index 69214fda508c0..3c0ffbe52e421 100644 --- a/library/std/tests/sync/mutex.rs +++ b/library/std/tests/sync/mutex.rs @@ -9,7 +9,6 @@ use std::{hint, mem, thread}; //////////////////////////////////////////////////////////////////////////////////////////////////// // Nonpoison & Poison Tests //////////////////////////////////////////////////////////////////////////////////////////////////// - use super::nonpoison_and_poison_unwrap_test; nonpoison_and_poison_unwrap_test!( @@ -112,6 +111,7 @@ nonpoison_and_poison_unwrap_test!( self.0.fetch_add(1, Ordering::SeqCst); } } + let num_drops = Arc::new(AtomicUsize::new(0)); let m = Mutex::new(Foo(num_drops.clone())); assert_eq!(num_drops.load(Ordering::SeqCst), 0); @@ -169,6 +169,28 @@ nonpoison_and_poison_unwrap_test!( } ); +nonpoison_and_poison_unwrap_test!( + name: test_set_drop, + test_body: { + use locks::Mutex; + + struct Foo(Arc); + impl Drop for Foo { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let num_drops = Arc::new(AtomicUsize::new(0)); + let m = Mutex::new(Foo(num_drops.clone())); + assert_eq!(num_drops.load(Ordering::SeqCst), 0); + + let different = Foo(Arc::new(AtomicUsize::new(42))); + maybe_unwrap!(m.set(different)); + assert_eq!(num_drops.load(Ordering::SeqCst), 1); + } +); + nonpoison_and_poison_unwrap_test!( name: test_replace, test_body: { @@ -277,6 +299,63 @@ nonpoison_and_poison_unwrap_test!( } ); +#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] +nonpoison_and_poison_unwrap_test!( + name: test_panics, + test_body: { + use locks::Mutex; + + let mutex = Mutex::new(42); + + let catch_unwind_result1 = panic::catch_unwind(AssertUnwindSafe(|| { + let _guard1 = maybe_unwrap!(mutex.lock()); + + panic!("test panic with mutex once"); + })); + assert!(catch_unwind_result1.is_err()); + + let catch_unwind_result2 = panic::catch_unwind(AssertUnwindSafe(|| { + let _guard2 = maybe_unwrap!(mutex.lock()); + + panic!("test panic with mutex twice"); + })); + assert!(catch_unwind_result2.is_err()); + + let catch_unwind_result3 = panic::catch_unwind(AssertUnwindSafe(|| { + let _guard3 = maybe_unwrap!(mutex.lock()); + + panic!("test panic with mutex thrice"); + })); + assert!(catch_unwind_result3.is_err()); + } +); + +#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] +nonpoison_and_poison_unwrap_test!( + name: test_mutex_arc_access_in_unwind, + test_body: { + use locks::Mutex; + + let arc = Arc::new(Mutex::new(1)); + let arc2 = arc.clone(); + let _ = thread::spawn(move || -> () { + struct Unwinder { + i: Arc>, + } + impl Drop for Unwinder { + fn drop(&mut self) { + *maybe_unwrap!(self.i.lock()) += 1; + } + } + let _u = Unwinder { i: arc2 }; + panic!(); + }) + .join(); + let lock = maybe_unwrap!(arc.lock()); + assert_eq!(*lock, 2); + } +); + //////////////////////////////////////////////////////////////////////////////////////////////////// // Poison Tests //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -440,28 +519,6 @@ fn test_mutex_arc_poison_mapped() { assert!(arc.is_poisoned()); } -#[test] -#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] -fn test_mutex_arc_access_in_unwind() { - let arc = Arc::new(Mutex::new(1)); - let arc2 = arc.clone(); - let _ = thread::spawn(move || -> () { - struct Unwinder { - i: Arc>, - } - impl Drop for Unwinder { - fn drop(&mut self) { - *self.i.lock().unwrap() += 1; - } - } - let _u = Unwinder { i: arc2 }; - panic!(); - }) - .join(); - let lock = arc.lock().unwrap(); - assert_eq!(*lock, 2); -} - #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn panic_while_mapping_unlocked_poison() { From b4efb1cc7a8b423cc8e21212ebe9b555e6d0e0f1 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Jul 2025 13:25:52 +0200 Subject: [PATCH 6/7] Update UI tests Blesses the ui tests that now have a name conflicts (because these types no longer have unique names). The full path distinguishes the different types. Co-authored-by: Trevor Gross --- .../issue-64130-non-send-future-diags.stderr | 4 ++-- tests/ui/async-await/issue-71137.stderr | 4 ++-- tests/ui/async-await/issues/issue-67893.rs | 2 +- tests/ui/async-await/issues/issue-67893.stderr | 6 +++--- tests/ui/lint/must_not_suspend/mutex.rs | 2 +- tests/ui/lint/must_not_suspend/mutex.stderr | 2 +- .../privacy/private-field-access-in-mutex-54062.rs | 2 +- .../private-field-access-in-mutex-54062.stderr | 2 +- tests/ui/suggestions/inner_type.fixed | 2 +- tests/ui/suggestions/inner_type.rs | 2 +- tests/ui/suggestions/inner_type.stderr | 4 ++-- tests/ui/sync/mutexguard-sync.stderr | 2 +- .../const-traits/span-bug-issue-121418.stderr | 6 +++--- tests/ui/typeck/assign-non-lval-derefmut.fixed | 4 ++-- tests/ui/typeck/assign-non-lval-derefmut.rs | 4 ++-- tests/ui/typeck/assign-non-lval-derefmut.stderr | 14 +++++++------- tests/ui/typeck/deref-multi.stderr | 2 +- 17 files changed, 32 insertions(+), 32 deletions(-) diff --git a/tests/ui/async-await/issue-64130-non-send-future-diags.stderr b/tests/ui/async-await/issue-64130-non-send-future-diags.stderr index d28807e223bef..beaf8e9c96d85 100644 --- a/tests/ui/async-await/issue-64130-non-send-future-diags.stderr +++ b/tests/ui/async-await/issue-64130-non-send-future-diags.stderr @@ -4,12 +4,12 @@ error: future cannot be sent between threads safely LL | is_send(foo()); | ^^^^^ future returned by `foo` is not `Send` | - = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, u32>` + = help: within `impl Future`, the trait `Send` is not implemented for `std::sync::MutexGuard<'_, u32>` note: future is not `Send` as this value is used across an await --> $DIR/issue-64130-non-send-future-diags.rs:17:11 | LL | let g = x.lock().unwrap(); - | - has type `MutexGuard<'_, u32>` which is not `Send` + | - has type `std::sync::MutexGuard<'_, u32>` which is not `Send` LL | baz().await; | ^^^^^ await occurs here, with `g` maybe used later note: required by a bound in `is_send` diff --git a/tests/ui/async-await/issue-71137.stderr b/tests/ui/async-await/issue-71137.stderr index 8739c22a31048..d567e3f2063de 100644 --- a/tests/ui/async-await/issue-71137.stderr +++ b/tests/ui/async-await/issue-71137.stderr @@ -4,12 +4,12 @@ error: future cannot be sent between threads safely LL | fake_spawn(wrong_mutex()); | ^^^^^^^^^^^^^ future returned by `wrong_mutex` is not `Send` | - = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, i32>` + = help: within `impl Future`, the trait `Send` is not implemented for `std::sync::MutexGuard<'_, i32>` note: future is not `Send` as this value is used across an await --> $DIR/issue-71137.rs:14:26 | LL | let mut guard = m.lock().unwrap(); - | --------- has type `MutexGuard<'_, i32>` which is not `Send` + | --------- has type `std::sync::MutexGuard<'_, i32>` which is not `Send` LL | (async { "right"; }).await; | ^^^^^ await occurs here, with `mut guard` maybe used later note: required by a bound in `fake_spawn` diff --git a/tests/ui/async-await/issues/issue-67893.rs b/tests/ui/async-await/issues/issue-67893.rs index 73cce38c94a0a..2020abe7a5a90 100644 --- a/tests/ui/async-await/issues/issue-67893.rs +++ b/tests/ui/async-await/issues/issue-67893.rs @@ -7,5 +7,5 @@ fn g(_: impl Send) {} fn main() { g(issue_67893::run()) - //~^ ERROR `MutexGuard<'_, ()>` cannot be sent between threads safely + //~^ ERROR `std::sync::MutexGuard<'_, ()>` cannot be sent between threads safely } diff --git a/tests/ui/async-await/issues/issue-67893.stderr b/tests/ui/async-await/issues/issue-67893.stderr index c01237255b89b..34f28dd53c7b9 100644 --- a/tests/ui/async-await/issues/issue-67893.stderr +++ b/tests/ui/async-await/issues/issue-67893.stderr @@ -1,8 +1,8 @@ -error[E0277]: `MutexGuard<'_, ()>` cannot be sent between threads safely +error[E0277]: `std::sync::MutexGuard<'_, ()>` cannot be sent between threads safely --> $DIR/issue-67893.rs:9:7 | LL | g(issue_67893::run()) - | - ^^^^^^^^^^^^^^^^^^ `MutexGuard<'_, ()>` cannot be sent between threads safely + | - ^^^^^^^^^^^^^^^^^^ `std::sync::MutexGuard<'_, ()>` cannot be sent between threads safely | | | required by a bound introduced by this call | @@ -11,7 +11,7 @@ LL | g(issue_67893::run()) LL | pub async fn run() { | ------------------ within this `impl Future` | - = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, ()>` + = help: within `impl Future`, the trait `Send` is not implemented for `std::sync::MutexGuard<'_, ()>` note: required because it's used within this `async` fn body --> $DIR/auxiliary/issue_67893.rs:9:20 | diff --git a/tests/ui/lint/must_not_suspend/mutex.rs b/tests/ui/lint/must_not_suspend/mutex.rs index d14f7130b4cff..8dd4cc1761517 100644 --- a/tests/ui/lint/must_not_suspend/mutex.rs +++ b/tests/ui/lint/must_not_suspend/mutex.rs @@ -5,7 +5,7 @@ async fn other() {} pub async fn uhoh(m: std::sync::Mutex<()>) { - let _guard = m.lock().unwrap(); //~ ERROR `MutexGuard` held across + let _guard = m.lock().unwrap(); //~ ERROR `std::sync::MutexGuard` held across other().await; } diff --git a/tests/ui/lint/must_not_suspend/mutex.stderr b/tests/ui/lint/must_not_suspend/mutex.stderr index ca53a753150ec..0db1f2575b197 100644 --- a/tests/ui/lint/must_not_suspend/mutex.stderr +++ b/tests/ui/lint/must_not_suspend/mutex.stderr @@ -1,4 +1,4 @@ -error: `MutexGuard` held across a suspend point, but should not be +error: `std::sync::MutexGuard` held across a suspend point, but should not be --> $DIR/mutex.rs:8:9 | LL | let _guard = m.lock().unwrap(); diff --git a/tests/ui/privacy/private-field-access-in-mutex-54062.rs b/tests/ui/privacy/private-field-access-in-mutex-54062.rs index f145f9c3e529b..c957e0bc7e860 100644 --- a/tests/ui/privacy/private-field-access-in-mutex-54062.rs +++ b/tests/ui/privacy/private-field-access-in-mutex-54062.rs @@ -8,7 +8,7 @@ fn main() {} fn testing(test: Test) { let _ = test.comps.inner.try_lock(); - //~^ ERROR: field `inner` of struct `Mutex` is private + //~^ ERROR: field `inner` of struct `std::sync::Mutex` is private } // https://github.com/rust-lang/rust/issues/54062 diff --git a/tests/ui/privacy/private-field-access-in-mutex-54062.stderr b/tests/ui/privacy/private-field-access-in-mutex-54062.stderr index 1424410759746..f7f846406485e 100644 --- a/tests/ui/privacy/private-field-access-in-mutex-54062.stderr +++ b/tests/ui/privacy/private-field-access-in-mutex-54062.stderr @@ -1,4 +1,4 @@ -error[E0616]: field `inner` of struct `Mutex` is private +error[E0616]: field `inner` of struct `std::sync::Mutex` is private --> $DIR/private-field-access-in-mutex-54062.rs:10:24 | LL | let _ = test.comps.inner.try_lock(); diff --git a/tests/ui/suggestions/inner_type.fixed b/tests/ui/suggestions/inner_type.fixed index 3dc939d6b5c4f..175a2a02acdf8 100644 --- a/tests/ui/suggestions/inner_type.fixed +++ b/tests/ui/suggestions/inner_type.fixed @@ -25,7 +25,7 @@ fn main() { let another_item = std::sync::Mutex::new(Struct { p: 42_u32 }); another_item.lock().unwrap().method(); - //~^ ERROR no method named `method` found for struct `Mutex` in the current scope [E0599] + //~^ ERROR no method named `method` found for struct `std::sync::Mutex` in the current scope [E0599] //~| HELP use `.lock().unwrap()` to borrow the `Struct`, blocking the current thread until it can be acquired let another_item = std::sync::RwLock::new(Struct { p: 42_u32 }); diff --git a/tests/ui/suggestions/inner_type.rs b/tests/ui/suggestions/inner_type.rs index 81a05c2531197..ab021414f56ce 100644 --- a/tests/ui/suggestions/inner_type.rs +++ b/tests/ui/suggestions/inner_type.rs @@ -25,7 +25,7 @@ fn main() { let another_item = std::sync::Mutex::new(Struct { p: 42_u32 }); another_item.method(); - //~^ ERROR no method named `method` found for struct `Mutex` in the current scope [E0599] + //~^ ERROR no method named `method` found for struct `std::sync::Mutex` in the current scope [E0599] //~| HELP use `.lock().unwrap()` to borrow the `Struct`, blocking the current thread until it can be acquired let another_item = std::sync::RwLock::new(Struct { p: 42_u32 }); diff --git a/tests/ui/suggestions/inner_type.stderr b/tests/ui/suggestions/inner_type.stderr index 5ac3d04f10414..67ebb5789b70c 100644 --- a/tests/ui/suggestions/inner_type.stderr +++ b/tests/ui/suggestions/inner_type.stderr @@ -30,11 +30,11 @@ help: use `.borrow_mut()` to mutably borrow the `Struct`, panicking if any LL | other_item.borrow_mut().some_mutable_method(); | +++++++++++++ -error[E0599]: no method named `method` found for struct `Mutex` in the current scope +error[E0599]: no method named `method` found for struct `std::sync::Mutex` in the current scope --> $DIR/inner_type.rs:27:18 | LL | another_item.method(); - | ^^^^^^ method not found in `Mutex>` + | ^^^^^^ method not found in `std::sync::Mutex>` | note: the method `method` exists on the type `Struct` --> $DIR/inner_type.rs:9:5 diff --git a/tests/ui/sync/mutexguard-sync.stderr b/tests/ui/sync/mutexguard-sync.stderr index 1501a793d5e8b..ab9983c1f2c1c 100644 --- a/tests/ui/sync/mutexguard-sync.stderr +++ b/tests/ui/sync/mutexguard-sync.stderr @@ -8,7 +8,7 @@ LL | test_sync(guard); | = help: the trait `Sync` is not implemented for `Cell` = note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead - = note: required for `MutexGuard<'_, Cell>` to implement `Sync` + = note: required for `std::sync::MutexGuard<'_, Cell>` to implement `Sync` note: required by a bound in `test_sync` --> $DIR/mutexguard-sync.rs:5:17 | diff --git a/tests/ui/traits/const-traits/span-bug-issue-121418.stderr b/tests/ui/traits/const-traits/span-bug-issue-121418.stderr index 92cfecd054049..f31129d9cb7c7 100644 --- a/tests/ui/traits/const-traits/span-bug-issue-121418.stderr +++ b/tests/ui/traits/const-traits/span-bug-issue-121418.stderr @@ -14,8 +14,8 @@ error[E0277]: the size for values of type `(dyn T + 'static)` cannot be known at LL | pub const fn new() -> std::sync::Mutex {} | ^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | - = help: within `Mutex<(dyn T + 'static)>`, the trait `Sized` is not implemented for `(dyn T + 'static)` -note: required because it appears within the type `Mutex<(dyn T + 'static)>` + = help: within `std::sync::Mutex<(dyn T + 'static)>`, the trait `Sized` is not implemented for `(dyn T + 'static)` +note: required because it appears within the type `std::sync::Mutex<(dyn T + 'static)>` --> $SRC_DIR/std/src/sync/poison/mutex.rs:LL:COL = note: the return type of a function must have a statically known size @@ -27,7 +27,7 @@ LL | pub const fn new() -> std::sync::Mutex {} | | | implicitly returns `()` as its body has no tail or `return` expression | - = note: expected struct `Mutex<(dyn T + 'static)>` + = note: expected struct `std::sync::Mutex<(dyn T + 'static)>` found unit type `()` error: aborting due to 3 previous errors diff --git a/tests/ui/typeck/assign-non-lval-derefmut.fixed b/tests/ui/typeck/assign-non-lval-derefmut.fixed index 6ecec574f2ece..e6f97a9e86c68 100644 --- a/tests/ui/typeck/assign-non-lval-derefmut.fixed +++ b/tests/ui/typeck/assign-non-lval-derefmut.fixed @@ -5,11 +5,11 @@ fn main() { *x.lock().unwrap() = 2; //~^ ERROR invalid left-hand side of assignment *x.lock().unwrap() += 1; - //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + //~^ ERROR binary assignment operation `+=` cannot be applied to type `std::sync::MutexGuard<'_, usize>` let mut y = x.lock().unwrap(); *y = 2; //~^ ERROR mismatched types *y += 1; - //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + //~^ ERROR binary assignment operation `+=` cannot be applied to type `std::sync::MutexGuard<'_, usize>` } diff --git a/tests/ui/typeck/assign-non-lval-derefmut.rs b/tests/ui/typeck/assign-non-lval-derefmut.rs index ac1be913e2a9d..a53a52c7e4dd9 100644 --- a/tests/ui/typeck/assign-non-lval-derefmut.rs +++ b/tests/ui/typeck/assign-non-lval-derefmut.rs @@ -5,11 +5,11 @@ fn main() { x.lock().unwrap() = 2; //~^ ERROR invalid left-hand side of assignment x.lock().unwrap() += 1; - //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + //~^ ERROR binary assignment operation `+=` cannot be applied to type `std::sync::MutexGuard<'_, usize>` let mut y = x.lock().unwrap(); y = 2; //~^ ERROR mismatched types y += 1; - //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + //~^ ERROR binary assignment operation `+=` cannot be applied to type `std::sync::MutexGuard<'_, usize>` } diff --git a/tests/ui/typeck/assign-non-lval-derefmut.stderr b/tests/ui/typeck/assign-non-lval-derefmut.stderr index 16fb1e9c5c3a4..f57b5abe2eedc 100644 --- a/tests/ui/typeck/assign-non-lval-derefmut.stderr +++ b/tests/ui/typeck/assign-non-lval-derefmut.stderr @@ -11,15 +11,15 @@ help: consider dereferencing here to assign to the mutably borrowed value LL | *x.lock().unwrap() = 2; | + -error[E0368]: binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` +error[E0368]: binary assignment operation `+=` cannot be applied to type `std::sync::MutexGuard<'_, usize>` --> $DIR/assign-non-lval-derefmut.rs:7:5 | LL | x.lock().unwrap() += 1; | -----------------^^^^^ | | - | cannot use `+=` on type `MutexGuard<'_, usize>` + | cannot use `+=` on type `std::sync::MutexGuard<'_, usize>` | -note: the foreign item type `MutexGuard<'_, usize>` doesn't implement `AddAssign<{integer}>` +note: the foreign item type `std::sync::MutexGuard<'_, usize>` doesn't implement `AddAssign<{integer}>` --> $SRC_DIR/std/src/sync/poison/mutex.rs:LL:COL | = note: not implement `AddAssign<{integer}>` @@ -36,22 +36,22 @@ LL | let mut y = x.lock().unwrap(); LL | y = 2; | ^ expected `MutexGuard<'_, usize>`, found integer | - = note: expected struct `MutexGuard<'_, usize>` + = note: expected struct `std::sync::MutexGuard<'_, usize>` found type `{integer}` help: consider dereferencing here to assign to the mutably borrowed value | LL | *y = 2; | + -error[E0368]: binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` +error[E0368]: binary assignment operation `+=` cannot be applied to type `std::sync::MutexGuard<'_, usize>` --> $DIR/assign-non-lval-derefmut.rs:13:5 | LL | y += 1; | -^^^^^ | | - | cannot use `+=` on type `MutexGuard<'_, usize>` + | cannot use `+=` on type `std::sync::MutexGuard<'_, usize>` | -note: the foreign item type `MutexGuard<'_, usize>` doesn't implement `AddAssign<{integer}>` +note: the foreign item type `std::sync::MutexGuard<'_, usize>` doesn't implement `AddAssign<{integer}>` --> $SRC_DIR/std/src/sync/poison/mutex.rs:LL:COL | = note: not implement `AddAssign<{integer}>` diff --git a/tests/ui/typeck/deref-multi.stderr b/tests/ui/typeck/deref-multi.stderr index 02513853c486d..c4fa49e43ef27 100644 --- a/tests/ui/typeck/deref-multi.stderr +++ b/tests/ui/typeck/deref-multi.stderr @@ -63,7 +63,7 @@ LL | x.lock().unwrap() | ^^^^^^^^^^^^^^^^^ expected `i32`, found `MutexGuard<'_, &i32>` | = note: expected type `i32` - found struct `MutexGuard<'_, &i32>` + found struct `std::sync::MutexGuard<'_, &i32>` help: consider dereferencing the type | LL | **x.lock().unwrap() From ac4878962be0e3548a817e632ad962aa5ab4fa87 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Jul 2025 22:17:48 +0200 Subject: [PATCH 7/7] use generic function instead of macro Also cleans up some other test documentation. --- library/std/tests/sync/lib.rs | 62 ++++++++++++++++----------------- library/std/tests/sync/mutex.rs | 57 ++++++++++++++++-------------- 2 files changed, 61 insertions(+), 58 deletions(-) diff --git a/library/std/tests/sync/lib.rs b/library/std/tests/sync/lib.rs index 93433a3b858c3..94f1fe96b6a26 100644 --- a/library/std/tests/sync/lib.rs +++ b/library/std/tests/sync/lib.rs @@ -33,25 +33,29 @@ mod rwlock; #[path = "../common/mod.rs"] mod common; -// A macro that generates two test cases for both the poison and nonpoison locks. -// -// To write a test that tests both `poison` and `nonpoison` locks, import any of the types -// under both `poison` and `nonpoison` using the module name `locks` instead. For example, write -// `use locks::Mutex;` instead of `use std::sync::poiosn::Mutex`. This will import the correct type -// for each test variant. -// -// Write a test as normal in the `test_body`, but instead of calling `unwrap` on `poison` methods -// that return a `LockResult` or similar, call the macro `maybe_unwrap!(...)` on the result. -// -// For example, call `maybe_unwrap!(mutex.lock())` instead of `mutex.lock().unwrap()` or -// `maybe_unwrap!(rwlock.read())` instead of `rwlock.read().unwrap()`. -// -// For the `poison` types, `maybe_unwrap!` will simply unwrap the `Result` (usually this is a form -// of `LockResult`, but it could also be other kinds of results). For the `nonpoison` types, it is a -// no-op. -// -// The `poison` test will have the same `name`, but with a suffix of `_unwrap_poisoned`. -#[macro_export] +#[track_caller] +fn result_unwrap(x: Result) -> T { + x.unwrap() +} + +/// A macro that generates two test cases for both the poison and nonpoison locks. +/// +/// To write a test that tests both `poison` and `nonpoison` locks, import any of the types +/// under both `poison` and `nonpoison` using the module name `locks` instead. For example, write +/// `use locks::Mutex;` instead of `use std::sync::poiosn::Mutex`. This will import the correct type +/// for each test variant. +/// +/// Write a test as normal in the `test_body`, but instead of calling `unwrap` on `poison` methods +/// that return a `LockResult` or similar, call the function `maybe_unwrap(...)` on the result. +/// +/// For example, call `maybe_unwrap(mutex.lock())` instead of `mutex.lock().unwrap()` or +/// `maybe_unwrap(rwlock.read())` instead of `rwlock.read().unwrap()`. +/// +/// For the `poison` types, `maybe_unwrap` will simply unwrap the `Result` (usually this is a form +/// of `LockResult`, but it could also be other kinds of results). For the `nonpoison` types, it is +/// a no-op (the identity function). +/// +/// The test names will be prefiex with `poison_` or `nonpoison_`. macro_rules! nonpoison_and_poison_unwrap_test { ( name: $name:ident, @@ -59,28 +63,24 @@ macro_rules! nonpoison_and_poison_unwrap_test { ) => { // Creates the nonpoison test. #[test] - fn $name() { + fn ${concat(nonpoison_, $name)}() { + #[allow(unused_imports)] + use ::std::convert::identity as maybe_unwrap; use ::std::sync::nonpoison as locks; - #[allow(unused_macros)] - macro_rules! maybe_unwrap { - ($e:expr) => { $e }; - } - $($test_body)* } // Creates the poison test with the suffix `_unwrap_poisoned`. #[test] - fn ${concat($name, _unwrap_poisoned)}() { + fn ${concat(poison_, $name)}() { + #[allow(unused_imports)] + use super::result_unwrap as maybe_unwrap; use ::std::sync::poison as locks; - #[allow(unused_macros)] - macro_rules! maybe_unwrap { - ($e:expr) => { ::std::result::Result::unwrap($e) }; - } - $($test_body)* } } } + +use nonpoison_and_poison_unwrap_test; diff --git a/library/std/tests/sync/mutex.rs b/library/std/tests/sync/mutex.rs index 3c0ffbe52e421..f0648c36476de 100644 --- a/library/std/tests/sync/mutex.rs +++ b/library/std/tests/sync/mutex.rs @@ -17,8 +17,8 @@ nonpoison_and_poison_unwrap_test!( use locks::Mutex; let m = Mutex::new(()); - drop(maybe_unwrap!(m.lock())); - drop(maybe_unwrap!(m.lock())); + drop(maybe_unwrap(m.lock())); + drop(maybe_unwrap(m.lock())); } ); @@ -34,7 +34,7 @@ nonpoison_and_poison_unwrap_test!( fn inc(m: &Mutex) { for _ in 0..J { - *maybe_unwrap!(m.lock()) += 1; + *maybe_unwrap(m.lock()) += 1; } } @@ -58,7 +58,7 @@ nonpoison_and_poison_unwrap_test!( for _ in 0..2 * K { rx.recv().unwrap(); } - assert_eq!(*maybe_unwrap!(m.lock()), J * K * 2); + assert_eq!(*maybe_unwrap(m.lock()), J * K * 2); } ); @@ -96,7 +96,7 @@ nonpoison_and_poison_unwrap_test!( use locks::Mutex; let m = Mutex::new(NonCopy(10)); - assert_eq!(maybe_unwrap!(m.into_inner()), NonCopy(10)); + assert_eq!(maybe_unwrap(m.into_inner()), NonCopy(10)); } ); @@ -116,7 +116,7 @@ nonpoison_and_poison_unwrap_test!( let m = Mutex::new(Foo(num_drops.clone())); assert_eq!(num_drops.load(Ordering::SeqCst), 0); { - let _inner = maybe_unwrap!(m.into_inner()); + let _inner = maybe_unwrap(m.into_inner()); assert_eq!(num_drops.load(Ordering::SeqCst), 0); } assert_eq!(num_drops.load(Ordering::SeqCst), 1); @@ -129,8 +129,8 @@ nonpoison_and_poison_unwrap_test!( use locks::Mutex; let mut m = Mutex::new(NonCopy(10)); - *maybe_unwrap!(m.get_mut()) = NonCopy(20); - assert_eq!(maybe_unwrap!(m.into_inner()), NonCopy(20)); + *maybe_unwrap(m.get_mut()) = NonCopy(20); + assert_eq!(maybe_unwrap(m.into_inner()), NonCopy(20)); } ); @@ -144,7 +144,7 @@ nonpoison_and_poison_unwrap_test!( let m = Mutex::new(Cloneable(10)); - assert_eq!(maybe_unwrap!(m.get_cloned()), Cloneable(10)); + assert_eq!(maybe_unwrap(m.get_cloned()), Cloneable(10)); } ); @@ -159,9 +159,9 @@ nonpoison_and_poison_unwrap_test!( { let m = Mutex::new(init()); - assert_eq!(*maybe_unwrap!(m.lock()), init()); - maybe_unwrap!(m.set(value())); - assert_eq!(*maybe_unwrap!(m.lock()), value()); + assert_eq!(*maybe_unwrap(m.lock()), init()); + maybe_unwrap(m.set(value())); + assert_eq!(*maybe_unwrap(m.lock()), value()); } inner(|| NonCopy(10), || NonCopy(20)); @@ -169,6 +169,7 @@ nonpoison_and_poison_unwrap_test!( } ); +// Ensure that old values that are replaced by `set` are correctly dropped. nonpoison_and_poison_unwrap_test!( name: test_set_drop, test_body: { @@ -186,7 +187,7 @@ nonpoison_and_poison_unwrap_test!( assert_eq!(num_drops.load(Ordering::SeqCst), 0); let different = Foo(Arc::new(AtomicUsize::new(42))); - maybe_unwrap!(m.set(different)); + maybe_unwrap(m.set(different)); assert_eq!(num_drops.load(Ordering::SeqCst), 1); } ); @@ -202,9 +203,9 @@ nonpoison_and_poison_unwrap_test!( { let m = Mutex::new(init()); - assert_eq!(*maybe_unwrap!(m.lock()), init()); - assert_eq!(maybe_unwrap!(m.replace(value())), init()); - assert_eq!(*maybe_unwrap!(m.lock()), value()); + assert_eq!(*maybe_unwrap(m.lock()), init()); + assert_eq!(maybe_unwrap(m.replace(value())), init()); + assert_eq!(*maybe_unwrap(m.lock()), value()); } inner(|| NonCopy(10), || NonCopy(20)); @@ -257,8 +258,8 @@ nonpoison_and_poison_unwrap_test!( let arc2 = Arc::new(Mutex::new(arc)); let (tx, rx) = channel(); let _t = thread::spawn(move || { - let lock = maybe_unwrap!(arc2.lock()); - let lock2 = maybe_unwrap!(lock.lock()); + let lock = maybe_unwrap(arc2.lock()); + let lock2 = maybe_unwrap(lock.lock()); assert_eq!(*lock2, 1); tx.send(()).unwrap(); }); @@ -273,12 +274,12 @@ nonpoison_and_poison_unwrap_test!( let mutex: &Mutex<[i32]> = &Mutex::new([1, 2, 3]); { - let b = &mut *maybe_unwrap!(mutex.lock()); + let b = &mut *maybe_unwrap(mutex.lock()); b[0] = 4; b[2] = 5; } let comp: &[i32] = &[4, 2, 5]; - assert_eq!(&*maybe_unwrap!(mutex.lock()), comp); + assert_eq!(&*maybe_unwrap(mutex.lock()), comp); } ); @@ -289,16 +290,18 @@ nonpoison_and_poison_unwrap_test!( let arr = [0; 4]; let lock = Mutex::new(arr); - let guard = maybe_unwrap!(lock.lock()); + let guard = maybe_unwrap(lock.lock()); 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!(*maybe_unwrap!(lock.lock()), [0, 42, 0, 0]); + assert_eq!(*maybe_unwrap(lock.lock()), [0, 42, 0, 0]); } ); +// Ensures that both mutex types are able to be locked even after threads that hold the guards +// panic. This should be true even for the `nonpoison::Mutex`. #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] nonpoison_and_poison_unwrap_test!( name: test_panics, @@ -308,21 +311,21 @@ nonpoison_and_poison_unwrap_test!( let mutex = Mutex::new(42); let catch_unwind_result1 = panic::catch_unwind(AssertUnwindSafe(|| { - let _guard1 = maybe_unwrap!(mutex.lock()); + let _guard1 = maybe_unwrap(mutex.lock()); panic!("test panic with mutex once"); })); assert!(catch_unwind_result1.is_err()); let catch_unwind_result2 = panic::catch_unwind(AssertUnwindSafe(|| { - let _guard2 = maybe_unwrap!(mutex.lock()); + let _guard2 = maybe_unwrap(mutex.lock()); panic!("test panic with mutex twice"); })); assert!(catch_unwind_result2.is_err()); let catch_unwind_result3 = panic::catch_unwind(AssertUnwindSafe(|| { - let _guard3 = maybe_unwrap!(mutex.lock()); + let _guard3 = maybe_unwrap(mutex.lock()); panic!("test panic with mutex thrice"); })); @@ -344,14 +347,14 @@ nonpoison_and_poison_unwrap_test!( } impl Drop for Unwinder { fn drop(&mut self) { - *maybe_unwrap!(self.i.lock()) += 1; + *maybe_unwrap(self.i.lock()) += 1; } } let _u = Unwinder { i: arc2 }; panic!(); }) .join(); - let lock = maybe_unwrap!(arc.lock()); + let lock = maybe_unwrap(arc.lock()); assert_eq!(*lock, 2); } );