From 5fe3a5631d2225c57e8c49aebd65753f27ae4cc8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 10 May 2017 17:42:53 -0700 Subject: [PATCH] server: eliminate possible initial connection churn by ignoring duplicate inbound conns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a bug that was introduced when the connection handling was re-worked to properly handle the case of concurrent connections being made. In certain cases after a successful initial connection, a peer’s stray goroutine would still attempt to establish a second outbound connection even though a connection had already been established. This was properly handled by the connecting peer, but not he receiving peer. This commit adds the additional logic to the receiving peer to ensure that we properly handle this case. --- server.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/server.go b/server.go index 6d6d7eb7..0753fe62 100644 --- a/server.go +++ b/server.go @@ -757,15 +757,23 @@ func (s *server) inboundPeerConnected(conn net.Conn) { s.peersMtx.Lock() defer s.peersMtx.Unlock() + // If we already have an inbound connection to this peer, then ignore + // this new connection. + nodePub := conn.(*brontide.Conn).RemotePub() + pubStr := string(nodePub.SerializeCompressed()) + if _, ok := s.inboundPeers[pubStr]; ok { + srvrLog.Debugf("Ignoring duplicate inbound connection") + conn.Close() + return + } + srvrLog.Infof("New inbound connection from %v", conn.RemoteAddr()) localPub := s.identityPriv.PubKey() - nodePub := conn.(*brontide.Conn).RemotePub() // Check to see if we should drop our connection, if not, then we'll // close out this connection with the remote peer. This // prevents us from having duplicate connections, or none. - pubStr := string(nodePub.SerializeCompressed()) if connectedPeer, ok := s.peersByPub[pubStr]; ok { // If the connection we've already established should be kept, // then we'll close out this connection s.t there's only a