diff --git a/p2p/src/peers.rs b/p2p/src/peers.rs index fa871c3386..15ad9aa6db 100644 --- a/p2p/src/peers.rs +++ b/p2p/src/peers.rs @@ -97,15 +97,19 @@ impl Peers { self.save_peer(&peer_data) } - pub fn is_known(&self, addr: PeerAddr) -> bool { + /// Check if this peer address is already known (are we already connected to it)? + /// We try to get the read lock but if we experience contention + /// and this attempt fails then return an error allowing the caller + /// to decide how best to handle this. + pub fn is_known(&self, addr: PeerAddr) -> Result { let peers = match self.peers.try_read_for(LOCK_TIMEOUT) { Some(peers) => peers, None => { error!("is_known: failed to get peers lock"); - return false; + return Err(Error::Internal); } }; - peers.contains_key(&addr) + Ok(peers.contains_key(&addr)) } /// Get vec of peers we are currently connected to. diff --git a/p2p/src/serv.rs b/p2p/src/serv.rs index 28cc5fe93a..f804ce135c 100644 --- a/p2p/src/serv.rs +++ b/p2p/src/serv.rs @@ -224,9 +224,21 @@ impl Server { debug!("Peer {} banned, refusing connection.", peer_addr); return true; } - if self.peers.is_known(peer_addr) { - debug!("Peer {} already known, refusing connection.", peer_addr); - return true; + // The call to is_known() can fail due to contention on the peers map. + // If it fails we want to default to refusing the connection. + match self.peers.is_known(peer_addr) { + Ok(true) => { + debug!("Peer {} already known, refusing connection.", peer_addr); + return true; + } + Err(_) => { + error!( + "Peer {} is_known check failed, refusing connection.", + peer_addr + ); + return true; + } + _ => (), } } false diff --git a/servers/src/grin/seed.rs b/servers/src/grin/seed.rs index fd0d29db59..5f57044848 100644 --- a/servers/src/grin/seed.rs +++ b/servers/src/grin/seed.rs @@ -239,14 +239,14 @@ fn monitor_peers( max_peer_attempts as usize, ); - for p in new_peers.iter().filter(|p| !peers.is_known(p.addr)) { - trace!( - "monitor_peers: on {}:{}, queue to soon try {}", - config.host, - config.port, - p.addr, - ); - tx.send(p.addr).unwrap(); + // Only queue up connection attempts for candidate peers where we + // are confident we do not yet know about this peer. + // The call to is_known() may fail due to contention on the peers map. + // Do not attempt any connection where is_known() fails for any reason. + for p in new_peers { + if let Ok(false) = peers.is_known(p.addr) { + tx.send(p.addr).unwrap(); + } } }