From a8e8af635131faec6ce125064326e5abcc73f6bc Mon Sep 17 00:00:00 2001 From: Bert Peters Date: Sat, 9 Sep 2023 11:10:08 +0200 Subject: [PATCH] Make dependency tracking a compile time setting --- Cargo.toml | 2 ++ src/lib.rs | 52 +++++++++------------------------------ src/reporting.rs | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 41 deletions(-) create mode 100644 src/reporting.rs diff --git a/Cargo.toml b/Cargo.toml index c86b5c5..74db0f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,8 @@ name = "mutex" harness = false [features] +default = ["backtraces"] +backtraces = [] # Feature names do not match crate names pending namespaced features. lockapi = ["lock_api"] parkinglot = ["parking_lot", "lockapi"] diff --git a/src/lib.rs b/src/lib.rs index 59cbd60..9c958eb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,7 +46,6 @@ //! //! [paper]: https://whileydave.com/publications/pk07_jea/ #![cfg_attr(docsrs, feature(doc_cfg))] -use std::backtrace::Backtrace; use std::cell::RefCell; use std::fmt; use std::marker::PhantomData; @@ -54,7 +53,6 @@ use std::ops::Deref; use std::ops::DerefMut; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; -use std::sync::Arc; use std::sync::Mutex; use std::sync::MutexGuard; use std::sync::OnceLock; @@ -66,6 +64,8 @@ pub use lock_api; #[cfg(feature = "parkinglot")] #[cfg_attr(docsrs, doc(cfg(feature = "parkinglot")))] pub use parking_lot; +use reporting::Dep; +use reporting::Reportable; use crate::graph::DiGraph; @@ -76,6 +76,7 @@ pub mod lockapi; #[cfg(feature = "parkinglot")] #[cfg_attr(docsrs, doc(cfg(feature = "parkinglot")))] pub mod parkinglot; +mod reporting; pub mod stdsync; thread_local! { @@ -143,24 +144,14 @@ impl MutexId { if let Some(&previous) = locks.borrow().last() { let mut graph = get_dependency_graph(); - graph - .add_edge(previous, self.value(), MutexDep::capture) - .err() + graph.add_edge(previous, self.value(), Dep::capture).err() } else { None } }); if let Some(cycle) = opt_cycle { - // Panic without holding the lock to avoid needlessly poisoning it - let mut message = String::from("Found cycle in mutex dependency graph:\n"); - - for entry in cycle { - use std::fmt::Write; - let _ = writeln!(message, "{entry}"); - } - - panic!("{message}"); + panic!("{}", Dep::panic_message(&cycle)) } HELD_LOCKS.with(|locks| locks.borrow_mut().push(self.value())); @@ -270,24 +261,9 @@ impl<'a> Drop for BorrowedMutex<'a> { } } -#[derive(Clone)] -struct MutexDep(Arc); - -impl MutexDep { - pub fn capture() -> Self { - Self(Arc::new(Backtrace::capture())) - } -} - -impl fmt::Display for MutexDep { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) - } -} - /// Get a reference to the current dependency graph -fn get_dependency_graph() -> impl DerefMut> { - static DEPENDENCY_GRAPH: OnceLock>> = OnceLock::new(); +fn get_dependency_graph() -> impl DerefMut> { + static DEPENDENCY_GRAPH: OnceLock>> = OnceLock::new(); DEPENDENCY_GRAPH .get_or_init(Default::default) @@ -318,17 +294,11 @@ mod tests { let c = LazyMutexId::new(); let mut graph = get_dependency_graph(); - assert!(graph - .add_edge(a.value(), b.value(), MutexDep::capture) - .is_ok()); - assert!(graph - .add_edge(b.value(), c.value(), MutexDep::capture) - .is_ok()); + assert!(graph.add_edge(a.value(), b.value(), Dep::capture).is_ok()); + assert!(graph.add_edge(b.value(), c.value(), Dep::capture).is_ok()); // Creating an edge c → a should fail as it introduces a cycle. - assert!(graph - .add_edge(c.value(), a.value(), MutexDep::capture) - .is_err()); + assert!(graph.add_edge(c.value(), a.value(), Dep::capture).is_err()); // Drop graph handle so we can drop vertices without deadlocking drop(graph); @@ -337,7 +307,7 @@ mod tests { // If b's destructor correctly ran correctly we can now add an edge from c to a. assert!(get_dependency_graph() - .add_edge(c.value(), a.value(), MutexDep::capture) + .add_edge(c.value(), a.value(), Dep::capture) .is_ok()); } diff --git a/src/reporting.rs b/src/reporting.rs new file mode 100644 index 0000000..4149690 --- /dev/null +++ b/src/reporting.rs @@ -0,0 +1,64 @@ +//! Cycle reporting primitives +//! +//! This module exposes [`Dep`], which resolves to either something that tracks dependencies or to +//! something that doesn't. It should only be assumed to implement the [`Reportable`] trait. +use std::backtrace::Backtrace; +use std::borrow::Cow; +use std::fmt::Write; +use std::sync::Arc; + +#[cfg(feature = "backtraces")] +pub type Dep = MutexDep>; +#[cfg(not(feature = "backtraces"))] +pub type Dep = MutexDep<()>; + +// Base message to be reported when cycle is detected +const BASE_MESSAGE: &str = "Found cycle in mutex dependency graph:"; + +pub trait Reportable: Clone { + /// Capture the current state + fn capture() -> Self; + + /// Format a trace of state for human readable consumption. + fn panic_message(trace: &[Self]) -> Cow<'static, str>; +} + +#[derive(Clone)] +pub struct MutexDep(T); + +/// Use a unit as tracing data: no tracing. +/// +/// This should have no runtime overhead for capturing traces and should therefore be cheap enough +/// for most purposes. +impl Reportable for MutexDep<()> { + fn capture() -> Self { + Self(()) + } + + fn panic_message(_trace: &[Self]) -> Cow<'static, str> { + Cow::Borrowed(BASE_MESSAGE) + } +} + +/// Use a full backtrace as tracing data +/// +/// Capture the entire backtrace which may be expensive. This implementation does not force capture +/// in the event that backtraces are disabled at runtime, so the exact overhead can still be +/// controlled a little. +/// +/// N.B. the [`Backtrace`] needs to be wrapped in an Arc as backtraces are not [`Clone`]. +impl Reportable for MutexDep> { + fn capture() -> Self { + Self(Arc::new(Backtrace::capture())) + } + + fn panic_message(trace: &[Self]) -> Cow<'static, str> { + let mut message = format!("{BASE_MESSAGE}\n"); + + for entry in trace { + let _ = writeln!(message, "{}", entry.0); + } + + message.into() + } +}