diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 43234fc71c5..3549740bfb5 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -26,6 +26,7 @@ use ln::peer_channel_encryptor::{PeerChannelEncryptor,NextNoiseStep}; use ln::wire; use ln::wire::Encode; use util::atomic_counter::AtomicCounter; +use util::fairrwlock::FairRwLock; use util::events::{MessageSendEvent, MessageSendEventsProvider}; use util::logger::Logger; use routing::network_graph::{NetworkGraph, NetGraphMsgHandler}; @@ -33,7 +34,7 @@ use routing::network_graph::{NetworkGraph, NetGraphMsgHandler}; use prelude::*; use io; use alloc::collections::LinkedList; -use sync::{Arc, Mutex, MutexGuard, RwLock}; +use sync::{Arc, Mutex, MutexGuard}; use core::sync::atomic::{AtomicUsize, Ordering}; use core::{cmp, hash, fmt, mem}; use core::ops::Deref; @@ -413,7 +414,7 @@ pub struct PeerManager, - peers: RwLock>, + peers: FairRwLock>, /// Only add to this set when noise completes. /// Locked *after* peers. When an item is removed, it must be removed with the `peers` write /// lock held. Entries may be added with only the `peers` read lock held (though the @@ -525,7 +526,7 @@ impl P PeerManager { message_handler, - peers: RwLock::new(PeerHolder { + peers: FairRwLock::new(PeerHolder { peers: HashMap::new(), }), node_id_to_descriptor: Mutex::new(HashMap::new()), diff --git a/lightning/src/util/fairrwlock.rs b/lightning/src/util/fairrwlock.rs new file mode 100644 index 00000000000..74658462920 --- /dev/null +++ b/lightning/src/util/fairrwlock.rs @@ -0,0 +1,52 @@ +#[cfg(feature = "std")] +mod rwlock { + use std::sync::{TryLockResult, LockResult, RwLock, RwLockReadGuard, RwLockWriteGuard}; + use std::sync::atomic::{AtomicUsize, Ordering}; + + /// Rust libstd's RwLock does not provide any fairness guarantees (and, in fact, when used on + /// Linux with pthreads under the hood, readers trivially and completely starve writers). + /// Because we often hold read locks while doing message processing in multiple threads which + /// can use significant CPU time, with write locks being time-sensitive but relatively small in + /// CPU time, we can end up with starvation completely blocking incoming connections or pings, + /// especially during initial graph sync. + /// + /// Thus, we need to block readers when a writer is pending, which we do with a trivial RwLock + /// wrapper here. Its not particularly optimized, but provides some reasonable fairness by + /// blocking readers (by taking the write lock) if there are writers pending when we go to take + /// a read lock. + pub struct FairRwLock { + lock: RwLock, + waiting_writers: AtomicUsize, + } + + impl FairRwLock { + pub fn new(t: T) -> Self { + Self { lock: RwLock::new(t), waiting_writers: AtomicUsize::new(0) } + } + + pub fn write(&self) -> LockResult> { + self.waiting_writers.fetch_add(1, Ordering::AcqRel); + let res = self.lock.write(); + self.waiting_writers.fetch_sub(1, Ordering::AcqRel); + res + } + + pub fn try_write(&self) -> TryLockResult> { + self.lock.try_write() + } + + pub fn read(&self) -> LockResult> { + if self.waiting_writers.load(Ordering::Acquire) != 0 { + let _write_queue_lock = self.lock.write(); + } + self.lock.read() + } + } +} +#[cfg(feature = "std")] +pub use self::rwlock::*; + +#[cfg(not(feature = "std"))] +pub type FairRwLock = crate::sync::RwLock; + + diff --git a/lightning/src/util/mod.rs b/lightning/src/util/mod.rs index 34e66190121..e47c7165fef 100644 --- a/lightning/src/util/mod.rs +++ b/lightning/src/util/mod.rs @@ -23,6 +23,7 @@ pub mod message_signing; pub(crate) mod atomic_counter; pub(crate) mod byte_utils; pub(crate) mod chacha20; +pub(crate) mod fairrwlock; #[cfg(feature = "fuzztarget")] pub mod zbase32; #[cfg(not(feature = "fuzztarget"))]