From 24c845349678b9064d089c349dd2c4a3fc1673b5 Mon Sep 17 00:00:00 2001 From: Bert Peters Date: Wed, 21 Apr 2021 20:21:46 +0200 Subject: [PATCH] Document API and design --- src/graph.rs | 28 +++++++++++++--------------- src/lib.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++++ src/stdsync.rs | 40 +++++++++++++++++++++++++++++++++------- 3 files changed, 93 insertions(+), 22 deletions(-) diff --git a/src/graph.rs b/src/graph.rs index 8a16af5..8c84868 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -7,17 +7,16 @@ type Order = usize; /// Directed Graph with dynamic topological sorting /// -/// Design and implementation based "A Dynamic Topological Sort Algorithm for -/// Directed Acyclic Graphs" by David J. Pearce and Paul H.J. Kelly which can -/// be found on [the author's website][paper]. +/// Design and implementation based "A Dynamic Topological Sort Algorithm for Directed Acyclic +/// Graphs" by David J. Pearce and Paul H.J. Kelly which can be found on [the author's +/// website][paper]. /// -/// Variable- and method names have been chosen to reflect most closely -/// resemble the names in the original paper. +/// Variable- and method names have been chosen to reflect most closely resemble the names in the +/// original paper. /// -/// This digraph tracks its own topological order and updates it when new edges -/// are added to the graph. After a cycle has been introduced, the order is no -/// longer kept up to date as it doesn't exist, but new edges are still -/// tracked. Nodes are added implicitly when they're used in edges. +/// This digraph tracks its own topological order and updates it when new edges are added to the +/// graph. After a cycle has been introduced, the order is no longer kept up to date as it doesn't +/// exist, but new edges are still tracked. Nodes are added implicitly when they're used in edges. /// /// [paper]: https://whileydave.com/publications/pk07_jea/ #[derive(Clone, Default, Debug)] @@ -36,10 +35,9 @@ pub struct DiGraph { impl DiGraph { /// Add a new node to the graph. /// - /// If the node already existed, this function does not add it and uses the - /// existing node data. The function returns mutable references to the - /// in-edges, out-edges, and finally the index of the node in the topological - /// order. + /// If the node already existed, this function does not add it and uses the existing node data. + /// The function returns mutable references to the in-edges, out-edges, and finally the index of + /// the node in the topological order. /// /// New nodes are appended to the end of the topological order when added. fn add_node(&mut self, n: MutexId) -> (&mut HashSet, &mut HashSet, Order) { @@ -183,8 +181,8 @@ impl DiGraph { l.push(v); } - // Original paper cleverly merges the two lists by using that both are - // sorted. We just sort again. This is slower but also much simpler. + // Original paper cleverly merges the two lists by using that both are sorted. We just sort + // again. This is slower but also much simpler. orders.sort_unstable(); for (node, order) in l.into_iter().zip(orders) { diff --git a/src/lib.rs b/src/lib.rs index b395c26..63964e3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,50 @@ +//! Mutexes can deadlock each other, but you can avoid this by always acquiring your locks in a +//! consistent order. This crate provides tracing to ensure that you do. +//! +//! This crate tracks a virtual "stack" of locks that the current thread holds, and whenever a new +//! lock is acquired, a dependency is created from the last lock to the new one. These dependencies +//! together form a graph. As long as that graph does not contain any cycles, your program is +//! guaranteed to never deadlock. +//! +//! # Panics +//! +//! The primary method by which this crate signals an invalid lock acquisition order is by +//! panicking. When a cycle is created in the dependency graph when acquiring a lock, the thread +//! will instead panic. This panic will not poison the underlying mutex. Each following acquired +//! that introduces a **new** dependency will also panic, until enough mutexes are deallocated to +//! break the cycle in the graph. +//! +//! # Structure +//! +//! Each module in this crate exposes wrappers for a specific base-mutex with dependency trakcing +//! added. For now, that is limited to [`stdsync`] which provides wrappers for the base locks in the +//! standard library. More back-ends may be added as features in the future. +//! +//! # Performance considerations +//! +//! Tracing a mutex adds overhead to certain mutex operations in order to do the required +//! bookkeeping. The following actions have the following overhead. +//! +//! - **Acquiring a lock** locks the global dependency graph temporarily to check if the new lock +//! would introduce a cyclic dependency. This crate uses the algorithm proposed in ["A Dynamic +//! Topological Sort Algorithm for Directed Acyclic Graphs" by David J. Pearce and Paul H.J. +//! Kelly][paper] to detect cycles as efficently as possible. In addition, a thread local lock set +//! is updated with the new lock. +//! +//! - **Releasing a lock** updates a thread local lock set to remove the released lock. +//! +//! - **Allocating a lock** performs an atomic update to a shared counter. +//! +//! - **Deallocating a mutex** temporarily locks the global dependency graph to remove the lock from +//! it. If the graph contained a cycle, a complete scan of the (now pruned) graph is done to +//! determine if this is still the case. +//! +//! These operations have been reasonably optimized, but the performance penalty may yet be too much +//! for production use. In those cases, it may be beneficial to instead use debug-only versions +//! (such as [`stdsync::DebugMutex`]) which evaluate to a tracing mutex when debug assertions are +//! enabled, and to the underlying mutex when they're not. +//! +//! [paper]: https://whileydave.com/publications/pk07_jea/ use std::cell::RefCell; use std::fmt; use std::ops::DerefMut; diff --git a/src/stdsync.rs b/src/stdsync.rs index 39ec940..995ec3c 100644 --- a/src/stdsync.rs +++ b/src/stdsync.rs @@ -1,4 +1,4 @@ -//! Tracing mutex wrappers for types found in `std::sync`. +//! Tracing mutex wrappers for locks found in `std::sync`. //! //! This module provides wrappers for `std::sync` primitives with exactly the same API and //! functionality as their counterparts, with the exception that their acquisition order is @@ -32,14 +32,40 @@ use crate::get_depedency_graph; use crate::BorrowedMutex; use crate::MutexId; -/// Wrapper for `std::sync::Mutex` +/// Debug-only tracing `Mutex`. +/// +/// Type alias that resolves to [`TracingMutex`] when debug assertions are enabled and to +/// [`std::sync::Mutex`] when they're not. Use this if you want to have the benefits of cycle +/// detection in development but do not want to pay the performance penalty in release. +#[cfg(debug_assertions)] +pub type DebugMutex = TracingMutex; +#[cfg(not(debug_assertions))] +pub type DebugMutex = Mutex; + +/// Debug-only tracing `RwLock`. +/// +/// Type alias that resolves to [`RwLock`] when debug assertions are enabled and to +/// [`std::sync::RwLock`] when they're not. Use this if you want to have the benefits of cycle +/// detection in development but do not want to pay the performance penalty in release. +#[cfg(debug_assertions)] +pub type DebugRwLock = TracingRwLock; +#[cfg(not(debug_assertions))] +pub type DebugRwLock = RwLock; + +/// Wrapper for [`std::sync::Mutex`]. +/// +/// Refer to the [crate-level][`crate`] documentaiton for the differences between this struct and +/// the one it wraps. #[derive(Debug)] pub struct TracingMutex { inner: Mutex, id: MutexId, } -/// Wrapper for `std::sync::MutexGuard` +/// Wrapper for [`std::sync::MutexGuard`]. +/// +/// Refer to the [crate-level][`crate`] documentaiton for the differences between this struct and +/// the one it wraps. #[derive(Debug)] pub struct TracingMutexGuard<'a, T> { inner: MutexGuard<'a, T>, @@ -164,14 +190,14 @@ impl<'a, T: fmt::Display> fmt::Display for TracingMutexGuard<'a, T> { } } -/// Wrapper for `std::sync::RwLock` +/// Wrapper for [`std::sync::RwLock`]. #[derive(Debug)] pub struct TracingRwLock { inner: RwLock, id: MutexId, } -/// Hybrid wrapper for both `std::sync::RwLockReadGuard` and `std::sync::RwLockWriteGuard`. +/// Hybrid wrapper for both [`std::sync::RwLockReadGuard`] and [`std::sync::RwLockWriteGuard`]. /// /// Please refer to [`TracingReadGuard`] and [`TracingWriteGuard`] for usable types. #[derive(Debug)] @@ -180,9 +206,9 @@ pub struct TracingRwLockGuard { mutex: BorrowedMutex, } -/// Wrapper around `std::sync::RwLockReadGuard`. +/// Wrapper around [`std::sync::RwLockReadGuard`]. pub type TracingReadGuard<'a, T> = TracingRwLockGuard>; -/// Wrapper around `std::sync::RwLockWriteGuard`. +/// Wrapper around [`std::sync::RwLockWriteGuard`]. pub type TracingWriteGuard<'a, T> = TracingRwLockGuard>; impl TracingRwLock {