From 82bbb807add57f75c0a5d36ec140b923e551a4e7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 9 Mar 2022 06:23:39 +0000 Subject: [PATCH] Refactor debug sync methods into helper functions --- lightning/src/debug_sync.rs | 83 +++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/lightning/src/debug_sync.rs b/lightning/src/debug_sync.rs index 7ee5ee52..3eb342af 100644 --- a/lightning/src/debug_sync.rs +++ b/lightning/src/debug_sync.rs @@ -64,6 +64,50 @@ impl std::hash::Hash for MutexMetadata { fn hash(&self, hasher: &mut H) { hasher.write_u64(self.mutex_idx); } } +impl MutexMetadata { + fn new() -> MutexMetadata { + MutexMetadata { + locked_before: StdMutex::new(HashSet::new()), + mutex_idx: MUTEX_IDX.fetch_add(1, Ordering::Relaxed) as u64, + #[cfg(feature = "backtrace")] + mutex_construction_bt: Backtrace::new(), + } + } + + fn pre_lock(this: &Arc) { + MUTEXES_HELD.with(|held| { + // For each mutex which is currently locked, check that no mutex's locked-before + // set includes the mutex we're about to lock, which would imply a lockorder + // inversion. + for locked in held.borrow().iter() { + for locked_dep in locked.locked_before.lock().unwrap().iter() { + if *locked_dep == *this { + #[cfg(feature = "backtrace")] + panic!("Tried to violate existing lockorder.\nMutex that should be locked after the current lock was created at the following backtrace.\nNote that to get a backtrace for the lockorder violation, you should set RUST_BACKTRACE=1\n{:?}", locked.mutex_construction_bt); + #[cfg(not(feature = "backtrace"))] + panic!("Tried to violate existing lockorder. Build with the backtrace feature for more info."); + } + } + // Insert any already-held mutexes in our locked-before set. + this.locked_before.lock().unwrap().insert(Arc::clone(locked)); + } + held.borrow_mut().insert(Arc::clone(this)); + }); + } + + fn try_locked(this: &Arc) { + MUTEXES_HELD.with(|held| { + // Since a try-lock will simply fail if the lock is held already, we do not + // consider try-locks to ever generate lockorder inversions. However, if a try-lock + // succeeds, we do consider it to have created lockorder dependencies. + for locked in held.borrow().iter() { + this.locked_before.lock().unwrap().insert(Arc::clone(locked)); + } + held.borrow_mut().insert(Arc::clone(this)); + }); + } +} + pub struct Mutex { inner: StdMutex, deps: Arc, @@ -110,51 +154,18 @@ impl DerefMut for MutexGuard<'_, T> { impl Mutex { pub fn new(inner: T) -> Mutex { - Mutex { - inner: StdMutex::new(inner), - deps: Arc::new(MutexMetadata { - locked_before: StdMutex::new(HashSet::new()), - mutex_idx: MUTEX_IDX.fetch_add(1, Ordering::Relaxed) as u64, - #[cfg(feature = "backtrace")] - mutex_construction_bt: Backtrace::new(), - }), - } + Mutex { inner: StdMutex::new(inner), deps: Arc::new(MutexMetadata::new()) } } pub fn lock<'a>(&'a self) -> LockResult> { - MUTEXES_HELD.with(|held| { - // For each mutex which is currently locked, check that no mutex's locked-before - // set includes the mutex we're about to lock, which would imply a lockorder - // inversion. - for locked in held.borrow().iter() { - for locked_dep in locked.locked_before.lock().unwrap().iter() { - if *locked_dep == self.deps { - #[cfg(feature = "backtrace")] - panic!("Tried to violate existing lockorder.\nMutex that should be locked after the current lock was created at the following backtrace.\nNote that to get a backtrace for the lockorder violation, you should set RUST_BACKTRACE=1\n{:?}", locked.mutex_construction_bt); - #[cfg(not(feature = "backtrace"))] - panic!("Tried to violate existing lockorder. Build with the backtrace feature for more info."); - } - } - // Insert any already-held mutexes in our locked-before set. - self.deps.locked_before.lock().unwrap().insert(Arc::clone(locked)); - } - held.borrow_mut().insert(Arc::clone(&self.deps)); - }); + MutexMetadata::pre_lock(&self.deps); self.inner.lock().map(|lock| MutexGuard { mutex: self, lock }).map_err(|_| ()) } pub fn try_lock<'a>(&'a self) -> LockResult> { let res = self.inner.try_lock().map(|lock| MutexGuard { mutex: self, lock }).map_err(|_| ()); if res.is_ok() { - MUTEXES_HELD.with(|held| { - // Since a try-lock will simply fail if the lock is held already, we do not - // consider try-locks to ever generate lockorder inversions. However, if a try-lock - // succeeds, we do consider it to have created lockorder dependencies. - for locked in held.borrow().iter() { - self.deps.locked_before.lock().unwrap().insert(Arc::clone(locked)); - } - held.borrow_mut().insert(Arc::clone(&self.deps)); - }); + MutexMetadata::try_locked(&self.deps); } res } -- 2.30.2