From 25ae542ade0442e9d77556d641ad35786af69af3 Mon Sep 17 00:00:00 2001 From: Bert Peters Date: Fri, 17 Jan 2025 22:31:40 +0100 Subject: [PATCH 1/3] Add support for LazyLock --- Cargo.toml | 3 + build.rs | 10 +++ src/lib.rs | 8 +++ src/parkinglot.rs | 6 +- src/stdsync.rs | 25 ++++--- src/stdsync/tracing/lazy_lock.rs | 113 +++++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 13 deletions(-) create mode 100644 build.rs create mode 100644 src/stdsync/tracing/lazy_lock.rs diff --git a/Cargo.toml b/Cargo.toml index decc54b..4b28bc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,3 +36,6 @@ backtraces = [] # Feature names do not match crate names pending namespaced features. lockapi = ["lock_api"] parkinglot = ["parking_lot", "lockapi"] + +[build-dependencies] +autocfg = "1.4.0" diff --git a/build.rs b/build.rs new file mode 100644 index 0000000..098bf58 --- /dev/null +++ b/build.rs @@ -0,0 +1,10 @@ +use autocfg::AutoCfg; + +fn main() { + // To avoid bumping MSRV unnecessarily, we can sniff certain features. Reevaluate this on major + // releases. + let ac = AutoCfg::new().unwrap(); + ac.emit_has_path("std::sync::LazyLock"); + + autocfg::rerun_path("build.rs"); +} diff --git a/src/lib.rs b/src/lib.rs index 1a5c46a..4c428e9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -193,6 +193,14 @@ impl MutexId { unreachable!("Tried to drop lock for mutex {:?} but it wasn't held", self) }); } + + /// Execute the given closure while the guard is held. + pub fn with_held(&self, f: impl FnOnce() -> T) -> T { + // Note: we MUST construct the RAII guard, we cannot simply mark held + mark released, as + // f() may panic and corrupt our state. + let _guard = self.get_borrowed(); + f() + } } impl Default for MutexId { diff --git a/src/parkinglot.rs b/src/parkinglot.rs index 6f12a0a..79616d2 100644 --- a/src/parkinglot.rs +++ b/src/parkinglot.rs @@ -138,16 +138,14 @@ pub mod tracing { /// This method will panic if `f` panics, poisoning this `Once`. In addition, this function /// panics when the lock acquisition order is determined to be inconsistent. pub fn call_once(&self, f: impl FnOnce()) { - let _borrow = self.id.get_borrowed(); - self.inner.call_once(f); + self.id.with_held(|| self.inner.call_once(f)); } /// Performs the given initialization routine once and only once. /// /// This method is identical to [`Once::call_once`] except it ignores poisoning. pub fn call_once_force(&self, f: impl FnOnce(OnceState)) { - let _borrow = self.id.get_borrowed(); - self.inner.call_once_force(f); + self.id.with_held(|| self.inner.call_once_force(f)); } } } diff --git a/src/stdsync.rs b/src/stdsync.rs index 7d16b68..76a3f69 100644 --- a/src/stdsync.rs +++ b/src/stdsync.rs @@ -30,6 +30,12 @@ pub use tracing::{ Condvar, Mutex, MutexGuard, Once, OnceLock, RwLock, RwLockReadGuard, RwLockWriteGuard, }; +#[cfg(all(has_std__sync__LazyLock, debug_assertions))] +pub use tracing::LazyLock; + +#[cfg(all(has_std__sync__LazyLock, not(debug_assertions)))] +pub use std::sync::LazyLock; + /// Dependency tracing versions of [`std::sync`]. pub mod tracing { use std::fmt; @@ -47,6 +53,12 @@ pub mod tracing { use crate::BorrowedMutex; use crate::LazyMutexId; + #[cfg(has_std__sync__LazyLock)] + pub use lazy_lock::LazyLock; + + #[cfg(has_std__sync__LazyLock)] + mod lazy_lock; + /// Wrapper for [`std::sync::Mutex`]. /// /// Refer to the [crate-level][`crate`] documentation for the differences between this struct and @@ -460,8 +472,7 @@ pub mod tracing { where F: FnOnce(), { - let _guard = self.mutex_id.get_borrowed(); - self.inner.call_once(f); + self.mutex_id.with_held(|| self.inner.call_once(f)) } /// Performs the same operation as [`call_once`][Once::call_once] except it ignores @@ -475,8 +486,7 @@ pub mod tracing { where F: FnOnce(&OnceState), { - let _guard = self.mutex_id.get_borrowed(); - self.inner.call_once_force(f); + self.mutex_id.with_held(|| self.inner.call_once_force(f)) } /// Returns true if some `call_once` has completed successfully. @@ -550,9 +560,7 @@ pub mod tracing { /// As this method may block until initialization is complete, it participates in cycle /// detection. pub fn set(&self, value: T) -> Result<(), T> { - let _guard = self.id.get_borrowed(); - - self.inner.set(value) + self.id.with_held(|| self.inner.set(value)) } /// Gets the contents of the cell, initializing it with `f` if the cell was empty. @@ -562,8 +570,7 @@ pub mod tracing { where F: FnOnce() -> T, { - let _guard = self.id.get_borrowed(); - self.inner.get_or_init(f) + self.id.with_held(|| self.inner.get_or_init(f)) } /// Takes the value out of this `OnceLock`, moving it back to an uninitialized state. diff --git a/src/stdsync/tracing/lazy_lock.rs b/src/stdsync/tracing/lazy_lock.rs new file mode 100644 index 0000000..16953d1 --- /dev/null +++ b/src/stdsync/tracing/lazy_lock.rs @@ -0,0 +1,113 @@ +//! Wrapper implementation for LazyLock +//! +//! This lives in a separate module as LazyLock would otherwise raise our MSRV to 1.80. Reevaluate +//! this in the future. +use std::fmt; +use std::fmt::Debug; +use std::ops::Deref; + +use crate::LazyMutexId; + +/// Wrapper for [`std::sync::LazyLock`] +/// +/// This wrapper participates in cycle detection like all other primitives in this crate. It should +/// only be possible to encounter cycles when acquiring mutexes in the initialisation function. +/// +/// # Examples +/// +/// ``` +/// use tracing_mutex::stdsync::tracing::LazyLock; +/// +/// static LOCK: LazyLock = LazyLock::new(|| { +/// println!("Hello, world!"); +/// 42 +/// }); +/// +/// // This should print "Hello, world!" +/// println!("{}", *LOCK); +/// // This should not. +/// println!("{}", *LOCK); +/// ``` +pub struct LazyLock T> { + inner: std::sync::LazyLock, + id: LazyMutexId, +} + +impl T> LazyLock { + /// Creates a new lazy value with the given initializing function. + pub const fn new(f: F) -> LazyLock { + Self { + id: LazyMutexId::new(), + inner: std::sync::LazyLock::new(f), + } + } + + /// Force this lazy lock to be evaluated. + /// + /// This is equivalent to dereferencing, but is more explicit. + pub fn force(this: &LazyLock) -> &T { + &*this + } +} + +impl T> Deref for LazyLock { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.id.with_held(|| &*self.inner) + } +} + +impl Default for LazyLock { + /// Return a `LazyLock` that is initialized through [`Default`]. + fn default() -> Self { + Self::new(Default::default) + } +} + +impl Debug for LazyLock { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Cannot implement this ourselves because the get() used is nightly, so delegate. + self.inner.fmt(f) + } +} + +#[cfg(test)] +mod tests { + use crate::stdsync::Mutex; + + use super::*; + + #[test] + fn test_only_init_once() { + let mut init_counter = 0; + + let lock = LazyLock::new(|| { + init_counter += 1; + 42 + }); + + assert_eq!(*lock, 42); + LazyLock::force(&lock); + + // Ensure we can access the init counter + drop(lock); + + assert_eq!(init_counter, 1); + } + + #[test] + #[should_panic(expected = "Found cycle")] + fn test_panic_with_cycle() { + let mutex = Mutex::new(()); + + let lock = LazyLock::new(|| *mutex.lock().unwrap()); + + // Establish the relation from lock to mutex + LazyLock::force(&lock); + + // Now do it the other way around, which should crash + let _guard = mutex.lock().unwrap(); + LazyLock::force(&lock); + } +} From 7a943ebba6cb8754650561431412e9a63a960c5e Mon Sep 17 00:00:00 2001 From: Bert Peters Date: Sat, 18 Jan 2025 12:02:23 +0100 Subject: [PATCH 2/3] Fix clippy issues --- Cargo.lock | 5 +++-- src/stdsync/tracing/lazy_lock.rs | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 84c3481..c91aa51 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -25,9 +25,9 @@ checksum = "15c4c2c83f81532e5845a733998b6971faca23490340a418e9b72a3ec9de12ea" [[package]] name = "autocfg" -version = "1.1.0" +version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26" [[package]] name = "bitflags" @@ -619,6 +619,7 @@ dependencies = [ name = "tracing-mutex" version = "0.3.0" dependencies = [ + "autocfg", "criterion", "lock_api", "parking_lot", diff --git a/src/stdsync/tracing/lazy_lock.rs b/src/stdsync/tracing/lazy_lock.rs index 16953d1..3b10c89 100644 --- a/src/stdsync/tracing/lazy_lock.rs +++ b/src/stdsync/tracing/lazy_lock.rs @@ -29,6 +29,8 @@ use crate::LazyMutexId; /// println!("{}", *LOCK); /// ``` pub struct LazyLock T> { + // MSRV violation is fine, this is gated behind a cfg! check + #[allow(clippy::incompatible_msrv)] inner: std::sync::LazyLock, id: LazyMutexId, } @@ -38,6 +40,8 @@ impl T> LazyLock { pub const fn new(f: F) -> LazyLock { Self { id: LazyMutexId::new(), + // MSRV violation is fine, this is gated behind a cfg! check + #[allow(clippy::incompatible_msrv)] inner: std::sync::LazyLock::new(f), } } @@ -46,7 +50,7 @@ impl T> LazyLock { /// /// This is equivalent to dereferencing, but is more explicit. pub fn force(this: &LazyLock) -> &T { - &*this + this } } From 4b17c45a4a555fec5d0ae5eee20ab1ea5b999d0b Mon Sep 17 00:00:00 2001 From: Bert Peters Date: Sat, 18 Jan 2025 12:29:38 +0100 Subject: [PATCH 3/3] Update docs --- CHANGELOG.md | 5 +++++ README.md | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8cc932..691bb38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- On Rust 1.80 or newer, a wrapper for `std::sync::LazyLock` is now available. The MSRV has not been + changed; older versions simply don't get this wrapper. + ### Changed - Reworked CI to better test continued support for the minimum supported Rust version diff --git a/README.md b/README.md index 06cd825..6be98cf 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Add this dependency to your `Cargo.lock` file like any other: ```toml [dependencies] -tracing-mutex = "0.2" +tracing-mutex = "0.3" ``` Then use the locks provided by this library instead of the ones you would use otherwise.