Make dependency tracking a compile time setting

Author: bertptrs

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"]

src/lib.rs

@@ -46,15 +46,13 @@
 //!
 //! [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;
 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<Backtrace>);
-
-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<Target = DiGraph<usize, MutexDep>> {
-    static DEPENDENCY_GRAPH: OnceLock<Mutex<DiGraph<usize, MutexDep>>> = OnceLock::new();
+fn get_dependency_graph() -> impl DerefMut<Target = DiGraph<usize, Dep>> {
+    static DEPENDENCY_GRAPH: OnceLock<Mutex<DiGraph<usize, Dep>>> = 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());
     }
 

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<Arc<Backtrace>>;
+#[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>(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<Arc<Backtrace>> {
+    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()
+    }
+}