From 30d0f08b6073e9c2e545a3567838a9e1e885fea2 Mon Sep 17 00:00:00 2001 From: Mica White Date: Mon, 23 Dec 2024 15:31:07 -0500 Subject: Remove scopeguard The scopeguard crate was being used for its `defer_on_unwind` macro. The problem was that it runs even if the runtime was already panicking. There aren't any changes to the macro which could have fixed this. I instead wrote my own function to check for a specific panicking closure. --- src/collection/boxed.rs | 25 +------ src/collection/owned.rs | 23 +----- src/collection/ref.rs | 23 +----- src/collection/retry.rs | 193 ++++++++++++++++++++++++------------------------ src/collection/utils.rs | 127 ++++++++++++++++++++----------- 5 files changed, 186 insertions(+), 205 deletions(-) (limited to 'src/collection') diff --git a/src/collection/boxed.rs b/src/collection/boxed.rs index 3cfc336..0014cc3 100644 --- a/src/collection/boxed.rs +++ b/src/collection/boxed.rs @@ -1,5 +1,5 @@ use std::alloc::Layout; -use std::cell::{RefCell, UnsafeCell}; +use std::cell::UnsafeCell; use std::fmt::Debug; use std::marker::PhantomData; @@ -28,16 +28,7 @@ unsafe impl RawLock for BoxedLockCollection { } unsafe fn raw_lock(&self) { - let locks = self.locks(); - let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_locks_from_panic(&locked) - }; - - for lock in self.locks() { - lock.raw_lock(); - locked.borrow_mut().push(*lock); - } + utils::ordered_lock(self.locks()) } unsafe fn raw_try_lock(&self) -> bool { @@ -51,16 +42,7 @@ unsafe impl RawLock for BoxedLockCollection { } unsafe fn raw_read(&self) { - let locks = self.locks(); - let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_reads_from_panic(&locked) - }; - - for lock in self.locks() { - lock.raw_read(); - locked.borrow_mut().push(*lock); - } + utils::ordered_read(self.locks()); } unsafe fn raw_try_read(&self) -> bool { @@ -141,6 +123,7 @@ unsafe impl Send for BoxedLockCollection {} unsafe impl Sync for BoxedLockCollection {} impl Drop for BoxedLockCollection { + #[mutants::skip] fn drop(&mut self) { self.locks.clear(); diff --git a/src/collection/owned.rs b/src/collection/owned.rs index 3ea93b6..69680f4 100644 --- a/src/collection/owned.rs +++ b/src/collection/owned.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::marker::PhantomData; use crate::lockable::{Lockable, LockableIntoInner, OwnedLockable, RawLock, Sharable}; @@ -21,16 +20,7 @@ unsafe impl RawLock for OwnedLockCollection { } unsafe fn raw_lock(&self) { - let locks = get_locks(&self.data); - let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_locks_from_panic(&locked) - }; - - for lock in locks { - lock.raw_lock(); - locked.borrow_mut().push(lock); - } + utils::ordered_lock(&get_locks(&self.data)) } unsafe fn raw_try_lock(&self) -> bool { @@ -46,16 +36,7 @@ unsafe impl RawLock for OwnedLockCollection { } unsafe fn raw_read(&self) { - let locks = get_locks(&self.data); - let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_reads_from_panic(&locked) - }; - - for lock in locks { - lock.raw_read(); - locked.borrow_mut().push(lock); - } + utils::ordered_read(&get_locks(&self.data)) } unsafe fn raw_try_read(&self) -> bool { diff --git a/src/collection/ref.rs b/src/collection/ref.rs index 31ef173..b0b142e 100644 --- a/src/collection/ref.rs +++ b/src/collection/ref.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::fmt::Debug; use std::marker::PhantomData; @@ -53,16 +52,7 @@ unsafe impl RawLock for RefLockCollection<'_, L> { } unsafe fn raw_lock(&self) { - let locks = &self.locks; - let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_locks_from_panic(&locked) - }; - - for lock in &self.locks { - lock.raw_lock(); - locked.borrow_mut().push(*lock); - } + utils::ordered_lock(&self.locks) } unsafe fn raw_try_lock(&self) -> bool { @@ -76,16 +66,7 @@ unsafe impl RawLock for RefLockCollection<'_, L> { } unsafe fn raw_read(&self) { - let locks = &self.locks; - let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_reads_from_panic(&locked) - }; - - for lock in &self.locks { - lock.raw_read(); - locked.borrow_mut().push(*lock); - } + utils::ordered_read(&self.locks) } unsafe fn raw_try_read(&self) -> bool { diff --git a/src/collection/retry.rs b/src/collection/retry.rs index fb2401e..28602f2 100644 --- a/src/collection/retry.rs +++ b/src/collection/retry.rs @@ -1,4 +1,5 @@ use crate::collection::utils; +use crate::handle_unwind::handle_unwind; use crate::lockable::{ Lockable, LockableAsMut, LockableIntoInner, OwnedLockable, RawLock, Sharable, }; @@ -50,47 +51,46 @@ unsafe impl RawLock for RetryingLockCollection { } let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_locks_from_panic(&locked) - }; - - unsafe { - 'outer: loop { - // safety: we have the thread key - locks[first_index].raw_lock(); - for (i, lock) in locks.iter().enumerate() { - if i == first_index { - continue; - } - - // If the lock has been killed, then this returns false - // instead of panicking. This sounds like a problem, but if - // it does return false, then the lock function is called - // immediately after, causing a panic + handle_unwind( + || unsafe { + 'outer: loop { // safety: we have the thread key - if lock.raw_try_lock() { - locked.borrow_mut().push(*lock) - } else { - for lock in locks.iter().take(i) { - // safety: we already locked all of these - lock.raw_unlock(); + locks[first_index].raw_lock(); + for (i, lock) in locks.iter().enumerate() { + if i == first_index { + continue; } - if first_index >= i { - // safety: this is already locked and can't be unlocked - // by the previous loop - locks[first_index].raw_unlock(); + // If the lock has been killed, then this returns false + // instead of panicking. This sounds like a problem, but if + // it does return false, then the lock function is called + // immediately after, causing a panic + // safety: we have the thread key + if lock.raw_try_lock() { + locked.borrow_mut().push(*lock) + } else { + for lock in locks.iter().take(i) { + // safety: we already locked all of these + lock.raw_unlock(); + } + + if first_index >= i { + // safety: this is already locked and can't be unlocked + // by the previous loop + locks[first_index].raw_unlock(); + } + + first_index = i; + continue 'outer; } - - first_index = i; - continue 'outer; } - } - // safety: we locked all the data - break; - } - }; + // safety: we locked all the data + break; + } + }, + || utils::attempt_to_recover_locks_from_panic(&locked), + ) } unsafe fn raw_try_lock(&self) -> bool { @@ -101,26 +101,25 @@ unsafe impl RawLock for RetryingLockCollection { } let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_locks_from_panic(&locked) - }; - - unsafe { - for (i, lock) in locks.iter().enumerate() { - // safety: we have the thread key - if lock.raw_try_lock() { - locked.borrow_mut().push(*lock); - } else { - for lock in locks.iter().take(i) { - // safety: we already locked all of these - lock.raw_unlock(); + handle_unwind( + || unsafe { + for (i, lock) in locks.iter().enumerate() { + // safety: we have the thread key + if lock.raw_try_lock() { + locked.borrow_mut().push(*lock); + } else { + for lock in locks.iter().take(i) { + // safety: we already locked all of these + lock.raw_unlock(); + } + return false; } - return false; } - } - } - true + true + }, + || utils::attempt_to_recover_locks_from_panic(&locked), + ) } unsafe fn raw_unlock(&self) { @@ -140,41 +139,40 @@ unsafe impl RawLock for RetryingLockCollection { } let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_reads_from_panic(&locked) - }; - - 'outer: loop { - // safety: we have the thread key - locks[first_index].raw_read(); - for (i, lock) in locks.iter().enumerate() { - if i == first_index { - continue; - } - + handle_unwind( + || 'outer: loop { // safety: we have the thread key - if lock.raw_try_read() { - locked.borrow_mut().push(*lock); - } else { - for lock in locks.iter().take(i) { - // safety: we already locked all of these - lock.raw_unlock_read(); + locks[first_index].raw_read(); + for (i, lock) in locks.iter().enumerate() { + if i == first_index { + continue; } - if first_index >= i { - // safety: this is already locked and can't be unlocked - // by the previous loop - locks[first_index].raw_unlock_read(); - } + // safety: we have the thread key + if lock.raw_try_read() { + locked.borrow_mut().push(*lock); + } else { + for lock in locks.iter().take(i) { + // safety: we already locked all of these + lock.raw_unlock_read(); + } - first_index = i; - continue 'outer; + if first_index >= i { + // safety: this is already locked and can't be unlocked + // by the previous loop + locks[first_index].raw_unlock_read(); + } + + first_index = i; + continue 'outer; + } } - } - // safety: we locked all the data - break; - } + // safety: we locked all the data + break; + }, + || utils::attempt_to_recover_reads_from_panic(&locked), + ) } unsafe fn raw_try_read(&self) -> bool { @@ -185,26 +183,25 @@ unsafe impl RawLock for RetryingLockCollection { } let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - utils::attempt_to_recover_reads_from_panic(&locked) - }; - - unsafe { - for (i, lock) in locks.iter().enumerate() { - // safety: we have the thread key - if lock.raw_try_read() { - locked.borrow_mut().push(*lock); - } else { - for lock in locks.iter().take(i) { - // safety: we already locked all of these - lock.raw_unlock_read(); + handle_unwind( + || unsafe { + for (i, lock) in locks.iter().enumerate() { + // safety: we have the thread key + if lock.raw_try_read() { + locked.borrow_mut().push(*lock); + } else { + for lock in locks.iter().take(i) { + // safety: we already locked all of these + lock.raw_unlock_read(); + } + return false; } - return false; } - } - } - true + true + }, + || utils::attempt_to_recover_reads_from_panic(&locked), + ) } unsafe fn raw_unlock_read(&self) { diff --git a/src/collection/utils.rs b/src/collection/utils.rs index d845450..f418386 100644 --- a/src/collection/utils.rs +++ b/src/collection/utils.rs @@ -1,74 +1,113 @@ use std::cell::RefCell; +use crate::handle_unwind::handle_unwind; use crate::lockable::RawLock; +pub unsafe fn ordered_lock(locks: &[&dyn RawLock]) { + let locked = RefCell::new(Vec::with_capacity(locks.len())); + + handle_unwind( + || { + for lock in locks { + lock.raw_lock(); + locked.borrow_mut().push(*lock); + } + }, + || attempt_to_recover_locks_from_panic(&locked), + ) +} + +pub unsafe fn ordered_read(locks: &[&dyn RawLock]) { + let locked = RefCell::new(Vec::with_capacity(locks.len())); + + handle_unwind( + || { + for lock in locks { + lock.raw_read(); + locked.borrow_mut().push(*lock); + } + }, + || attempt_to_recover_reads_from_panic(&locked), + ) +} + /// Locks the locks in the order they are given. This causes deadlock if the /// locks contain duplicates, or if this is called by multiple threads with the /// locks in different orders. pub unsafe fn ordered_try_lock(locks: &[&dyn RawLock]) -> bool { let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - // safety: everything in locked is locked - attempt_to_recover_locks_from_panic(&locked) - }; - unsafe { - for (i, lock) in locks.iter().enumerate() { - // safety: we have the thread key - if lock.raw_try_lock() { - locked.borrow_mut().push(*lock); - } else { - for lock in &locks[0..i] { - // safety: this lock was already acquired - lock.raw_unlock(); + handle_unwind( + || unsafe { + for (i, lock) in locks.iter().enumerate() { + // safety: we have the thread key + if lock.raw_try_lock() { + locked.borrow_mut().push(*lock); + } else { + for lock in &locks[0..i] { + // safety: this lock was already acquired + lock.raw_unlock(); + } + return false; } - return false; } - } - true - } + true + }, + || + // safety: everything in locked is locked + attempt_to_recover_locks_from_panic(&locked), + ) } /// Locks the locks in the order they are given. This causes deadlock if this /// is called by multiple threads with the locks in different orders. pub unsafe fn ordered_try_read(locks: &[&dyn RawLock]) -> bool { let locked = RefCell::new(Vec::with_capacity(locks.len())); - scopeguard::defer_on_unwind! { - // safety: everything in locked is locked - attempt_to_recover_reads_from_panic(&locked) - }; - unsafe { - for (i, lock) in locks.iter().enumerate() { - // safety: we have the thread key - if lock.raw_try_read() { - locked.borrow_mut().push(*lock); - } else { - for lock in &locks[0..i] { - // safety: this lock was already acquired - lock.raw_unlock_read(); + handle_unwind( + || unsafe { + for (i, lock) in locks.iter().enumerate() { + // safety: we have the thread key + if lock.raw_try_read() { + locked.borrow_mut().push(*lock); + } else { + for lock in &locks[0..i] { + // safety: this lock was already acquired + lock.raw_unlock_read(); + } + return false; } - return false; } - } - true - } + true + }, + || + // safety: everything in locked is locked + attempt_to_recover_reads_from_panic(&locked), + ) } pub unsafe fn attempt_to_recover_locks_from_panic(locked: &RefCell>) { - scopeguard::defer_on_unwind! { locked.borrow().iter().for_each(|l| l.kill()); }; - let mut locked = locked.borrow_mut(); - while let Some(locked_lock) = locked.pop() { - locked_lock.raw_unlock(); - } + handle_unwind( + || { + let mut locked = locked.borrow_mut(); + while let Some(locked_lock) = locked.pop() { + locked_lock.raw_unlock(); + } + }, + || locked.borrow().iter().for_each(|l| l.kill()), + ) } pub unsafe fn attempt_to_recover_reads_from_panic(locked: &RefCell>) { - scopeguard::defer_on_unwind! { locked.borrow().iter().for_each(|l| l.kill()); }; - let mut locked = locked.borrow_mut(); - while let Some(locked_lock) = locked.pop() { - locked_lock.raw_unlock_read(); - } + handle_unwind( + || { + let mut locked = locked.borrow_mut(); + while let Some(locked_lock) = locked.pop() { + locked_lock.raw_unlock_read(); + } + }, + || locked.borrow().iter().for_each(|l| l.kill()), + ) } -- cgit v1.2.3