From 6418a86e5176229b9719155a2b9a640d4e0c1175 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Tue, 9 May 2023 11:44:48 +0200 Subject: [PATCH] Use a total lockorder for `NetworkGraph`'s `PartialEq` impl `NetworkGraph`'s `PartialEq` impl before this commit was deadlock-prone. Similarly to `ChannelMonitor`'s, `PartialEq` impl, we use position in memory for a total lockorder. This uses the assumption that the objects cannot move within memory while the inner locks are held. --- lightning/src/routing/gossip.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index d11eaa6b..f134bd05 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -40,7 +40,7 @@ use crate::io_extras::{copy, sink}; use crate::prelude::*; use core::{cmp, fmt}; use core::convert::TryFrom; -use crate::sync::{RwLock, RwLockReadGuard}; +use crate::sync::{RwLock, RwLockReadGuard, LockTestExt}; #[cfg(feature = "std")] use core::sync::atomic::{AtomicUsize, Ordering}; use crate::sync::Mutex; @@ -1327,9 +1327,14 @@ impl fmt::Display for NetworkGraph where L::Target: Logger { impl Eq for NetworkGraph where L::Target: Logger {} impl PartialEq for NetworkGraph where L::Target: Logger { fn eq(&self, other: &Self) -> bool { - self.genesis_hash == other.genesis_hash && - *self.channels.read().unwrap() == *other.channels.read().unwrap() && - *self.nodes.read().unwrap() == *other.nodes.read().unwrap() + // For a total lockorder, sort by position in memory and take the inner locks in that order. + // (Assumes that we can't move within memory while a lock is held). + let ord = ((self as *const _) as usize) < ((other as *const _) as usize); + let a = if ord { (&self.channels, &self.nodes) } else { (&other.channels, &other.nodes) }; + let b = if ord { (&other.channels, &other.nodes) } else { (&self.channels, &self.nodes) }; + let (channels_a, channels_b) = (a.0.unsafe_well_ordered_double_lock_self(), b.0.unsafe_well_ordered_double_lock_self()); + let (nodes_a, nodes_b) = (a.1.unsafe_well_ordered_double_lock_self(), b.1.unsafe_well_ordered_double_lock_self()); + self.genesis_hash.eq(&other.genesis_hash) && channels_a.eq(&channels_b) && nodes_a.eq(&nodes_b) } } -- 2.30.2