diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f9e090d..f8ae2726 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# 0.6.0 (January 24, 2023) + +This version renames the [`silence_atomic_ordering_warning` configuration option](https://docs.rs/shuttle/0.5.0/shuttle/struct.Config.html#structfield.silence_atomic_ordering_warning) to `silence_warnings`, as well as the corresponding environment variables, to enable future warnings to be controlled by the same mechanism. + +* Implement `lazy_static` support (#93) + # 0.5.0 (November 22, 2022) This version updates the embedded `rand` library to v0.8. diff --git a/Cargo.toml b/Cargo.toml index 49e3bcdb..1b7300e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "shuttle" -version = "0.5.0" +version = "0.6.0" edition = "2021" license = "Apache-2.0" description = "A library for testing concurrent Rust code" diff --git a/src/lazy_static.rs b/src/lazy_static.rs new file mode 100644 index 00000000..bb43eb23 --- /dev/null +++ b/src/lazy_static.rs @@ -0,0 +1,146 @@ +//! Shuttle's implementation of the [`lazy_static`] crate, v1.4.0. +//! +//! Using this structure, it is possible to have `static`s that require code to be executed at +//! runtime in order to be initialized. Lazy statics should be created with the +//! [`lazy_static!`](crate::lazy_static!) macro. +//! +//! # Warning about drop behavior +//! +//! Shuttle's implementation of `lazy_static` will drop the static value at the end of an execution, +//! and so run the value's [`Drop`] implementation. The actual `lazy_static` crate does not drop the +//! static values, so this difference may cause false positives. +//! +//! To disable the warning printed about this issue, set the `SHUTTLE_SILENCE_WARNINGS` environment +//! variable to any value, or set the [`silence_warnings`](crate::Config::silence_warnings) field of +//! [`Config`](crate::Config) to true. +//! +//! [`lazy_static`]: https://crates.io/crates/lazy_static + +use crate::runtime::execution::ExecutionState; +use crate::runtime::storage::StorageKey; +use crate::sync::Once; +use std::marker::PhantomData; + +/// Shuttle's implementation of `lazy_static::Lazy` (aka the unstable `std::lazy::Lazy`). +// Sadly, the fields of this thing need to be public because function pointers in const fns are +// unstable, so an explicit instantiation is the only way to construct this struct. User code should +// not rely on these fields. +pub struct Lazy { + #[doc(hidden)] + pub cell: Once, + #[doc(hidden)] + pub init: fn() -> T, + #[doc(hidden)] + pub _p: PhantomData, +} + +impl std::fmt::Debug for Lazy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Lazy").finish_non_exhaustive() + } +} + +impl Lazy { + /// Get a reference to the lazy value, initializing it first if necessary. + pub fn get(&'static self) -> &T { + // Safety: see the usage below + unsafe fn extend_lt<'a, T>(t: &'a T) -> &'static T { + std::mem::transmute(t) + } + + // We implement lazy statics by using a `Once` to mediate initialization of the static, just + // as the real implementation does. If two threads race on initializing the static, they + // will race on the `Once::call_once`, and only one of them will initialize the static. Once + // it's initialized, all future accesses can bypass the `Once` entirely and just access the + // storage cell. + + let initialize = ExecutionState::with(|state| state.get_storage::<_, DropGuard>(self).is_none()); + + if initialize { + // There's not yet a value for this static, so try to initialize it (possibly racing) + self.cell.call_once(|| { + let value = (self.init)(); + ExecutionState::with(|state| state.init_storage(self, DropGuard(value))); + }); + } + + // At this point we're guaranteed that a value exists for this static, so read it + ExecutionState::with(|state| { + let value: &DropGuard = state.get_storage(self).expect("should be initialized"); + // Safety: this *isn't* safe. We are promoting to a `'static` lifetime here, but this + // object does not actually live that long. It would be possible for this reference to + // escape the client code and be used after it becomes invalid when the execution ends. + // But there's not really any way around this -- the semantics of static values and + // Shuttle executions are incompatible. + // + // In reality, this should be safe because any code that uses a Shuttle `lazy_static` + // probably uses a regular `lazy_static` in its non-test version, and if that version + // compiles then the Shuttle version is also safe. We choose this unsafe path because + // it's intended to be used only in testing code, which we want to remain as compatible + // with real-world code as possible and so needs to preserve the semantics of statics. + // + // See also https://github.com/tokio-rs/loom/pull/125 + unsafe { extend_lt(&value.0) } + }) + } +} + +impl From<&Lazy> for StorageKey { + fn from(lazy: &Lazy) -> Self { + StorageKey(lazy as *const _ as usize, 0x3) + } +} + +/// Support trait for enabling a few common operation on lazy static values. +/// +/// This is implemented by each defined lazy static, and used by the free functions in this crate. +pub trait LazyStatic { + #[doc(hidden)] + fn initialize(lazy: &Self); +} + +/// Takes a shared reference to a lazy static and initializes it if it has not been already. +/// +/// This can be used to control the initialization point of a lazy static. +pub fn initialize(lazy: &T) { + LazyStatic::initialize(lazy); +} + +static PRINTED_DROP_WARNING: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + +fn maybe_warn_about_drop() { + use owo_colors::OwoColorize; + use std::sync::atomic::Ordering; + + if PRINTED_DROP_WARNING + .compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) + .is_ok() + { + if std::env::var("SHUTTLE_SILENCE_WARNINGS").is_ok() { + return; + } + + if ExecutionState::with(|state| state.config.silence_warnings) { + return; + } + + eprintln!( + "{}: Shuttle runs the `Drop` method of `lazy_static` values at the end of an execution, \ + unlike the actual `lazy_static` implementation. This difference may cause false positives. \ + See https://docs.rs/shuttle/*/shuttle/lazy_static/index.html#warning-about-drop-behavior \ + for details or to disable this warning.", + "WARNING".yellow(), + ); + } +} + +/// Small wrapper to trigger the warning about drop behavior when a `lazy_static` value drops for +/// the first time. +#[derive(Debug)] +struct DropGuard(T); + +impl Drop for DropGuard { + fn drop(&mut self) { + maybe_warn_about_drop(); + } +} diff --git a/src/lib.rs b/src/lib.rs index 797513d0..be3daaec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -182,6 +182,7 @@ pub mod future; pub mod hint; +pub mod lazy_static; pub mod rand; pub mod sync; pub mod thread; @@ -213,9 +214,12 @@ pub struct Config { /// not abort a currently running test iteration; the limit is only checked between iterations. pub max_time: Option, - /// Whether to enable warnings about [Shuttle's unsound implementation of - /// `atomic`](crate::sync::atomic#warning-about-relaxed-behaviors). - pub silence_atomic_ordering_warning: bool, + /// Whether to silence warnings about Shuttle behaviors that may miss bugs or introduce false + /// positives: + /// 1. [Unsound implementation of `atomic`](crate::sync::atomic#warning-about-relaxed-behaviors) + /// may miss bugs + /// 2. [`lazy_static` values are dropped](mod@crate::lazy_static) at the end of an execution + pub silence_warnings: bool, } impl Config { @@ -226,7 +230,7 @@ impl Config { failure_persistence: FailurePersistence::Print, max_steps: MaxSteps::FailAfter(1_000_000), max_time: None, - silence_atomic_ordering_warning: false, + silence_warnings: false, } } } @@ -414,3 +418,69 @@ macro_rules! __thread_local_inner { }; } } + +/// Declare a new [lazy static value](crate::lazy_static::Lazy), like the `lazy_static` crate. +// These macros are copied from the lazy_static crate. +#[macro_export] +macro_rules! lazy_static { + ($(#[$attr:meta])* static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => { + // use `()` to explicitly forward the information about private items + $crate::__lazy_static_internal!($(#[$attr])* () static ref $N : $T = $e; $($t)*); + }; + ($(#[$attr:meta])* pub static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => { + $crate::__lazy_static_internal!($(#[$attr])* (pub) static ref $N : $T = $e; $($t)*); + }; + ($(#[$attr:meta])* pub ($($vis:tt)+) static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => { + $crate::__lazy_static_internal!($(#[$attr])* (pub ($($vis)+)) static ref $N : $T = $e; $($t)*); + }; + () => () +} + +#[macro_export] +#[doc(hidden)] +macro_rules! __lazy_static_internal { + // optional visibility restrictions are wrapped in `()` to allow for + // explicitly passing otherwise implicit information about private items + ($(#[$attr:meta])* ($($vis:tt)*) static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => { + $crate::__lazy_static_internal!(@MAKE TY, $(#[$attr])*, ($($vis)*), $N); + $crate::__lazy_static_internal!(@TAIL, $N : $T = $e); + $crate::lazy_static!($($t)*); + }; + (@TAIL, $N:ident : $T:ty = $e:expr) => { + impl ::std::ops::Deref for $N { + type Target = $T; + fn deref(&self) -> &$T { + #[inline(always)] + fn __static_ref_initialize() -> $T { $e } + + #[inline(always)] + fn __stability() -> &'static $T { + static LAZY: $crate::lazy_static::Lazy<$T> = + $crate::lazy_static::Lazy { + cell: $crate::sync::Once::new(), + init: __static_ref_initialize, + _p: std::marker::PhantomData, + }; + LAZY.get() + } + __stability() + } + } + impl $crate::lazy_static::LazyStatic for $N { + fn initialize(lazy: &Self) { + let _ = &**lazy; + } + } + }; + // `vis` is wrapped in `()` to prevent parsing ambiguity + (@MAKE TY, $(#[$attr:meta])*, ($($vis:tt)*), $N:ident) => { + #[allow(missing_copy_implementations)] + #[allow(non_camel_case_types)] + #[allow(dead_code)] + $(#[$attr])* + $($vis)* struct $N {__private_field: ()} + #[doc(hidden)] + $($vis)* static $N: $N = $N {__private_field: ()}; + }; + () => () +} diff --git a/src/sync/atomic/mod.rs b/src/sync/atomic/mod.rs index c969282d..28ff7e80 100644 --- a/src/sync/atomic/mod.rs +++ b/src/sync/atomic/mod.rs @@ -44,9 +44,8 @@ //! correctness, the [Loom] crate provides support for reasoning about Acquire and Release orderings //! and partial support for Relaxed orderings. //! -//! To disable the warning printed about this issue, set the `SHUTTLE_SILENCE_ORDERING_WARNING` -//! environment variable to any value, or set the -//! [`silence_atomic_ordering_warning`](crate::Config::silence_atomic_ordering_warning) field of +//! To disable the warning printed about this issue, set the `SHUTTLE_SILENCE_WARNINGS` environment +//! variable to any value, or set the [`silence_warnings`](crate::Config::silence_warnings) field of //! [`Config`](crate::Config) to true. //! //! [Loom]: https://crates.io/crates/loom @@ -81,7 +80,7 @@ fn maybe_warn_about_ordering(order: Ordering) { return; } - if ExecutionState::with(|state| state.config.silence_atomic_ordering_warning) { + if ExecutionState::with(|state| state.config.silence_warnings) { return; } diff --git a/tests/basic/lazy_static.rs b/tests/basic/lazy_static.rs new file mode 100644 index 00000000..80fb9076 --- /dev/null +++ b/tests/basic/lazy_static.rs @@ -0,0 +1,235 @@ +use shuttle::scheduler::DfsScheduler; +use shuttle::thread::ThreadId; +use shuttle::{check_dfs, check_random, lazy_static, thread, Runner}; +use std::collections::{HashMap, HashSet}; +use std::sync::Arc; +use test_log::test; + +#[test] +fn basic() { + shuttle::lazy_static! { + static ref HASH_MAP: HashMap = { + let mut m = HashMap::new(); + m.insert(1, 1); + m + }; + } + + check_dfs(|| assert_eq!(HASH_MAP.get(&1), Some(&1)), None); +} + +#[test] +fn racing_init() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + static COUNTER: AtomicUsize = AtomicUsize::new(0); + + shuttle::lazy_static! { + static ref HASH_MAP: HashMap = { + let mut m = HashMap::new(); + m.insert(1, 1); + COUNTER.fetch_add(1, Ordering::SeqCst); + m + }; + } + + check_dfs( + || { + let thds = (0..2) + .map(|_| { + thread::spawn(move || { + assert_eq!(HASH_MAP.get(&1), Some(&1)); + }) + }) + .collect::>(); + + for thd in thds { + thd.join().unwrap(); + } + + assert_eq!(COUNTER.swap(0, Ordering::SeqCst), 1); + }, + None, + ); +} + +// Same as `racing_init` but each thread first tries to manually initialize the static +#[test] +fn racing_init_explicit() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + static COUNTER: AtomicUsize = AtomicUsize::new(0); + + shuttle::lazy_static! { + static ref HASH_MAP: HashMap = { + let mut m = HashMap::new(); + m.insert(1, 1); + COUNTER.fetch_add(1, Ordering::SeqCst); + m + }; + } + + check_dfs( + || { + let thds = (0..2) + .map(|_| { + thread::spawn(move || { + lazy_static::initialize(&HASH_MAP); + assert_eq!(HASH_MAP.get(&1), Some(&1)); + }) + }) + .collect::>(); + + for thd in thds { + thd.join().unwrap(); + } + + assert_eq!(COUNTER.swap(0, Ordering::SeqCst), 1); + }, + None, + ); +} + +#[test] +fn init_with_yield() { + shuttle::lazy_static! { + static ref THING: Arc = { + // check that it's valid to yield in the initializer + thread::yield_now(); + Default::default() + }; + } + + check_dfs( + || { + let thd = thread::spawn(|| { + assert_eq!(**THING, 0); + }); + + assert_eq!(**THING, 0); + + thd.join().unwrap(); + }, + None, + ); +} + +// Run N threads racing to acquire a Mutex and record the order they resolved the race in. Check +// that we saw all N! possible orderings to make sure `lazy_static` allows any thread to win the +// initialization race. +#[track_caller] +fn run_mutex_test(num_threads: usize, tester: impl FnOnce(Box)) { + use std::sync::Mutex; + + shuttle::lazy_static! { + static ref THREADS: Mutex> = Mutex::new(Vec::new()); + } + + let initializers = Arc::new(Mutex::new(HashSet::new())); + let initializers_clone = Arc::clone(&initializers); + + tester(Box::new(move || { + let thds = (0..num_threads) + .map(|_| { + thread::spawn(|| { + THREADS.lock().unwrap().push(thread::current().id()); + }) + }) + .collect::>(); + + for thd in thds { + thd.join().unwrap(); + } + + assert_eq!(THREADS.lock().unwrap().len(), num_threads); + initializers.lock().unwrap().insert(THREADS.lock().unwrap().clone()); + })); + + let initializers = Arc::try_unwrap(initializers_clone).unwrap().into_inner().unwrap(); + assert_eq!(initializers.len(), (1..num_threads + 1).product::()); +} + +// Check all the interleavings of two racing threads trying to acquire a static Mutex. +#[test] +fn mutex_dfs() { + run_mutex_test(2, |f| check_dfs(f, None)); +} + +// Like `mutex_dfs` but with more threads, making it too expensive to do DFS. +#[test] +fn mutex_random() { + // Not guaranteed to pass, but should be pretty likely for 4 threads to see all 24 interleavings + // in 10,000 iterations of a random scheduler + run_mutex_test(4, |f| check_random(f, 10_000)); +} + +#[test] +fn chained() { + shuttle::lazy_static! { + static ref S1: Arc = Default::default(); + static ref S2: Arc = Arc::new(**S1); + } + + check_dfs( + move || { + let thd = thread::spawn(|| { + assert_eq!(**S2, 0); + }); + + assert_eq!(**S1, 0); + + thd.join().unwrap(); + }, + None, + ); +} + +// Ensure that concurrent Shuttle tests see an isolated version of a lazy_static. This test is best +// effort, as it spawns OS threads and hopes they race. +#[test] +fn shared_static() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + static COUNTER: AtomicUsize = AtomicUsize::new(0); + + shuttle::lazy_static! { + static ref S: usize = { + COUNTER.fetch_add(1, Ordering::SeqCst); + 0 + }; + } + + let mut total_executions = 0; + + // Try a bunch of times to provoke the race + for _ in 0..50 { + #[allow(clippy::needless_collect)] // https://github.com/rust-lang/rust-clippy/issues/7207 + let threads = (0..3) + .map(|_| { + std::thread::spawn(move || { + let scheduler = DfsScheduler::new(None, false); + let runner = Runner::new(scheduler, Default::default()); + runner.run(move || { + let thds = (0..2) + .map(|_| { + thread::spawn(move || { + assert_eq!(*S, 0); + }) + }) + .collect::>(); + + for thd in thds { + thd.join().unwrap(); + } + }) + }) + }) + .collect::>(); + + total_executions += threads.into_iter().map(|handle| handle.join().unwrap()).sum::(); + } + + // The static should be initialized exactly once per test execution, otherwise the tests are + // incorrectly sharing state + assert_eq!(total_executions, COUNTER.load(Ordering::SeqCst)); +} diff --git a/tests/basic/mod.rs b/tests/basic/mod.rs index 46fab735..a2fd45b7 100644 --- a/tests/basic/mod.rs +++ b/tests/basic/mod.rs @@ -5,6 +5,7 @@ mod condvar; mod config; mod dfs; mod execution; +mod lazy_static; mod metrics; mod mpsc; mod mutex;