From b24adcbd1497aa657b39ccbe832219518ff159f9 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sat, 6 Dec 2025 08:04:46 -0500 Subject: [PATCH] Add panic-on-poison to no-`std` `std_shims::sync::Mutex` We already had this behavior on `std`. It was omitted when no-`std` due to deferring to `spin::Mutex`, which does not track poisoning at all. This increases the parity of the two. Part of https://github.com/serai-dex/serai/issues/698. --- common/std-shims/src/sync.rs | 60 +++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/common/std-shims/src/sync.rs b/common/std-shims/src/sync.rs index 17061cca..f3ea3bbb 100644 --- a/common/std-shims/src/sync.rs +++ b/common/std-shims/src/sync.rs @@ -6,12 +6,63 @@ pub use std::sync::{Arc, Weak}; mod mutex_shim { #[cfg(not(feature = "std"))] - pub use spin::{Mutex, MutexGuard}; + mod spin_mutex { + use core::ops::{Deref, DerefMut}; + + // We wrap this in an `Option` so we can consider `None` as poisoned + pub(super) struct Mutex(spin::Mutex>); + + /// An acquired view of a `Mutex`. + pub struct MutexGuard<'mutex, T> { + mutex: spin::MutexGuard<'mutex, Option>, + // This is `Some` for the lifetime of this guard, and is only represented as an `Option` due + // to needing to move it on `Drop` (which solely gives us a mutable reference to `self`) + value: Option, + } + + impl Mutex { + pub(super) const fn new(value: T) -> Self { + Self(spin::Mutex::new(Some(value))) + } + + pub(super) fn lock(&self) -> MutexGuard<'_, T> { + let mut mutex = self.0.lock(); + // Take from the `Mutex` so future acquisitions will see `None` unless this is restored + let value = mutex.take(); + // Check the prior acquisition did in fact restore the value + if value.is_none() { + panic!("locking a `spin::Mutex` held by a thread which panicked"); + } + MutexGuard { mutex, value } + } + } + + impl Deref for MutexGuard<'_, T> { + type Target = T; + fn deref(&self) -> &T { + self.value.as_ref().expect("no value yet checked upon lock acquisition") + } + } + impl DerefMut for MutexGuard<'_, T> { + fn deref_mut(&mut self) -> &mut T { + self.value.as_mut().expect("no value yet checked upon lock acquisition") + } + } + + impl<'mutex, T> Drop for MutexGuard<'mutex, T> { + fn drop(&mut self) { + // Restore the value + *self.mutex = self.value.take(); + } + } + } + #[cfg(not(feature = "std"))] + pub use spin_mutex::*; + #[cfg(feature = "std")] pub use std::sync::{Mutex, MutexGuard}; /// A shimmed `Mutex` with an API mutual to `spin` and `std`. - #[derive(Default, Debug)] pub struct ShimMutex(Mutex); impl ShimMutex { /// Construct a new `Mutex`. @@ -21,8 +72,9 @@ mod mutex_shim { /// Acquire a lock on the contents of the `Mutex`. /// - /// On no-`std` environments, this may spin until the lock is acquired. On `std` environments, - /// this may panic if the `Mutex` was poisoned. + /// This will panic if the `Mutex` was poisoned. + /// + /// On no-`std` environments, the implementation presumably defers to that of a spin lock. pub fn lock(&self) -> MutexGuard<'_, T> { #[cfg(feature = "std")] let res = self.0.lock().unwrap();