From ac53267d39e25658602a686e5094ab92576c03e1 Mon Sep 17 00:00:00 2001 From: marvin-j97 Date: Sat, 27 Jul 2024 13:24:54 +0200 Subject: [PATCH 1/4] feat: try_take --- src/lib.rs | 254 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 253 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 56e3bac..f434b92 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -210,9 +210,35 @@ impl From>> for RcMutexGuardian { } // **************************************************************************** -// impl ::take +// macros // **************************************************************************** +macro_rules! try_take { + ( $handle: ident, $guard:ty, $guardian:ident, $lfunc:ident ) => {{ + use std::mem; + use std::sync::TryLockError::{Poisoned, WouldBlock}; + + // We want to express that it's safe to keep the read guard around for as long as the + // Arc/Rc is around. Unfortunately, we can't say this directly with lifetimes, because + // we have to move the Arc/Rc below, which Rust doesn't know allows the borrow to + // continue. We therefore transmute to a 'static Guard, and ensure that any borrows we + // expose are bounded by the lifetime of the guardian (which also holds the Arc/Rc). + let lock: sync::TryLockResult<$guard> = unsafe { mem::transmute($handle.$lfunc()) }; + + match lock { + Ok(guard) => Ok(Some($guardian { + _handle: $handle, + inner: Some(guard), + })), + Err(WouldBlock) => Ok(None), + Err(Poisoned(guard)) => Err(sync::PoisonError::new(Some($guardian { + _handle: $handle, + inner: Some(guard.into_inner()), + }))), + } + }}; +} + macro_rules! take { ( $handle: ident, $guard:ty, $guardian:ident, $lfunc:ident ) => {{ use std::mem; @@ -237,6 +263,10 @@ macro_rules! take { }}; } +// **************************************************************************** +// impl +// **************************************************************************** + impl ArcRwLockReadGuardian { /// Locks the given rwlock with shared read access, blocking the current thread until it can be /// acquired. @@ -257,6 +287,17 @@ impl ArcRwLockReadGuardian { read ) } + + pub fn try_take( + handle: sync::Arc>, + ) -> sync::LockResult>> { + try_take!( + handle, + sync::RwLockReadGuard<'static, T>, + ArcRwLockReadGuardian, + try_read + ) + } } impl ArcRwLockWriteGuardian { @@ -283,6 +324,17 @@ impl ArcRwLockWriteGuardian { write ) } + + pub fn try_take( + handle: sync::Arc>, + ) -> sync::LockResult>> { + try_take!( + handle, + sync::RwLockWriteGuard<'static, T>, + ArcRwLockWriteGuardian, + try_write + ) + } } impl ArcMutexGuardian { @@ -301,6 +353,17 @@ impl ArcMutexGuardian { pub fn take(handle: sync::Arc>) -> sync::LockResult> { take!(handle, sync::MutexGuard<'static, T>, ArcMutexGuardian, lock) } + + pub fn try_take( + handle: sync::Arc>, + ) -> sync::LockResult>> { + try_take!( + handle, + sync::MutexGuard<'static, T>, + ArcMutexGuardian, + try_lock + ) + } } // And this is all the same as above, but with s/Arc/Rc/ @@ -325,6 +388,17 @@ impl RcRwLockReadGuardian { read ) } + + pub fn try_take( + handle: rc::Rc>, + ) -> sync::LockResult>> { + try_take!( + handle, + sync::RwLockReadGuard<'static, T>, + RcRwLockReadGuardian, + try_read + ) + } } impl RcRwLockWriteGuardian { @@ -351,6 +425,17 @@ impl RcRwLockWriteGuardian { write ) } + + pub fn try_take( + handle: rc::Rc>, + ) -> sync::LockResult>> { + try_take!( + handle, + sync::RwLockWriteGuard<'static, T>, + RcRwLockWriteGuardian, + try_write + ) + } } impl RcMutexGuardian { @@ -369,6 +454,17 @@ impl RcMutexGuardian { pub fn take(handle: rc::Rc>) -> sync::LockResult> { take!(handle, sync::MutexGuard<'static, T>, RcMutexGuardian, lock) } + + pub fn try_take( + handle: rc::Rc>, + ) -> sync::LockResult>> { + try_take!( + handle, + sync::MutexGuard<'static, T>, + RcMutexGuardian, + try_lock + ) + } } // **************************************************************************** @@ -495,6 +591,49 @@ mod tests { assert_eq!(&*x, &false); } + #[test] + fn arc_rw_try() { + let base = sync::Arc::new(sync::RwLock::new(true)); + + let mut x = ArcRwLockWriteGuardian::try_take(base.clone()) + .unwrap() + .unwrap(); + + // guardian dereferences correctly + assert_eq!(&*x, &true); + + // guardian can write + *x = false; + assert_eq!(&*x, &false); + + // guardian holds write lock + assert!(base.try_read().is_err(), "guardian holds write lock"); + + // guardian can be moved + let x_ = x; + assert_eq!(&*x_, &false); + + // moving guardian does not release lock + assert!(base.try_read().is_err(), "guardian still holds write lock"); + + // try_take returns None if it would block + assert!(ArcRwLockWriteGuardian::try_take(base.clone()) + .unwrap() + .is_none()); + + assert!(ArcRwLockReadGuardian::try_take(base.clone()) + .unwrap() + .is_none()); + + // dropping guardian drops write lock + drop(x_); + assert!(base.try_read().is_ok(), "guardian drops write lock"); + + // guardian works even after all other Arcs have been dropped + let x = ArcRwLockWriteGuardian::take(base).unwrap(); + assert_eq!(&*x, &false); + } + #[test] fn arc_mu() { let base = sync::Arc::new(sync::Mutex::new(true)); @@ -527,6 +666,41 @@ mod tests { assert_eq!(&*x, &false); } + #[test] + fn arc_mu_try() { + let base = sync::Arc::new(sync::Mutex::new(true)); + + let mut x = ArcMutexGuardian::try_take(base.clone()).unwrap().unwrap(); + + // guardian dereferences correctly + assert_eq!(&*x, &true); + + // guardian can write + *x = false; + assert_eq!(&*x, &false); + + // guardian holds lock + assert!(base.try_lock().is_err(), "guardian holds lock"); + + // guardian can be moved + let x_ = x; + assert_eq!(&*x_, &false); + + // moving guardian does not release lock + assert!(base.try_lock().is_err(), "guardian still holds lock"); + + // try_take returns None if it would block + assert!(ArcMutexGuardian::try_take(base.clone()).unwrap().is_none()); + + // dropping guardian drops lock + drop(x_); + assert!(base.try_lock().is_ok(), "guardian drops lock"); + + // guardian works even after all other Arcs have been dropped + let x = ArcMutexGuardian::take(base).unwrap(); + assert_eq!(&*x, &false); + } + #[test] fn rc_rw_read() { let base = rc::Rc::new(sync::RwLock::new(true)); @@ -601,6 +775,49 @@ mod tests { assert_eq!(&*x, &false); } + #[test] + fn rc_rw_try() { + let base = rc::Rc::new(sync::RwLock::new(true)); + + let mut x = RcRwLockWriteGuardian::try_take(base.clone()) + .unwrap() + .unwrap(); + + // guardian dereferences correctly + assert_eq!(&*x, &true); + + // guardian can write + *x = false; + assert_eq!(&*x, &false); + + // guardian holds write lock + assert!(base.try_read().is_err(), "guardian holds write lock"); + + // guardian can be moved + let x_ = x; + assert_eq!(&*x_, &false); + + // moving guardian does not release lock + assert!(base.try_read().is_err(), "guardian still holds write lock"); + + // try_take returns None if it would block + assert!(RcRwLockWriteGuardian::try_take(base.clone()) + .unwrap() + .is_none()); + + assert!(RcRwLockReadGuardian::try_take(base.clone()) + .unwrap() + .is_none()); + + // dropping guardian drops write lock + drop(x_); + assert!(base.try_read().is_ok(), "guardian drops write lock"); + + // guardian works even after all other Rcs have been dropped + let x = RcRwLockWriteGuardian::take(base).unwrap(); + assert_eq!(&*x, &false); + } + #[test] fn rc_mu() { let base = rc::Rc::new(sync::Mutex::new(true)); @@ -632,4 +849,39 @@ mod tests { let x = RcMutexGuardian::take(base).unwrap(); assert_eq!(&*x, &false); } + + #[test] + fn rc_mu_try() { + let base = rc::Rc::new(sync::Mutex::new(true)); + + let mut x = RcMutexGuardian::take(base.clone()).unwrap(); + + // guardian dereferences correctly + assert_eq!(&*x, &true); + + // guardian can write + *x = false; + assert_eq!(&*x, &false); + + // guardian holds lock + assert!(base.try_lock().is_err(), "guardian holds lock"); + + // guardian can be moved + let x_ = x; + assert_eq!(&*x_, &false); + + // moving guardian does not release lock + assert!(base.try_lock().is_err(), "guardian still holds lock"); + + // try_take returns None if it would block + assert!(RcMutexGuardian::try_take(base.clone()).unwrap().is_none()); + + // dropping guardian drops lock + drop(x_); + assert!(base.try_lock().is_ok(), "guardian drops lock"); + + // guardian works even after all other Rcs have been dropped + let x = RcMutexGuardian::take(base).unwrap(); + assert_eq!(&*x, &false); + } } From 8a54e0697e418d40e8caf66f8826a7a6e1acabed Mon Sep 17 00:00:00 2001 From: marvin-j97 Date: Sat, 27 Jul 2024 13:26:53 +0200 Subject: [PATCH 2/4] transpose try_take result --- src/lib.rs | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f434b92..b812e16 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -226,12 +226,12 @@ macro_rules! try_take { let lock: sync::TryLockResult<$guard> = unsafe { mem::transmute($handle.$lfunc()) }; match lock { - Ok(guard) => Ok(Some($guardian { + Ok(guard) => Some(Ok($guardian { _handle: $handle, inner: Some(guard), })), - Err(WouldBlock) => Ok(None), - Err(Poisoned(guard)) => Err(sync::PoisonError::new(Some($guardian { + Err(WouldBlock) => None, + Err(Poisoned(guard)) => Some(Err(sync::PoisonError::new($guardian { _handle: $handle, inner: Some(guard.into_inner()), }))), @@ -290,7 +290,7 @@ impl ArcRwLockReadGuardian { pub fn try_take( handle: sync::Arc>, - ) -> sync::LockResult>> { + ) -> Option>> { try_take!( handle, sync::RwLockReadGuard<'static, T>, @@ -327,7 +327,7 @@ impl ArcRwLockWriteGuardian { pub fn try_take( handle: sync::Arc>, - ) -> sync::LockResult>> { + ) -> Option>> { try_take!( handle, sync::RwLockWriteGuard<'static, T>, @@ -356,7 +356,7 @@ impl ArcMutexGuardian { pub fn try_take( handle: sync::Arc>, - ) -> sync::LockResult>> { + ) -> Option>> { try_take!( handle, sync::MutexGuard<'static, T>, @@ -391,7 +391,7 @@ impl RcRwLockReadGuardian { pub fn try_take( handle: rc::Rc>, - ) -> sync::LockResult>> { + ) -> Option>> { try_take!( handle, sync::RwLockReadGuard<'static, T>, @@ -428,7 +428,7 @@ impl RcRwLockWriteGuardian { pub fn try_take( handle: rc::Rc>, - ) -> sync::LockResult>> { + ) -> Option>> { try_take!( handle, sync::RwLockWriteGuard<'static, T>, @@ -457,7 +457,7 @@ impl RcMutexGuardian { pub fn try_take( handle: rc::Rc>, - ) -> sync::LockResult>> { + ) -> Option>> { try_take!( handle, sync::MutexGuard<'static, T>, @@ -617,13 +617,9 @@ mod tests { assert!(base.try_read().is_err(), "guardian still holds write lock"); // try_take returns None if it would block - assert!(ArcRwLockWriteGuardian::try_take(base.clone()) - .unwrap() - .is_none()); + assert!(ArcRwLockWriteGuardian::try_take(base.clone()).is_none()); - assert!(ArcRwLockReadGuardian::try_take(base.clone()) - .unwrap() - .is_none()); + assert!(ArcRwLockReadGuardian::try_take(base.clone()).is_none()); // dropping guardian drops write lock drop(x_); @@ -690,7 +686,7 @@ mod tests { assert!(base.try_lock().is_err(), "guardian still holds lock"); // try_take returns None if it would block - assert!(ArcMutexGuardian::try_take(base.clone()).unwrap().is_none()); + assert!(ArcMutexGuardian::try_take(base.clone()).is_none()); // dropping guardian drops lock drop(x_); @@ -801,13 +797,9 @@ mod tests { assert!(base.try_read().is_err(), "guardian still holds write lock"); // try_take returns None if it would block - assert!(RcRwLockWriteGuardian::try_take(base.clone()) - .unwrap() - .is_none()); + assert!(RcRwLockWriteGuardian::try_take(base.clone()).is_none()); - assert!(RcRwLockReadGuardian::try_take(base.clone()) - .unwrap() - .is_none()); + assert!(RcRwLockReadGuardian::try_take(base.clone()).is_none()); // dropping guardian drops write lock drop(x_); @@ -874,7 +866,7 @@ mod tests { assert!(base.try_lock().is_err(), "guardian still holds lock"); // try_take returns None if it would block - assert!(RcMutexGuardian::try_take(base.clone()).unwrap().is_none()); + assert!(RcMutexGuardian::try_take(base.clone()).is_none()); // dropping guardian drops lock drop(x_); From e41f4bfda06458f154f6994537805018ab54a773 Mon Sep 17 00:00:00 2001 From: Marvin <33938500+marvin-j97@users.noreply.github.com> Date: Sun, 4 Aug 2024 14:12:54 +0200 Subject: [PATCH 3/4] docs for try_take --- src/lib.rs | 90 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b812e16..92947c3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -213,52 +213,48 @@ impl From>> for RcMutexGuardian { // macros // **************************************************************************** -macro_rules! try_take { +macro_rules! take { ( $handle: ident, $guard:ty, $guardian:ident, $lfunc:ident ) => {{ use std::mem; - use std::sync::TryLockError::{Poisoned, WouldBlock}; // We want to express that it's safe to keep the read guard around for as long as the // Arc/Rc is around. Unfortunately, we can't say this directly with lifetimes, because // we have to move the Arc/Rc below, which Rust doesn't know allows the borrow to // continue. We therefore transmute to a 'static Guard, and ensure that any borrows we // expose are bounded by the lifetime of the guardian (which also holds the Arc/Rc). - let lock: sync::TryLockResult<$guard> = unsafe { mem::transmute($handle.$lfunc()) }; + let lock: sync::LockResult<$guard> = unsafe { mem::transmute($handle.$lfunc()) }; match lock { - Ok(guard) => Some(Ok($guardian { + Ok(guard) => Ok($guardian { _handle: $handle, inner: Some(guard), - })), - Err(WouldBlock) => None, - Err(Poisoned(guard)) => Some(Err(sync::PoisonError::new($guardian { + }), + Err(guard) => Err(sync::PoisonError::new($guardian { _handle: $handle, inner: Some(guard.into_inner()), - }))), + })), } }}; } -macro_rules! take { +macro_rules! try_take { ( $handle: ident, $guard:ty, $guardian:ident, $lfunc:ident ) => {{ use std::mem; + use std::sync::TryLockError::{Poisoned, WouldBlock}; - // We want to express that it's safe to keep the read guard around for as long as the - // Arc/Rc is around. Unfortunately, we can't say this directly with lifetimes, because - // we have to move the Arc/Rc below, which Rust doesn't know allows the borrow to - // continue. We therefore transmute to a 'static Guard, and ensure that any borrows we - // expose are bounded by the lifetime of the guardian (which also holds the Arc/Rc). - let lock: sync::LockResult<$guard> = unsafe { mem::transmute($handle.$lfunc()) }; + // Safe following the same reasoning as in take!. + let lock: sync::TryLockResult<$guard> = unsafe { mem::transmute($handle.$lfunc()) }; match lock { - Ok(guard) => Ok($guardian { + Ok(guard) => Some(Ok($guardian { _handle: $handle, inner: Some(guard), - }), - Err(guard) => Err(sync::PoisonError::new($guardian { + })), + Err(WouldBlock) => None, + Err(Poisoned(guard)) => Some(Err(sync::PoisonError::new($guardian { _handle: $handle, inner: Some(guard.into_inner()), - })), + }))), } }}; } @@ -288,6 +284,16 @@ impl ArcRwLockReadGuardian { ) } + /// Attempts to acquire this rwlock with shared read access. + /// + /// If the access could not be granted at this time, then `None` is returned. + /// Otherwise, an RAII guard is returned which will release the shared access when it is dropped. + /// The guardian also holds a strong reference to the lock's `Arc`, which is dropped when the + /// guard is. + /// + /// This function does not block. + /// + /// This function does not provide any guarantees with respect to the ordering of whether contentious readers or writers will acquire the lock first. pub fn try_take( handle: sync::Arc>, ) -> Option>> { @@ -325,6 +331,16 @@ impl ArcRwLockWriteGuardian { ) } + /// Attempts to lock this rwlock with exclusive write access. + /// + /// If the access could not be granted at this time, then `None` is returned. + /// Otherwise, an RAII guard is returned, which will drop the write access of this rwlock when dropped. + /// The guardian also holds a strong reference to the lock's `Arc`, which is dropped when the + /// guard is. + /// + /// This function does not block. + /// + /// This function does not provide any guarantees with respect to the ordering of whether contentious readers or writers will acquire the lock first. pub fn try_take( handle: sync::Arc>, ) -> Option>> { @@ -354,6 +370,14 @@ impl ArcMutexGuardian { take!(handle, sync::MutexGuard<'static, T>, ArcMutexGuardian, lock) } + /// Attempts to acquire this lock. + /// + /// If the lock could not be acquired at this time, then `None` is returned. + /// Otherwise, an RAII guard is returned. The lock will be unlocked when the guard is dropped. + /// The guardian also holds a strong reference to the lock's `Arc`, which is dropped + /// when the guard is. + /// + /// This function does not block. pub fn try_take( handle: sync::Arc>, ) -> Option>> { @@ -389,6 +413,16 @@ impl RcRwLockReadGuardian { ) } + /// Attempts to acquire this rwlock with shared read access. + /// + /// If the access could not be granted at this time, then `None` is returned. + /// Otherwise, an RAII guard is returned which will release the shared access when it is dropped. + /// The guardian also holds a strong reference to the lock's `Rc`, which is dropped when the + /// guard is. + /// + /// This function does not block. + /// + /// This function does not provide any guarantees with respect to the ordering of whether contentious readers or writers will acquire the lock first. pub fn try_take( handle: rc::Rc>, ) -> Option>> { @@ -426,6 +460,16 @@ impl RcRwLockWriteGuardian { ) } + /// Attempts to lock this rwlock with exclusive write access. + /// + /// If the access could not be granted at this time, then `None` is returned. + /// Otherwise, an RAII guard is returned, which will drop the write access of this rwlock when dropped. + /// The guardian also holds a strong reference to the lock's `Rc`, which is dropped when the + /// guard is. + /// + /// This function does not block. + /// + /// This function does not provide any guarantees with respect to the ordering of whether contentious readers or writers will acquire the lock first. pub fn try_take( handle: rc::Rc>, ) -> Option>> { @@ -455,6 +499,14 @@ impl RcMutexGuardian { take!(handle, sync::MutexGuard<'static, T>, RcMutexGuardian, lock) } + /// Attempts to acquire this lock. + /// + /// If the lock could not be acquired at this time, then `None` is returned. + /// Otherwise, an RAII guard is returned. The lock will be unlocked when the guard is dropped. + /// The guardian also holds a strong reference to the lock's `Rc`, which is dropped + /// when the guard is. + /// + /// This function does not block. pub fn try_take( handle: rc::Rc>, ) -> Option>> { From 76f27562386dda4b84ea8f4570fd6d5bc328435d Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 6 Aug 2024 08:30:14 +0200 Subject: [PATCH 4/4] Apply suggestions from code review --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 92947c3..1cd06da 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -332,7 +332,7 @@ impl ArcRwLockWriteGuardian { } /// Attempts to lock this rwlock with exclusive write access. - /// + /// /// If the access could not be granted at this time, then `None` is returned. /// Otherwise, an RAII guard is returned, which will drop the write access of this rwlock when dropped. /// The guardian also holds a strong reference to the lock's `Arc`, which is dropped when the @@ -461,7 +461,7 @@ impl RcRwLockWriteGuardian { } /// Attempts to lock this rwlock with exclusive write access. - /// + /// /// If the access could not be granted at this time, then `None` is returned. /// Otherwise, an RAII guard is returned, which will drop the write access of this rwlock when dropped. /// The guardian also holds a strong reference to the lock's `Rc`, which is dropped when the