From 4223cd6f8a67d06b0d281132a4c71e6e1f1ee742 Mon Sep 17 00:00:00 2001 From: Jack Fransham Date: Mon, 21 Jun 2021 14:35:06 +0200 Subject: [PATCH 1/7] Allow `MappedMutexGuard` to own its data --- lock_api/src/mutex.rs | 134 +++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 59 deletions(-) diff --git a/lock_api/src/mutex.rs b/lock_api/src/mutex.rs index f64fc13f..28fa8b2e 100644 --- a/lock_api/src/mutex.rs +++ b/lock_api/src/mutex.rs @@ -499,7 +499,7 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { s.mutex } - /// Makes a new `MappedMutexGuard` for a component of the locked data. + /// Makes a new `OwnedMappedMutexGuard` for a component of the locked data. /// /// This operation cannot fail as the `MutexGuard` passed /// in already locked the mutex. @@ -508,21 +508,23 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { /// used as `MutexGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> + pub fn map(s: Self, f: F) -> OwnedMappedMutexGuard<'a, R, U> where - F: FnOnce(&mut T) -> &mut U, + F: FnOnce(&'a mut T) -> U, { let raw = &s.mutex.raw; - let data = f(unsafe { &mut *s.mutex.data.get() }); + let data = unsafe { &mut *s.mutex.data.get() }; mem::forget(s); - MappedMutexGuard { + + let data = f(data); + OwnedMappedMutexGuard { raw, data, marker: PhantomData, } } - /// Attempts to make a new `MappedMutexGuard` for a component of the + /// Attempts to make a new `OwnedMappedMutexGuard` for a component of the /// locked data. The original guard is returned if the closure returns `None`. /// /// This operation cannot fail as the `MutexGuard` passed @@ -532,17 +534,21 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { /// used as `MutexGuard::try_map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&mut T) -> Option<&mut U>, + F: FnOnce(&'a mut T) -> Option, { + let s = mem::ManuallyDrop::new(s); + let raw = &s.mutex.raw; - let data = match f(unsafe { &mut *s.mutex.data.get() }) { + let data = unsafe { &mut *s.mutex.data.get() }; + + let data = match f(data) { Some(data) => data, - None => return Err(s), + None => return Err(mem::ManuallyDrop::into_inner(s)), }; - mem::forget(s); - Ok(MappedMutexGuard { + + Ok(OwnedMappedMutexGuard { raw, data, marker: PhantomData, @@ -775,79 +781,93 @@ impl Drop for ArcMutexGuard { /// An RAII mutex guard returned by `MutexGuard::map`, which can point to a /// subfield of the protected data. /// -/// The main difference between `MappedMutexGuard` and `MutexGuard` is that the +/// The main difference between `OwnedMappedMutexGuard` and `MutexGuard` is that the /// former doesn't support temporarily unlocking and re-locking, since that /// could introduce soundness issues if the locked object is modified by another /// thread. #[must_use = "if unused the Mutex will immediately unlock"] -pub struct MappedMutexGuard<'a, R: RawMutex, T: ?Sized> { +pub struct OwnedMappedMutexGuard<'a, R: RawMutex, T: 'a> { raw: &'a R, - data: *mut T, - marker: PhantomData<&'a mut T>, + // We use `&'a mut` to make this type invariant over `'a` + marker: PhantomData<&'a mut ()>, + data: T, } -unsafe impl<'a, R: RawMutex + Sync + 'a, T: ?Sized + Sync + 'a> Sync - for MappedMutexGuard<'a, R, T> -{ -} -unsafe impl<'a, R: RawMutex + 'a, T: ?Sized + Send + 'a> Send for MappedMutexGuard<'a, R, T> where +/// An `OwnedMappedMutexGuard` that contains a mutable pointer (for backwards compatibility) +pub type MappedMutexGuard<'a, R, T> = OwnedMappedMutexGuard<'a, R, &'a mut T>; + +unsafe impl<'a, R: RawMutex + Sync + 'a, T: Sync + 'a> Sync for OwnedMappedMutexGuard<'a, R, T> {} +unsafe impl<'a, R: RawMutex + 'a, T: Send + 'a> Send for OwnedMappedMutexGuard<'a, R, T> where R::GuardMarker: Send { } -impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MappedMutexGuard<'a, R, T> { - /// Makes a new `MappedMutexGuard` for a component of the locked data. +impl<'a, R: RawMutex + 'a, T: 'a> OwnedMappedMutexGuard<'a, R, T> { + /// Makes a new `OwnedMappedMutexGuard` for a component of the locked data. /// - /// This operation cannot fail as the `MappedMutexGuard` passed + /// This operation cannot fail as the `OwnedMappedMutexGuard` passed /// in already locked the mutex. /// /// This is an associated function that needs to be - /// used as `MappedMutexGuard::map(...)`. A method would interfere with methods of + /// used as `OwnedMappedMutexGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> + pub fn map(s: Self, f: F) -> OwnedMappedMutexGuard<'a, R, U> where - F: FnOnce(&mut T) -> &mut U, + F: FnOnce(T) -> U, { + use core::ptr; + + let mut s = mem::ManuallyDrop::new(s); + let raw = s.raw; - let data = f(unsafe { &mut *s.data }); - mem::forget(s); - MappedMutexGuard { + + let data = unsafe { ptr::read(&mut s.data) }; + + let data = f(data); + OwnedMappedMutexGuard { raw, data, marker: PhantomData, } } - /// Attempts to make a new `MappedMutexGuard` for a component of the + /// Attempts to make a new `OwnedMappedMutexGuard` for a component of the /// locked data. The original guard is returned if the closure returns `None`. /// - /// This operation cannot fail as the `MappedMutexGuard` passed + /// This operation cannot fail as the `OwnedMappedMutexGuard` passed /// in already locked the mutex. /// /// This is an associated function that needs to be - /// used as `MappedMutexGuard::try_map(...)`. A method would interfere with methods of + /// used as `OwnedMappedMutexGuard::try_map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&mut T) -> Option<&mut U>, + F: FnOnce(T) -> Result, { + use core::ptr; + + let mut s = mem::ManuallyDrop::new(s); + let raw = s.raw; - let data = match f(unsafe { &mut *s.data }) { - Some(data) => data, - None => return Err(s), - }; - mem::forget(s); - Ok(MappedMutexGuard { - raw, - data, - marker: PhantomData, - }) + let data = unsafe { ptr::read(&mut s.data) }; + + f(data) + .map(|data| OwnedMappedMutexGuard { + raw, + data, + marker: PhantomData, + }) + .map_err(|data| OwnedMappedMutexGuard { + raw, + data, + marker: PhantomData, + }) } } -impl<'a, R: RawMutexFair + 'a, T: ?Sized + 'a> MappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutexFair + 'a, T: 'a> OwnedMappedMutexGuard<'a, R, T> { /// Unlocks the mutex using a fair unlock protocol. /// /// By default, mutexes are unfair and allow the current thread to re-lock @@ -870,44 +890,40 @@ impl<'a, R: RawMutexFair + 'a, T: ?Sized + 'a> MappedMutexGuard<'a, R, T> { } } -impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> Deref for MappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: 'a> Deref for OwnedMappedMutexGuard<'a, R, T> { type Target = T; + #[inline] fn deref(&self) -> &T { - unsafe { &*self.data } + &self.data } } -impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> DerefMut for MappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: 'a> DerefMut for OwnedMappedMutexGuard<'a, R, T> { #[inline] fn deref_mut(&mut self) -> &mut T { - unsafe { &mut *self.data } + &mut self.data } } -impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> Drop for MappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: 'a> Drop for OwnedMappedMutexGuard<'a, R, T> { #[inline] fn drop(&mut self) { - // Safety: A MappedMutexGuard always holds the lock. + // Safety: A OwnedMappedMutexGuard always holds the lock. unsafe { self.raw.unlock(); } } } -impl<'a, R: RawMutex + 'a, T: fmt::Debug + ?Sized + 'a> fmt::Debug for MappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: fmt::Debug + 'a> fmt::Debug for OwnedMappedMutexGuard<'a, R, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&**self, f) } } -impl<'a, R: RawMutex + 'a, T: fmt::Display + ?Sized + 'a> fmt::Display - for MappedMutexGuard<'a, R, T> -{ +impl<'a, R: RawMutex + 'a, T: fmt::Display + 'a> fmt::Display for OwnedMappedMutexGuard<'a, R, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { (**self).fmt(f) } } - -#[cfg(feature = "owning_ref")] -unsafe impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> StableAddress for MappedMutexGuard<'a, R, T> {} From 8a598cad5f5d637e0725203c054b3835657aabf5 Mon Sep 17 00:00:00 2001 From: Jack Fransham Date: Tue, 24 Aug 2021 13:36:59 +0200 Subject: [PATCH 2/7] Change MappedRwLock types to allow owned data --- lock_api/src/mutex.rs | 98 ++++++++++-------- lock_api/src/rwlock.rs | 226 +++++++++++++++++++++++++---------------- 2 files changed, 194 insertions(+), 130 deletions(-) diff --git a/lock_api/src/mutex.rs b/lock_api/src/mutex.rs index 28fa8b2e..0025dac8 100644 --- a/lock_api/src/mutex.rs +++ b/lock_api/src/mutex.rs @@ -499,7 +499,7 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { s.mutex } - /// Makes a new `OwnedMappedMutexGuard` for a component of the locked data. + /// Makes a new `MappedMutexGuard` for a component of the locked data. /// /// This operation cannot fail as the `MutexGuard` passed /// in already locked the mutex. @@ -508,7 +508,7 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { /// used as `MutexGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> OwnedMappedMutexGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> where F: FnOnce(&'a mut T) -> U, { @@ -517,14 +517,14 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { mem::forget(s); let data = f(data); - OwnedMappedMutexGuard { + MappedMutexGuard { raw, data, marker: PhantomData, } } - /// Attempts to make a new `OwnedMappedMutexGuard` for a component of the + /// Attempts to make a new `MappedMutexGuard` for a component of the /// locked data. The original guard is returned if the closure returns `None`. /// /// This operation cannot fail as the `MutexGuard` passed @@ -534,21 +534,23 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { /// used as `MutexGuard::try_map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where F: FnOnce(&'a mut T) -> Option, { - let s = mem::ManuallyDrop::new(s); - let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; let data = match f(data) { Some(data) => data, - None => return Err(mem::ManuallyDrop::into_inner(s)), + None => return Err(s), }; - Ok(OwnedMappedMutexGuard { + // We use `mem::forget` instead of `ManuallyDrop` because we want to drop `self` if + // `f` panicks. This is safe, as `self` must outlive the `&'a mut T` reference. + mem::forget(s); + + Ok(MappedMutexGuard { raw, data, marker: PhantomData, @@ -669,8 +671,8 @@ impl<'a, R: RawMutex + 'a, T: fmt::Display + ?Sized + 'a> fmt::Display for Mutex unsafe impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> StableAddress for MutexGuard<'a, R, T> {} /// An RAII mutex guard returned by the `Arc` locking operations on `Mutex`. -/// -/// This is similar to the `MutexGuard` struct, except instead of using a reference to unlock the `Mutex` it +/// +/// This is similar to the `MutexGuard` struct, except instead of using a reference to unlock the `Mutex` it /// uses an `Arc`. This has several advantages, most notably that it has an `'static` lifetime. #[cfg(feature = "arc_lock")] #[must_use = "if unused the Mutex will immediately unlock"] @@ -719,7 +721,7 @@ impl ArcMutexGuard { // SAFETY: make sure the Arc gets it reference decremented let mut s = ManuallyDrop::new(s); - unsafe { ptr::drop_in_place(&mut s.mutex) }; + unsafe { ptr::drop_in_place(&mut s.mutex) }; } /// Temporarily unlocks the mutex to execute the given function. @@ -781,93 +783,99 @@ impl Drop for ArcMutexGuard { /// An RAII mutex guard returned by `MutexGuard::map`, which can point to a /// subfield of the protected data. /// -/// The main difference between `OwnedMappedMutexGuard` and `MutexGuard` is that the +/// The main difference between `MappedMutexGuard` and `MutexGuard` is that the /// former doesn't support temporarily unlocking and re-locking, since that /// could introduce soundness issues if the locked object is modified by another /// thread. #[must_use = "if unused the Mutex will immediately unlock"] -pub struct OwnedMappedMutexGuard<'a, R: RawMutex, T: 'a> { +pub struct MappedMutexGuard<'a, R: RawMutex, T: ?Sized + 'a> { raw: &'a R, // We use `&'a mut` to make this type invariant over `'a` marker: PhantomData<&'a mut ()>, + // `data` at the end so we can cast `MappedMutexGuard<'_, _, [T; N]>` to `MappedMutexGuard<'_, _, [T]>`. data: T, } -/// An `OwnedMappedMutexGuard` that contains a mutable pointer (for backwards compatibility) -pub type MappedMutexGuard<'a, R, T> = OwnedMappedMutexGuard<'a, R, &'a mut T>; - -unsafe impl<'a, R: RawMutex + Sync + 'a, T: Sync + 'a> Sync for OwnedMappedMutexGuard<'a, R, T> {} -unsafe impl<'a, R: RawMutex + 'a, T: Send + 'a> Send for OwnedMappedMutexGuard<'a, R, T> where +unsafe impl<'a, R: RawMutex + Sync + 'a, T: ?Sized + Sync + 'a> Sync + for MappedMutexGuard<'a, R, T> +{ +} +unsafe impl<'a, R: RawMutex + 'a, T: ?Sized + Send + 'a> Send for MappedMutexGuard<'a, R, T> where R::GuardMarker: Send { } -impl<'a, R: RawMutex + 'a, T: 'a> OwnedMappedMutexGuard<'a, R, T> { - /// Makes a new `OwnedMappedMutexGuard` for a component of the locked data. +impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { + /// Makes a new `MappedMutexGuard` for a component of the locked data. /// - /// This operation cannot fail as the `OwnedMappedMutexGuard` passed + /// This operation cannot fail as the `MappedMutexGuard` passed /// in already locked the mutex. /// /// This is an associated function that needs to be - /// used as `OwnedMappedMutexGuard::map(...)`. A method would interfere with methods of + /// used as `MappedMutexGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> OwnedMappedMutexGuard<'a, R, U> + pub fn map(mut s: Self, f: F) -> MappedMutexGuard<'a, R, U> where F: FnOnce(T) -> U, { use core::ptr; - let mut s = mem::ManuallyDrop::new(s); - let raw = s.raw; let data = unsafe { ptr::read(&mut s.data) }; - let data = f(data); - OwnedMappedMutexGuard { + + mem::forget(s); + + MappedMutexGuard { raw, data, marker: PhantomData, } } - /// Attempts to make a new `OwnedMappedMutexGuard` for a component of the + /// Attempts to make a new `MappedMutexGuard` for a component of the /// locked data. The original guard is returned if the closure returns `None`. /// - /// This operation cannot fail as the `OwnedMappedMutexGuard` passed + /// This operation cannot fail as the `MappedMutexGuard` passed /// in already locked the mutex. /// /// This is an associated function that needs to be - /// used as `OwnedMappedMutexGuard::try_map(...)`. A method would interfere with methods of + /// used as `MappedMutexGuard::try_map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(mut s: Self, f: F) -> Result, Self> where F: FnOnce(T) -> Result, { use core::ptr; - let mut s = mem::ManuallyDrop::new(s); - let raw = s.raw; let data = unsafe { ptr::read(&mut s.data) }; - f(data) - .map(|data| OwnedMappedMutexGuard { + // This relies on the fact that `Result::map` and `Result::map_err` will never panic (so long + // as the closures passed to them do not panic). If these closures are replaced with ones + // that may panic, this could lead to the mutex being unlocked twice. + let out = f(data) + .map(|data| MappedMutexGuard { raw, data, marker: PhantomData, }) - .map_err(|data| OwnedMappedMutexGuard { + .map_err(|data| MappedMutexGuard { raw, data, marker: PhantomData, - }) + }); + + mem::forget(s); + + out } } -impl<'a, R: RawMutexFair + 'a, T: 'a> OwnedMappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutexFair + 'a, T: 'a> MappedMutexGuard<'a, R, T> { /// Unlocks the mutex using a fair unlock protocol. /// /// By default, mutexes are unfair and allow the current thread to re-lock @@ -890,7 +898,7 @@ impl<'a, R: RawMutexFair + 'a, T: 'a> OwnedMappedMutexGuard<'a, R, T> { } } -impl<'a, R: RawMutex + 'a, T: 'a> Deref for OwnedMappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> Deref for MappedMutexGuard<'a, R, T> { type Target = T; #[inline] @@ -899,30 +907,30 @@ impl<'a, R: RawMutex + 'a, T: 'a> Deref for OwnedMappedMutexGuard<'a, R, T> { } } -impl<'a, R: RawMutex + 'a, T: 'a> DerefMut for OwnedMappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> DerefMut for MappedMutexGuard<'a, R, T> { #[inline] fn deref_mut(&mut self) -> &mut T { &mut self.data } } -impl<'a, R: RawMutex + 'a, T: 'a> Drop for OwnedMappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> Drop for MappedMutexGuard<'a, R, T> { #[inline] fn drop(&mut self) { - // Safety: A OwnedMappedMutexGuard always holds the lock. + // Safety: A MappedMutexGuard always holds the lock. unsafe { self.raw.unlock(); } } } -impl<'a, R: RawMutex + 'a, T: fmt::Debug + 'a> fmt::Debug for OwnedMappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: fmt::Debug + 'a> fmt::Debug for MappedMutexGuard<'a, R, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&**self, f) } } -impl<'a, R: RawMutex + 'a, T: fmt::Display + 'a> fmt::Display for OwnedMappedMutexGuard<'a, R, T> { +impl<'a, R: RawMutex + 'a, T: fmt::Display + 'a> fmt::Display for MappedMutexGuard<'a, R, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { (**self).fmt(f) } diff --git a/lock_api/src/rwlock.rs b/lock_api/src/rwlock.rs index c404934e..d684b46c 100644 --- a/lock_api/src/rwlock.rs +++ b/lock_api/src/rwlock.rs @@ -590,7 +590,7 @@ impl RwLock { } /// Locks this `RwLock` with read access, through an `Arc`. - /// + /// /// This method is similar to the `read` method; however, it requires the `RwLock` to be inside of an `Arc` /// and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -602,8 +602,8 @@ impl RwLock { } /// Attempts to lock this `RwLock` with read access, through an `Arc`. - /// - /// This method is similar to the `try_read` method; however, it requires the `RwLock` to be inside of an + /// + /// This method is similar to the `try_read` method; however, it requires the `RwLock` to be inside of an /// `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] @@ -617,7 +617,7 @@ impl RwLock { } /// Locks this `RwLock` with write access, through an `Arc`. - /// + /// /// This method is similar to the `write` method; however, it requires the `RwLock` to be inside of an `Arc` /// and the resulting write guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -629,8 +629,8 @@ impl RwLock { } /// Attempts to lock this `RwLock` with writ access, through an `Arc`. - /// - /// This method is similar to the `try_write` method; however, it requires the `RwLock` to be inside of an + /// + /// This method is similar to the `try_write` method; however, it requires the `RwLock` to be inside of an /// `Arc` and the resulting write guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] @@ -744,12 +744,15 @@ impl RwLock { } /// Attempts to acquire this `RwLock` with read access until a timeout is reached, through an `Arc`. - /// + /// /// This method is similar to the `try_read_for` method; however, it requires the `RwLock` to be inside of an /// `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] - pub fn try_read_arc_for(self: &Arc, timeout: R::Duration) -> Option> { + pub fn try_read_arc_for( + self: &Arc, + timeout: R::Duration, + ) -> Option> { if self.raw.try_lock_shared_for(timeout) { // SAFETY: locking guarantee is upheld Some(unsafe { self.read_guard_arc() }) @@ -759,12 +762,15 @@ impl RwLock { } /// Attempts to acquire this `RwLock` with read access until a timeout is reached, through an `Arc`. - /// + /// /// This method is similar to the `try_read_until` method; however, it requires the `RwLock` to be inside of /// an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] - pub fn try_read_arc_until(self: &Arc, timeout: R::Instant) -> Option> { + pub fn try_read_arc_until( + self: &Arc, + timeout: R::Instant, + ) -> Option> { if self.raw.try_lock_shared_until(timeout) { // SAFETY: locking guarantee is upheld Some(unsafe { self.read_guard_arc() }) @@ -774,12 +780,15 @@ impl RwLock { } /// Attempts to acquire this `RwLock` with write access until a timeout is reached, through an `Arc`. - /// + /// /// This method is similar to the `try_write_for` method; however, it requires the `RwLock` to be inside of /// an `Arc` and the resulting write guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] - pub fn try_write_arc_for(self: &Arc, timeout: R::Duration) -> Option> { + pub fn try_write_arc_for( + self: &Arc, + timeout: R::Duration, + ) -> Option> { if self.raw.try_lock_exclusive_for(timeout) { // SAFETY: locking guarantee is upheld Some(unsafe { self.write_guard_arc() }) @@ -789,12 +798,15 @@ impl RwLock { } /// Attempts to acquire this `RwLock` with read access until a timeout is reached, through an `Arc`. - /// + /// /// This method is similar to the `try_write_until` method; however, it requires the `RwLock` to be inside of /// an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] - pub fn try_write_arc_until(self: &Arc, timeout: R::Instant) -> Option> { + pub fn try_write_arc_until( + self: &Arc, + timeout: R::Instant, + ) -> Option> { if self.raw.try_lock_exclusive_until(timeout) { // SAFETY: locking guarantee is upheld Some(unsafe { self.write_guard_arc() }) @@ -848,7 +860,7 @@ impl RwLock { } /// Locks this `RwLock` with shared read access, through an `Arc`. - /// + /// /// This method is similar to the `read_recursive` method; however, it requires the `RwLock` to be inside of /// an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -860,7 +872,7 @@ impl RwLock { } /// Attempts to lock this `RwLock` with shared read access, through an `Arc`. - /// + /// /// This method is similar to the `try_read_recursive` method; however, it requires the `RwLock` to be inside /// of an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -924,7 +936,10 @@ impl RwLock { /// inside of an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] - pub fn try_read_arc_recursive_for(self: &Arc, timeout: R::Duration) -> Option> { + pub fn try_read_arc_recursive_for( + self: &Arc, + timeout: R::Duration, + ) -> Option> { if self.raw.try_lock_shared_recursive_for(timeout) { // SAFETY: locking guarantee is upheld Some(unsafe { self.read_guard_arc() }) @@ -939,7 +954,10 @@ impl RwLock { /// inside of an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] #[inline] - pub fn try_read_arc_recursive_until(self: &Arc, timeout: R::Instant) -> Option> { + pub fn try_read_arc_recursive_until( + self: &Arc, + timeout: R::Instant, + ) -> Option> { if self.raw.try_lock_shared_recursive_until(timeout) { // SAFETY: locking guarantee is upheld Some(unsafe { self.read_guard_arc() }) @@ -995,19 +1013,19 @@ impl RwLock { } /// # Safety - /// + /// /// The lock must be held when calling this method. #[cfg(feature = "arc_lock")] #[inline] unsafe fn upgradable_guard_arc(self: &Arc) -> ArcRwLockUpgradableReadGuard { ArcRwLockUpgradableReadGuard { rwlock: self.clone(), - marker: PhantomData + marker: PhantomData, } } /// Locks this `RwLock` with upgradable read access, through an `Arc`. - /// + /// /// This method is similar to the `upgradable_read` method; however, it requires the `RwLock` to be /// inside of an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -1019,7 +1037,7 @@ impl RwLock { } /// Attempts to lock this `RwLock` with upgradable read access, through an `Arc`. - /// + /// /// This method is similar to the `try_upgradable_read` method; however, it requires the `RwLock` to be /// inside of an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -1074,7 +1092,7 @@ impl RwLock { } /// Attempts to lock this `RwLock` with upgradable access until a timeout is reached, through an `Arc`. - /// + /// /// This method is similar to the `try_upgradable_read_for` method; however, it requires the `RwLock` to be /// inside of an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -1092,7 +1110,7 @@ impl RwLock { } /// Attempts to lock this `RwLock` with upgradable access until a timeout is reached, through an `Arc`. - /// + /// /// This method is similar to the `try_upgradable_read_until` method; however, it requires the `RwLock` to be /// inside of an `Arc` and the resulting read guard has no lifetime requirements. #[cfg(feature = "arc_lock")] @@ -1167,9 +1185,9 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { /// used as `RwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> where - F: FnOnce(&T) -> &U, + F: FnOnce(&'a T) -> U, { let raw = &s.rwlock.raw; let data = f(unsafe { &*s.rwlock.data.get() }); @@ -1191,9 +1209,9 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { /// used as `RwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&T) -> Option<&U>, + F: FnOnce(&'a T) -> Option, { let raw = &s.rwlock.raw; let data = match f(unsafe { &*s.rwlock.data.get() }) { @@ -1318,9 +1336,9 @@ impl<'a, R: RawRwLock + 'a, T: fmt::Display + ?Sized + 'a> fmt::Display #[cfg(feature = "owning_ref")] unsafe impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> StableAddress for RwLockReadGuard<'a, R, T> {} -/// An RAII rwlock guard returned by the `Arc` locking operations on `RwLock`. -/// -/// This is similar to the `RwLockReadGuard` struct, except instead of using a reference to unlock the `RwLock` +/// An RAII rwlock guard returned by the `Arc` locking operations on `RwLock`. +/// +/// This is similar to the `RwLockReadGuard` struct, except instead of using a reference to unlock the `RwLock` /// it uses an `Arc`. This has several advantages, most notably that it has an `'static` lifetime. #[cfg(feature = "arc_lock")] #[must_use = "if unused the RwLock will immediately unlock"] @@ -1426,9 +1444,7 @@ impl fmt::Debug for ArcRwLockReadGuard fmt::Display - for ArcRwLockReadGuard -{ +impl fmt::Display for ArcRwLockReadGuard { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { (**self).fmt(f) } @@ -1457,9 +1473,9 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { /// used as `RwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> where - F: FnOnce(&mut T) -> &mut U, + F: FnOnce(&'a mut T) -> U, { let raw = &s.rwlock.raw; let data = f(unsafe { &mut *s.rwlock.data.get() }); @@ -1481,9 +1497,9 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { /// used as `RwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&mut T) -> Option<&mut U>, + F: FnOnce(&'a mut T) -> Option, { let raw = &s.rwlock.raw; let data = match f(unsafe { &mut *s.rwlock.data.get() }) { @@ -1655,8 +1671,8 @@ impl<'a, R: RawRwLock + 'a, T: fmt::Display + ?Sized + 'a> fmt::Display #[cfg(feature = "owning_ref")] unsafe impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> StableAddress for RwLockWriteGuard<'a, R, T> {} -/// An RAII rwlock guard returned by the `Arc` locking operations on `RwLock`. -/// This is similar to the `RwLockWriteGuard` struct, except instead of using a reference to unlock the `RwLock` +/// An RAII rwlock guard returned by the `Arc` locking operations on `RwLock`. +/// This is similar to the `RwLockWriteGuard` struct, except instead of using a reference to unlock the `RwLock` /// it uses an `Arc`. This has several advantages, most notably that it has an `'static` lifetime. #[cfg(feature = "arc_lock")] #[must_use = "if unused the RwLock will immediately unlock"] @@ -1770,7 +1786,7 @@ impl ArcRwLockWriteGuard { /// Temporarily yields the `RwLock` to a waiting thread if there is one. /// - /// This method is functionally equivalent to the `bump` method on [`RwLockWriteGuard`]. + /// This method is functionally equivalent to the `bump` method on [`RwLockWriteGuard`]. #[inline] pub fn bump(s: &mut Self) { // Safety: An RwLockWriteGuard always holds an exclusive lock. @@ -1816,9 +1832,7 @@ impl fmt::Debug for ArcRwLockWriteGuard fmt::Display - for ArcRwLockWriteGuard -{ +impl fmt::Display for ArcRwLockWriteGuard { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { (**self).fmt(f) } @@ -2059,7 +2073,7 @@ unsafe impl<'a, R: RawRwLockUpgrade + 'a, T: ?Sized + 'a> StableAddress /// An RAII rwlock guard returned by the `Arc` locking operations on `RwLock`. /// This is similar to the `RwLockUpgradableReadGuard` struct, except instead of using a reference to unlock the -/// `RwLock` it uses an `Arc`. This has several advantages, most notably that it has an `'static` +/// `RwLock` it uses an `Arc`. This has several advantages, most notably that it has an `'static` /// lifetime. #[cfg(feature = "arc_lock")] #[must_use = "if unused the RwLock will immediately unlock"] @@ -2069,7 +2083,7 @@ pub struct ArcRwLockUpgradableReadGuard { } #[cfg(feature = "arc_lock")] -impl ArcRwLockUpgradableReadGuard { +impl ArcRwLockUpgradableReadGuard { /// Returns a reference to the rwlock, contained in its original `Arc`. pub fn rwlock(s: &Self) -> &Arc> { &s.rwlock @@ -2144,7 +2158,7 @@ impl ArcRwLockUpgradableReadGuard { // SAFETY: make sure we decrement the refcount properly let mut s = ManuallyDrop::new(s); - unsafe { ptr::drop_in_place(&mut s.rwlock) }; + unsafe { ptr::drop_in_place(&mut s.rwlock) }; } /// Temporarily unlocks the `RwLock` to execute the given function. @@ -2291,7 +2305,6 @@ impl fmt::Display } } - /// An RAII read lock guard returned by `RwLockReadGuard::map`, which can point to a /// subfield of the protected data. /// @@ -2302,8 +2315,8 @@ impl fmt::Display #[must_use = "if unused the RwLock will immediately unlock"] pub struct MappedRwLockReadGuard<'a, R: RawRwLock, T: ?Sized> { raw: &'a R, - data: *const T, marker: PhantomData<&'a T>, + data: T, } unsafe impl<'a, R: RawRwLock + 'a, T: ?Sized + Sync + 'a> Sync for MappedRwLockReadGuard<'a, R, T> {} @@ -2312,7 +2325,7 @@ unsafe impl<'a, R: RawRwLock + 'a, T: ?Sized + Sync + 'a> Send for MappedRwLockR { } -impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> MappedRwLockReadGuard<'a, R, T> { +impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { /// Make a new `MappedRwLockReadGuard` for a component of the locked data. /// /// This operation cannot fail as the `MappedRwLockReadGuard` passed @@ -2322,13 +2335,19 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> MappedRwLockReadGuard<'a, R, T> { /// used as `MappedRwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> + pub fn map(mut s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> where - F: FnOnce(&T) -> &U, + F: FnOnce(T) -> U, { + use core::ptr; + let raw = s.raw; - let data = f(unsafe { &*s.data }); + + let data = unsafe { ptr::read(&mut s.data) }; + let data = f(data); + mem::forget(s); + MappedRwLockReadGuard { raw, data, @@ -2346,25 +2365,37 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> MappedRwLockReadGuard<'a, R, T> { /// used as `MappedRwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(mut s: Self, f: F) -> Result, Self> where - F: FnOnce(&T) -> Option<&U>, + F: FnOnce(T) -> Result, { + use core::ptr; + let raw = s.raw; - let data = match f(unsafe { &*s.data }) { - Some(data) => data, - None => return Err(s), - }; + let data = unsafe { ptr::read(&mut s.data) }; + + // This relies on the fact that `Result::map` and `Result::map_err` will never panic (so long + // as the closures passed to them do not panic). If these closures are replaced with ones + // that may panic, this could lead to the mutex being unlocked twice. + let out = f(data) + .map(|data| MappedRwLockReadGuard { + raw, + data, + marker: PhantomData, + }) + .map_err(|data| MappedRwLockReadGuard { + raw, + data, + marker: PhantomData, + }); + mem::forget(s); - Ok(MappedRwLockReadGuard { - raw, - data, - marker: PhantomData, - }) + + out } } -impl<'a, R: RawRwLockFair + 'a, T: ?Sized + 'a> MappedRwLockReadGuard<'a, R, T> { +impl<'a, R: RawRwLockFair + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { /// Unlocks the `RwLock` using a fair unlock protocol. /// /// By default, `RwLock` is unfair and allow the current thread to re-lock @@ -2391,7 +2422,14 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> Deref for MappedRwLockReadGuard<'a, type Target = T; #[inline] fn deref(&self) -> &T { - unsafe { &*self.data } + &self.data + } +} + +impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> DerefMut for MappedRwLockReadGuard<'a, R, T> { + #[inline] + fn deref_mut(&mut self) -> &mut T { + &mut self.data } } @@ -2437,8 +2475,8 @@ unsafe impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> StableAddress #[must_use = "if unused the RwLock will immediately unlock"] pub struct MappedRwLockWriteGuard<'a, R: RawRwLock, T: ?Sized> { raw: &'a R, - data: *mut T, marker: PhantomData<&'a mut T>, + data: T, } unsafe impl<'a, R: RawRwLock + 'a, T: ?Sized + Sync + 'a> Sync @@ -2450,7 +2488,7 @@ unsafe impl<'a, R: RawRwLock + 'a, T: ?Sized + Send + 'a> Send for MappedRwLockW { } -impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> MappedRwLockWriteGuard<'a, R, T> { +impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { /// Make a new `MappedRwLockWriteGuard` for a component of the locked data. /// /// This operation cannot fail as the `MappedRwLockWriteGuard` passed @@ -2460,13 +2498,19 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> MappedRwLockWriteGuard<'a, R, T> { /// used as `MappedRwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> + pub fn map(mut s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> where - F: FnOnce(&mut T) -> &mut U, + F: FnOnce(T) -> U, { + use core::ptr; + let raw = s.raw; - let data = f(unsafe { &mut *s.data }); + + let data = unsafe { ptr::read(&mut s.data) }; + let data = f(data); + mem::forget(s); + MappedRwLockWriteGuard { raw, data, @@ -2484,25 +2528,37 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> MappedRwLockWriteGuard<'a, R, T> { /// used as `MappedRwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map(mut s: Self, f: F) -> Result, Self> where - F: FnOnce(&mut T) -> Option<&mut U>, + F: FnOnce(T) -> Result, { + use core::ptr; + let raw = s.raw; - let data = match f(unsafe { &mut *s.data }) { - Some(data) => data, - None => return Err(s), - }; + let data = unsafe { ptr::read(&mut s.data) }; + + // This relies on the fact that `Result::map` and `Result::map_err` will never panic (so long + // as the closures passed to them do not panic). If these closures are replaced with ones + // that may panic, this could lead to the mutex being unlocked twice. + let out = f(data) + .map(|data| MappedRwLockWriteGuard { + raw, + data, + marker: PhantomData, + }) + .map_err(|data| MappedRwLockWriteGuard { + raw, + data, + marker: PhantomData, + }); + mem::forget(s); - Ok(MappedRwLockWriteGuard { - raw, - data, - marker: PhantomData, - }) + + out } } -impl<'a, R: RawRwLockFair + 'a, T: ?Sized + 'a> MappedRwLockWriteGuard<'a, R, T> { +impl<'a, R: RawRwLockFair + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { /// Unlocks the `RwLock` using a fair unlock protocol. /// /// By default, `RwLock` is unfair and allow the current thread to re-lock @@ -2529,14 +2585,14 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> Deref for MappedRwLockWriteGuard<'a, type Target = T; #[inline] fn deref(&self) -> &T { - unsafe { &*self.data } + &self.data } } impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> DerefMut for MappedRwLockWriteGuard<'a, R, T> { #[inline] fn deref_mut(&mut self) -> &mut T { - unsafe { &mut *self.data } + &mut self.data } } From a89fb5c8bee52471abc6bdcb96f632074ddded3f Mon Sep 17 00:00:00 2001 From: Jack Fransham Date: Wed, 25 Aug 2021 10:51:25 +0200 Subject: [PATCH 3/7] Fix double drop in `Mapped*Guard::{try_,}map` --- lock_api/src/mutex.rs | 70 ++++++++++++--------- lock_api/src/rwlock.rs | 135 ++++++++++++++++++++++++----------------- 2 files changed, 120 insertions(+), 85 deletions(-) diff --git a/lock_api/src/mutex.rs b/lock_api/src/mutex.rs index 0025dac8..c7aebccc 100644 --- a/lock_api/src/mutex.rs +++ b/lock_api/src/mutex.rs @@ -10,13 +10,12 @@ use core::fmt; use core::marker::PhantomData; use core::mem; use core::ops::{Deref, DerefMut}; +use core::ptr; #[cfg(feature = "arc_lock")] use alloc::sync::Arc; #[cfg(feature = "arc_lock")] use core::mem::ManuallyDrop; -#[cfg(feature = "arc_lock")] -use core::ptr; #[cfg(feature = "owning_ref")] use owning_ref::StableAddress; @@ -511,6 +510,7 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> where F: FnOnce(&'a mut T) -> U, + U: 'a, { let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; @@ -815,18 +815,26 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { /// used as `MappedMutexGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(mut s: Self, f: F) -> MappedMutexGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> where F: FnOnce(T) -> U, { - use core::ptr; + let (data, raw) = { + let s = mem::ManuallyDrop::new(s); + (unsafe { ptr::read(&s.data) }, s.raw) + }; - let raw = s.raw; + // `panic::catch_unwind` isn't available in `core`, so we use a dummy guard to unlock + // the mutex in case of unwind. + let lock_guard: MappedMutexGuard<'a, R, ()> = MappedMutexGuard { + raw, + data: (), + marker: PhantomData, + }; - let data = unsafe { ptr::read(&mut s.data) }; let data = f(data); - mem::forget(s); + mem::forget(lock_guard); MappedMutexGuard { raw, @@ -845,33 +853,37 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { /// used as `MappedMutexGuard::try_map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(mut s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where F: FnOnce(T) -> Result, { - use core::ptr; - - let raw = s.raw; - let data = unsafe { ptr::read(&mut s.data) }; - - // This relies on the fact that `Result::map` and `Result::map_err` will never panic (so long - // as the closures passed to them do not panic). If these closures are replaced with ones - // that may panic, this could lead to the mutex being unlocked twice. - let out = f(data) - .map(|data| MappedMutexGuard { - raw, - data, - marker: PhantomData, - }) - .map_err(|data| MappedMutexGuard { - raw, - data, - marker: PhantomData, - }); + let (data, raw) = { + let s = mem::ManuallyDrop::new(s); + (unsafe { ptr::read(&s.data) }, s.raw) + }; - mem::forget(s); + // `panic::catch_unwind` isn't available in `core`, so we use a dummy guard to unlock + // the mutex in case of unwind. + let lock_guard: MappedMutexGuard<'a, R, ()> = MappedMutexGuard { + raw, + data: (), + marker: PhantomData, + }; + + let out = f(data); - out + mem::forget(lock_guard); + + out.map(|data| MappedMutexGuard { + raw, + data, + marker: PhantomData, + }) + .map_err(|data| MappedMutexGuard { + raw, + data, + marker: PhantomData, + }) } } diff --git a/lock_api/src/rwlock.rs b/lock_api/src/rwlock.rs index d684b46c..9b8d441f 100644 --- a/lock_api/src/rwlock.rs +++ b/lock_api/src/rwlock.rs @@ -10,13 +10,12 @@ use core::fmt; use core::marker::PhantomData; use core::mem; use core::ops::{Deref, DerefMut}; +use core::ptr; #[cfg(feature = "arc_lock")] use alloc::sync::Arc; #[cfg(feature = "arc_lock")] use core::mem::ManuallyDrop; -#[cfg(feature = "arc_lock")] -use core::ptr; #[cfg(feature = "owning_ref")] use owning_ref::StableAddress; @@ -2335,18 +2334,26 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { /// used as `MappedRwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(mut s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> where F: FnOnce(T) -> U, { - use core::ptr; + let (data, raw) = { + let s = mem::ManuallyDrop::new(s); + (unsafe { ptr::read(&s.data) }, s.raw) + }; - let raw = s.raw; + // `panic::catch_unwind` isn't available in `core`, so we use a dummy guard to unlock + // the mutex in case of unwind. + let lock_guard: MappedRwLockReadGuard<'a, R, ()> = MappedRwLockReadGuard { + raw, + data: (), + marker: PhantomData, + }; - let data = unsafe { ptr::read(&mut s.data) }; let data = f(data); - mem::forget(s); + mem::forget(lock_guard); MappedRwLockReadGuard { raw, @@ -2365,33 +2372,37 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { /// used as `MappedRwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(mut s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where F: FnOnce(T) -> Result, { - use core::ptr; - - let raw = s.raw; - let data = unsafe { ptr::read(&mut s.data) }; - - // This relies on the fact that `Result::map` and `Result::map_err` will never panic (so long - // as the closures passed to them do not panic). If these closures are replaced with ones - // that may panic, this could lead to the mutex being unlocked twice. - let out = f(data) - .map(|data| MappedRwLockReadGuard { - raw, - data, - marker: PhantomData, - }) - .map_err(|data| MappedRwLockReadGuard { - raw, - data, - marker: PhantomData, - }); + let (data, raw) = { + let s = mem::ManuallyDrop::new(s); + (unsafe { ptr::read(&s.data) }, s.raw) + }; - mem::forget(s); + // `panic::catch_unwind` isn't available in `core`, so we use a dummy guard to unlock + // the mutex in case of unwind. + let lock_guard: MappedRwLockReadGuard<'a, R, ()> = MappedRwLockReadGuard { + raw, + data: (), + marker: PhantomData, + }; - out + let out = f(data); + + mem::forget(lock_guard); + + out.map(|data| MappedRwLockReadGuard { + raw, + data, + marker: PhantomData, + }) + .map_err(|data| MappedRwLockReadGuard { + raw, + data, + marker: PhantomData, + }) } } @@ -2498,18 +2509,26 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { /// used as `MappedRwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(mut s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> where F: FnOnce(T) -> U, { - use core::ptr; + let (data, raw) = { + let s = mem::ManuallyDrop::new(s); + (unsafe { ptr::read(&s.data) }, s.raw) + }; - let raw = s.raw; + // `panic::catch_unwind` isn't available in `core`, so we use a dummy guard to unlock + // the mutex in case of unwind. + let lock_guard: MappedRwLockWriteGuard<'a, R, ()> = MappedRwLockWriteGuard { + raw, + data: (), + marker: PhantomData, + }; - let data = unsafe { ptr::read(&mut s.data) }; let data = f(data); - mem::forget(s); + mem::forget(lock_guard); MappedRwLockWriteGuard { raw, @@ -2528,33 +2547,37 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { /// used as `MappedRwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(mut s: Self, f: F) -> Result, Self> + pub fn try_map(s: Self, f: F) -> Result, Self> where F: FnOnce(T) -> Result, { - use core::ptr; - - let raw = s.raw; - let data = unsafe { ptr::read(&mut s.data) }; - - // This relies on the fact that `Result::map` and `Result::map_err` will never panic (so long - // as the closures passed to them do not panic). If these closures are replaced with ones - // that may panic, this could lead to the mutex being unlocked twice. - let out = f(data) - .map(|data| MappedRwLockWriteGuard { - raw, - data, - marker: PhantomData, - }) - .map_err(|data| MappedRwLockWriteGuard { - raw, - data, - marker: PhantomData, - }); + let (data, raw) = { + let s = mem::ManuallyDrop::new(s); + (unsafe { ptr::read(&s.data) }, s.raw) + }; - mem::forget(s); + // `panic::catch_unwind` isn't available in `core`, so we use a dummy guard to unlock + // the mutex in case of unwind. + let lock_guard: MappedRwLockWriteGuard<'a, R, ()> = MappedRwLockWriteGuard { + raw, + data: (), + marker: PhantomData, + }; + + let out = f(data); - out + mem::forget(lock_guard); + + out.map(|data| MappedRwLockWriteGuard { + raw, + data, + marker: PhantomData, + }) + .map_err(|data| MappedRwLockWriteGuard { + raw, + data, + marker: PhantomData, + }) } } From de6920c0a0cc89555b12c5cede5ca729ac80c1a4 Mon Sep 17 00:00:00 2001 From: Jack Fransham Date: Wed, 25 Aug 2021 11:05:22 +0200 Subject: [PATCH 4/7] Restrict mapping function bounds to prevent possible UB until it has been proven safe --- lock_api/src/mutex.rs | 5 ++--- lock_api/src/rwlock.rs | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lock_api/src/mutex.rs b/lock_api/src/mutex.rs index c7aebccc..22bb5c6c 100644 --- a/lock_api/src/mutex.rs +++ b/lock_api/src/mutex.rs @@ -509,8 +509,7 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> where - F: FnOnce(&'a mut T) -> U, - U: 'a, + F: FnOnce(&mut T) -> U, { let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; @@ -536,7 +535,7 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&'a mut T) -> Option, + F: FnOnce(&mut T) -> Option, { let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; diff --git a/lock_api/src/rwlock.rs b/lock_api/src/rwlock.rs index 9b8d441f..9295af00 100644 --- a/lock_api/src/rwlock.rs +++ b/lock_api/src/rwlock.rs @@ -1186,7 +1186,7 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> where - F: FnOnce(&'a T) -> U, + F: FnOnce(&T) -> U, { let raw = &s.rwlock.raw; let data = f(unsafe { &*s.rwlock.data.get() }); @@ -1210,7 +1210,7 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&'a T) -> Option, + F: FnOnce(&T) -> Option, { let raw = &s.rwlock.raw; let data = match f(unsafe { &*s.rwlock.data.get() }) { @@ -1474,7 +1474,7 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> where - F: FnOnce(&'a mut T) -> U, + F: FnOnce(&mut T) -> U, { let raw = &s.rwlock.raw; let data = f(unsafe { &mut *s.rwlock.data.get() }); @@ -1498,7 +1498,7 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&'a mut T) -> Option, + F: FnOnce(&mut T) -> Option, { let raw = &s.rwlock.raw; let data = match f(unsafe { &mut *s.rwlock.data.get() }) { From e2fa040ed56411563413cc368d2bc5050e782f87 Mon Sep 17 00:00:00 2001 From: Jack Fransham Date: Wed, 25 Aug 2021 14:15:24 +0200 Subject: [PATCH 5/7] WIP: Add more lifetime bounds --- lock_api/src/mutex.rs | 12 +++++++---- lock_api/src/rwlock.rs | 20 +++++++++++++------ src/mutex.rs | 45 ++++++++++++++++++++++++++++++++++++++++++ src/rwlock.rs | 33 +++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 10 deletions(-) diff --git a/lock_api/src/mutex.rs b/lock_api/src/mutex.rs index 22bb5c6c..7bc61d07 100644 --- a/lock_api/src/mutex.rs +++ b/lock_api/src/mutex.rs @@ -509,7 +509,8 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> where - F: FnOnce(&mut T) -> U, + F: FnOnce(&'a mut T) -> U + 'a, + U: 'a, { let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; @@ -535,7 +536,8 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&mut T) -> Option, + F: FnOnce(&'a mut T) -> Option + 'a, + U: 'a, { let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; @@ -816,7 +818,8 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> where - F: FnOnce(T) -> U, + F: FnOnce(T) -> U + 'a, + U: 'a, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -854,7 +857,8 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(T) -> Result, + F: FnOnce(T) -> Result + 'a, + U: 'a, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); diff --git a/lock_api/src/rwlock.rs b/lock_api/src/rwlock.rs index 9295af00..fd9ca9c0 100644 --- a/lock_api/src/rwlock.rs +++ b/lock_api/src/rwlock.rs @@ -1186,7 +1186,8 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> where - F: FnOnce(&T) -> U, + F: FnOnce(&'a T) -> U + 'a, + U: 'a, { let raw = &s.rwlock.raw; let data = f(unsafe { &*s.rwlock.data.get() }); @@ -1210,7 +1211,8 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&T) -> Option, + F: FnOnce(&'a T) -> Option + 'a, + U: 'a, { let raw = &s.rwlock.raw; let data = match f(unsafe { &*s.rwlock.data.get() }) { @@ -1474,7 +1476,8 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> where - F: FnOnce(&mut T) -> U, + F: FnOnce(&mut T) -> U + 'a, + U: 'a, { let raw = &s.rwlock.raw; let data = f(unsafe { &mut *s.rwlock.data.get() }); @@ -1498,7 +1501,8 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(&mut T) -> Option, + F: FnOnce(&mut T) -> Option + 'a, + U: 'a, { let raw = &s.rwlock.raw; let data = match f(unsafe { &mut *s.rwlock.data.get() }) { @@ -2336,7 +2340,8 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { #[inline] pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> where - F: FnOnce(T) -> U, + F: FnOnce(T) -> U + 'a, + U: 'a, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -2374,7 +2379,8 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { #[inline] pub fn try_map(s: Self, f: F) -> Result, Self> where - F: FnOnce(T) -> Result, + F: FnOnce(T) -> Result + 'a, + U: 'a, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -2512,6 +2518,7 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> where F: FnOnce(T) -> U, + U: 'a, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -2550,6 +2557,7 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { pub fn try_map(s: Self, f: F) -> Result, Self> where F: FnOnce(T) -> Result, + U: 'a, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); diff --git a/src/mutex.rs b/src/mutex.rs index 9f63cb94..02fbff84 100644 --- a/src/mutex.rs +++ b/src/mutex.rs @@ -109,6 +109,29 @@ pub type MutexGuard<'a, T> = lock_api::MutexGuard<'a, RawMutex, T>; /// thread. pub type MappedMutexGuard<'a, T> = lock_api::MappedMutexGuard<'a, RawMutex, T>; +#[test] +fn test_map() { + use crate::{lock_api::RawMutex, Mutex, MutexGuard}; + + const FOO: usize = 0; + + static OUTER: Mutex<&usize> = Mutex::const_new(RawMutex::INIT, &FOO); + static M: Mutex = Mutex::const_new(RawMutex::INIT, 0); + + let guard = MutexGuard::map(M.lock(), |inner: &'static mut usize| { + *OUTER.lock() = inner; + }); + drop(guard); + + let outer: &usize = &*OUTER.lock(); + + assert_eq!(*outer, 0); + + *M.lock() = 1; + + assert_eq!(*outer, 1); +} + #[cfg(test)] mod tests { use crate::{Condvar, Mutex}; @@ -117,6 +140,28 @@ mod tests { use std::sync::Arc; use std::thread; + #[cfg(doctest)] + mod doctests { + //! ```rust,no_run + //! use parking_lot::{Mutex, MutexGuard}; + //! + //! let m = Mutex::new((0, 0)); + //! let guard = MutexGuard::map(m.lock(), |inner| &mut inner.0); + //! ``` + //! + //! ```rust,compile_fail + //! use parking_lot::{Mutex, MutexGuard, lock_api::RawMutex}; + //! + //! let mut outer = &mut 0; + //! static M: Mutex = Mutex::const_new(RawMutex::INIT, 0); + //! let guard = MutexGuard::map(M.lock(), |inner: &'static mut i32| { + //! outer = inner; + //! }); + //! drop(guard); + //! *outer = 1; + //! ``` + } + #[cfg(feature = "serde")] use bincode::{deserialize, serialize}; diff --git a/src/rwlock.rs b/src/rwlock.rs index 70e1b1a7..02714535 100644 --- a/src/rwlock.rs +++ b/src/rwlock.rs @@ -135,6 +135,39 @@ mod tests { use std::thread; use std::time::Duration; + #[cfg(doctest)] + mod doctests { + //! ```rust,no_run + //! use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; + //! + //! let m = RwLock::new((0, 0)); + //! + //! let guard = RwLockReadGuard::map(m.read(), |inner| &inner.0); + //! ``` + //! + //! ```rust,no_run + //! use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; + //! + //! let m = RwLock::new((0, 0)); + //! + //! let guard = RwLockWriteGuard::map(m.write(), |inner| &mut inner.0); + //! ``` + //! + //! ```rust,compile_fail + //! use std::sync::Mutex; + //! use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard, RawRwLock}; + //! + //! let mut outer = &0; + //! static M: RwLock = RwLock::const_new(RawRwLock::INIT, 0); + //! + //! let guard = RwLockReadGuard::map(M.read(), |inner: &'static i32| { + //! outer = inner; + //! }); + //! drop(guard); + //! *outer = 1; + //! ``` + } + #[cfg(feature = "serde")] use bincode::{deserialize, serialize}; From 9f596ca3a989feb71cc2b90ef85e8be6684b4ccb Mon Sep 17 00:00:00 2001 From: Jack Fransham Date: Thu, 26 Aug 2021 12:41:57 +0200 Subject: [PATCH 6/7] fix: Fix lifetime issues (at the cost of worse type inference) --- core/src/spinwait.rs | 2 + lock_api/Cargo.toml | 1 + lock_api/src/lib.rs | 81 +++++++++++++++++++++++++++ lock_api/src/mutex.rs | 56 +++++++++++-------- lock_api/src/rwlock.rs | 97 +++++++++++++++++++------------- src/mutex.rs | 122 ++++++++++++++++++++++++++--------------- 6 files changed, 256 insertions(+), 103 deletions(-) diff --git a/core/src/spinwait.rs b/core/src/spinwait.rs index ad0327a3..c7134997 100644 --- a/core/src/spinwait.rs +++ b/core/src/spinwait.rs @@ -6,11 +6,13 @@ // copied, modified, or distributed except according to those terms. use crate::thread_parker; +#[allow(deprecated)] use std::sync::atomic::spin_loop_hint; // Wastes some CPU time for the given number of iterations, // using a hint to indicate to the CPU that we are spinning. #[inline] +#[allow(deprecated)] fn cpu_relax(iterations: u32) { for _ in 0..iterations { spin_loop_hint() diff --git a/lock_api/Cargo.toml b/lock_api/Cargo.toml index a5aaa313..37729064 100644 --- a/lock_api/Cargo.toml +++ b/lock_api/Cargo.toml @@ -10,6 +10,7 @@ categories = ["concurrency", "no-std"] edition = "2018" [dependencies] +fn_ops = "0.1" scopeguard = { version = "1.1.0", default-features = false } owning_ref = { version = "0.4.1", optional = true } diff --git a/lock_api/src/lib.rs b/lock_api/src/lib.rs index c99c68bd..9e4d60a1 100644 --- a/lock_api/src/lib.rs +++ b/lock_api/src/lib.rs @@ -114,3 +114,84 @@ pub use crate::remutex::*; mod rwlock; pub use crate::rwlock::*; + +/// A "shim trait" to allow generalizing over functions which return some generic +/// type which may borrow elements of its arguments, but without specifying that +/// the return type is `&`, `&mut` or something else concrete. This allows using +/// HRTB to force a caller to supply a function which works for any lifetime, +/// and therefore avoids the caller relying on a specific lifetime for the +/// argument, which can cause UB if the inner data lives for the static lifetime. +/// +/// It also allows the output type to depend on the input type, which is important +/// when using lifetimes in HRTBs but is not possible with the stable syntax for +/// the `Fn` traits. +pub trait FnOnceShim<'a, T: 'a> { + /// Equivalent to `std::ops::FnOnce::Output`. + type Output: 'a; + + /// Equivalent to `std::ops::FnOnce::call` + fn call(self, input: T) -> Self::Output; +} + +impl<'a, F, In, Out> FnOnceShim<'a, In> for F +where + F: FnOnce(In) -> Out, + In: 'a, + Out: 'a, +{ + type Output = Out; + + fn call(self, input: In) -> Self::Output { + self(input) + } +} + +/// As `FnOnceShim`, but specialized for functions which return an `Option` (used +/// for `try_map`). +pub trait FnOnceOptionShim<'a, T: 'a> { + /// Equivalent to `std::ops::FnOnce::Output`. + type Output: 'a; + + /// Equivalent to `std::ops::FnOnce::call` + fn call(self, input: T) -> Option; +} + +impl<'a, F, In, Out> FnOnceOptionShim<'a, In> for F +where + F: FnOnce(In) -> Option, + In: 'a, + Out: 'a, +{ + type Output = Out; + + fn call(self, input: In) -> Option { + self(input) + } +} + +/// As `FnOnceShim`, but specialized for functions which return an `Result` (used +/// for `try_map`). +pub trait FnOnceResultShim<'a, T: 'a> { + /// Equivalent to `std::ops::FnOnce::Output`. + type Output: 'a; + /// Equivalent to `std::ops::FnOnce::Output`. + type Error: 'a; + + /// Equivalent to `std::ops::FnOnce::call` + fn call(self, input: T) -> Result; +} + +impl<'a, F, In, Out, Error> FnOnceResultShim<'a, In> for F +where + F: FnOnce(In) -> Result, + In: 'a, + Out: 'a, + Error: 'a, +{ + type Output = Out; + type Error = Error; + + fn call(self, input: In) -> Result { + self(input) + } +} diff --git a/lock_api/src/mutex.rs b/lock_api/src/mutex.rs index 7bc61d07..e1fdccd6 100644 --- a/lock_api/src/mutex.rs +++ b/lock_api/src/mutex.rs @@ -5,12 +5,16 @@ // http://opensource.org/licenses/MIT>, at your option. This file may not be // copied, modified, or distributed except according to those terms. -use core::cell::UnsafeCell; -use core::fmt; -use core::marker::PhantomData; -use core::mem; -use core::ops::{Deref, DerefMut}; -use core::ptr; +use core::{ + cell::UnsafeCell, + fmt, + marker::PhantomData, + mem, + ops::{Deref, DerefMut}, + ptr, +}; + +use crate::{FnOnceOptionShim, FnOnceResultShim, FnOnceShim}; #[cfg(feature = "arc_lock")] use alloc::sync::Arc; @@ -507,16 +511,18 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { /// used as `MutexGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> + pub fn map( + s: Self, + f: F, + ) -> MappedMutexGuard<'a, R, >::Output> where - F: FnOnce(&'a mut T) -> U + 'a, - U: 'a, + for<'any> F: FnOnceShim<'any, &'any mut T>, { let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; mem::forget(s); - let data = f(data); + let data = f.call(data); MappedMutexGuard { raw, data, @@ -534,15 +540,17 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> { /// used as `MutexGuard::try_map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map( + s: Self, + f: F, + ) -> Result>::Output>, Self> where - F: FnOnce(&'a mut T) -> Option + 'a, - U: 'a, + for<'any> F: FnOnceOptionShim<'any, &'any mut T>, { let raw = &s.mutex.raw; let data = unsafe { &mut *s.mutex.data.get() }; - let data = match f(data) { + let data = match f.call(data) { Some(data) => data, None => return Err(s), }; @@ -816,10 +824,9 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { /// used as `MappedMutexGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedMutexGuard<'a, R, >::Output> where - F: FnOnce(T) -> U + 'a, - U: 'a, + for<'any> F: FnOnceShim<'any, T>, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -834,7 +841,7 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { marker: PhantomData, }; - let data = f(data); + let data = f.call(data); mem::forget(lock_guard); @@ -855,10 +862,15 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { /// used as `MappedMutexGuard::try_map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map( + s: Self, + f: F, + ) -> Result< + MappedMutexGuard<'a, R, >::Output>, + MappedMutexGuard<'a, R, >::Error>, + > where - F: FnOnce(T) -> Result + 'a, - U: 'a, + for<'any> F: FnOnceResultShim<'any, T>, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -873,7 +885,7 @@ impl<'a, R: RawMutex + 'a, T: 'a> MappedMutexGuard<'a, R, T> { marker: PhantomData, }; - let out = f(data); + let out = f.call(data); mem::forget(lock_guard); diff --git a/lock_api/src/rwlock.rs b/lock_api/src/rwlock.rs index fd9ca9c0..630a0eec 100644 --- a/lock_api/src/rwlock.rs +++ b/lock_api/src/rwlock.rs @@ -5,12 +5,16 @@ // http://opensource.org/licenses/MIT>, at your option. This file may not be // copied, modified, or distributed except according to those terms. -use core::cell::UnsafeCell; -use core::fmt; -use core::marker::PhantomData; -use core::mem; -use core::ops::{Deref, DerefMut}; -use core::ptr; +use core::{ + cell::UnsafeCell, + fmt, + marker::PhantomData, + mem, + ops::{Deref, DerefMut}, + ptr, +}; + +use crate::{FnOnceOptionShim, FnOnceResultShim, FnOnceShim}; #[cfg(feature = "arc_lock")] use alloc::sync::Arc; @@ -1184,13 +1188,15 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { /// used as `RwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> + pub fn map( + s: Self, + f: F, + ) -> MappedRwLockReadGuard<'a, R, >::Output> where - F: FnOnce(&'a T) -> U + 'a, - U: 'a, + for<'any> F: FnOnceShim<'any, &'any T>, { let raw = &s.rwlock.raw; - let data = f(unsafe { &*s.rwlock.data.get() }); + let data = f.call(unsafe { &*s.rwlock.data.get() }); mem::forget(s); MappedRwLockReadGuard { raw, @@ -1209,17 +1215,20 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> { /// used as `RwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map( + s: Self, + f: F, + ) -> Result>::Output>, Self> where - F: FnOnce(&'a T) -> Option + 'a, - U: 'a, + for<'any> F: FnOnceOptionShim<'any, &'any T>, { let raw = &s.rwlock.raw; - let data = match f(unsafe { &*s.rwlock.data.get() }) { + let data = match f.call(unsafe { &*s.rwlock.data.get() }) { Some(data) => data, None => return Err(s), }; mem::forget(s); + Ok(MappedRwLockReadGuard { raw, data, @@ -1474,13 +1483,15 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { /// used as `RwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> + pub fn map( + s: Self, + f: F, + ) -> MappedRwLockWriteGuard<'a, R, >::Output> where - F: FnOnce(&mut T) -> U + 'a, - U: 'a, + for<'any> F: FnOnceShim<'any, &'any mut T>, { let raw = &s.rwlock.raw; - let data = f(unsafe { &mut *s.rwlock.data.get() }); + let data = f.call(unsafe { &mut *s.rwlock.data.get() }); mem::forget(s); MappedRwLockWriteGuard { raw, @@ -1499,13 +1510,15 @@ impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockWriteGuard<'a, R, T> { /// used as `RwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map( + s: Self, + f: F, + ) -> Result>::Output>, Self> where - F: FnOnce(&mut T) -> Option + 'a, - U: 'a, + for<'any> F: FnOnceOptionShim<'any, &'any mut T>, { let raw = &s.rwlock.raw; - let data = match f(unsafe { &mut *s.rwlock.data.get() }) { + let data = match f.call(unsafe { &mut *s.rwlock.data.get() }) { Some(data) => data, None => return Err(s), }; @@ -2338,10 +2351,9 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { /// used as `MappedRwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, >::Output> where - F: FnOnce(T) -> U + 'a, - U: 'a, + for<'any> F: FnOnceShim<'any, T>, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -2356,7 +2368,7 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { marker: PhantomData, }; - let data = f(data); + let data = f.call(data); mem::forget(lock_guard); @@ -2377,10 +2389,15 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { /// used as `MappedRwLockReadGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map( + s: Self, + f: F, + ) -> Result< + MappedRwLockReadGuard<'a, R, >::Output>, + MappedRwLockReadGuard<'a, R, >::Error>, + > where - F: FnOnce(T) -> Result + 'a, - U: 'a, + for<'any> F: FnOnceResultShim<'any, T>, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -2395,7 +2412,7 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockReadGuard<'a, R, T> { marker: PhantomData, }; - let out = f(data); + let out = f.call(data); mem::forget(lock_guard); @@ -2515,10 +2532,9 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { /// used as `MappedRwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, U> + pub fn map(s: Self, f: F) -> MappedRwLockWriteGuard<'a, R, >::Output> where - F: FnOnce(T) -> U, - U: 'a, + for<'any> F: FnOnceShim<'any, T>, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -2533,7 +2549,7 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { marker: PhantomData, }; - let data = f(data); + let data = f.call(data); mem::forget(lock_guard); @@ -2554,10 +2570,15 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { /// used as `MappedRwLockWriteGuard::map(...)`. A method would interfere with methods of /// the same name on the contents of the locked data. #[inline] - pub fn try_map(s: Self, f: F) -> Result, Self> + pub fn try_map( + s: Self, + f: F, + ) -> Result< + MappedRwLockWriteGuard<'a, R, >::Output>, + MappedRwLockWriteGuard<'a, R, >::Error>, + > where - F: FnOnce(T) -> Result, - U: 'a, + for<'any> F: FnOnceResultShim<'any, T>, { let (data, raw) = { let s = mem::ManuallyDrop::new(s); @@ -2572,7 +2593,7 @@ impl<'a, R: RawRwLock + 'a, T: 'a> MappedRwLockWriteGuard<'a, R, T> { marker: PhantomData, }; - let out = f(data); + let out = f.call(data); mem::forget(lock_guard); diff --git a/src/mutex.rs b/src/mutex.rs index 02fbff84..81e05b12 100644 --- a/src/mutex.rs +++ b/src/mutex.rs @@ -109,27 +109,85 @@ pub type MutexGuard<'a, T> = lock_api::MutexGuard<'a, RawMutex, T>; /// thread. pub type MappedMutexGuard<'a, T> = lock_api::MappedMutexGuard<'a, RawMutex, T>; -#[test] -fn test_map() { - use crate::{lock_api::RawMutex, Mutex, MutexGuard}; - - const FOO: usize = 0; - - static OUTER: Mutex<&usize> = Mutex::const_new(RawMutex::INIT, &FOO); - static M: Mutex = Mutex::const_new(RawMutex::INIT, 0); - - let guard = MutexGuard::map(M.lock(), |inner: &'static mut usize| { - *OUTER.lock() = inner; - }); - drop(guard); - - let outer: &usize = &*OUTER.lock(); - - assert_eq!(*outer, 0); - - *M.lock() = 1; - - assert_eq!(*outer, 1); +#[cfg(doctest)] +mod doctests { + //! ```rust,no_run + //! use parking_lot::{Mutex, MutexGuard}; + //! + //! let m = Mutex::new((0, 0)); + //! let guard = MutexGuard::map( + //! m.lock(), + //! { |inner| &mut inner.0 } + //! as fn(&mut (usize, usize)) -> &mut usize + //! ); + //! ``` + //! + //! ```rust,no_run + //! use parking_lot::{Mutex, MutexGuard}; + //! + //! struct MutWrapper<'a, T>(&'a mut T); + //! + //! let m = Mutex::new((0, 0)); + //! let guard = MutexGuard::map( + //! m.lock(), + //! { |inner| MutWrapper(&mut inner.0) } + //! as fn(&mut (usize, usize)) -> MutWrapper<'_, usize> + //! ); + //! ``` + //! + //! ```rust,compile_fail + //! use parking_lot::{lock_api::RawMutex, MappedMutexGuard, Mutex, MutexGuard}; + //! + //! const FOO: usize = 0; + //! + //! static OUTER: Mutex<&usize> = Mutex::const_new(RawMutex::INIT, &FOO); + //! static M: Mutex = Mutex::const_new(RawMutex::INIT, 0); + //! + //! let guard = MappedMutexGuard::map( + //! MutexGuard::map(M.lock(), { |inner| inner } as fn(&mut usize) -> &mut usize), + //! |inner: &mut _| { + //! let inner = &*inner; + //! *OUTER.lock() = inner; + //! }, + //! ); + //! drop(guard); + //! + //! let outer: &usize = &*OUTER.lock(); + //! + //! assert_eq!(*outer, 0); + //! + //! *M.lock() = 1; + //! + //! assert_eq!(*outer, 1); + //! ``` + //! + //! ```rust,compile_fail + //! use parking_lot::{lock_api::RawMutex, MappedMutexGuard, Mutex, MutexGuard}; + //! + //! const FOO: usize = 0; + //! + //! static OUTER: Mutex<&usize> = Mutex::const_new(RawMutex::INIT, &FOO); + //! static M: Mutex = Mutex::const_new(RawMutex::INIT, 0); + //! + //! struct MutWrapper<'a, T>(&'a mut T); + //! + //! let guard = MappedMutexGuard::map( + //! MutexGuard::map(M.lock(), { |inner| MutWrapper(inner) } as fn(&mut usize) -> MutWrapper<'_, usize>), + //! |inner: MutWrapper<'_, _>| { + //! let inner = &*inner.0; + //! *OUTER.lock() = inner; + //! }, + //! ); + //! drop(guard); + //! + //! let outer: &usize = &*OUTER.lock(); + //! + //! assert_eq!(*outer, 0); + //! + //! *M.lock() = 1; + //! + //! assert_eq!(*outer, 1); + //! ``` } #[cfg(test)] @@ -140,28 +198,6 @@ mod tests { use std::sync::Arc; use std::thread; - #[cfg(doctest)] - mod doctests { - //! ```rust,no_run - //! use parking_lot::{Mutex, MutexGuard}; - //! - //! let m = Mutex::new((0, 0)); - //! let guard = MutexGuard::map(m.lock(), |inner| &mut inner.0); - //! ``` - //! - //! ```rust,compile_fail - //! use parking_lot::{Mutex, MutexGuard, lock_api::RawMutex}; - //! - //! let mut outer = &mut 0; - //! static M: Mutex = Mutex::const_new(RawMutex::INIT, 0); - //! let guard = MutexGuard::map(M.lock(), |inner: &'static mut i32| { - //! outer = inner; - //! }); - //! drop(guard); - //! *outer = 1; - //! ``` - } - #[cfg(feature = "serde")] use bincode::{deserialize, serialize}; From 7f6f7781a2aa60efb7b8c16c035935440d6a0f61 Mon Sep 17 00:00:00 2001 From: Jack Fransham Date: Tue, 31 Aug 2021 11:04:20 +0200 Subject: [PATCH 7/7] Remove unnecessary `fn_ops` dependency --- lock_api/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/lock_api/Cargo.toml b/lock_api/Cargo.toml index 37729064..a5aaa313 100644 --- a/lock_api/Cargo.toml +++ b/lock_api/Cargo.toml @@ -10,7 +10,6 @@ categories = ["concurrency", "no-std"] edition = "2018" [dependencies] -fn_ops = "0.1" scopeguard = { version = "1.1.0", default-features = false } owning_ref = { version = "0.4.1", optional = true }