From 214d975adfafbd9b4eca85363a0124a8e88cff58 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Thu, 27 Oct 2016 21:28:30 -0500
Subject: [PATCH] server: Notify connmgr if server peer assoc fails.

This corrects a few issues introduced with the connection manager where
the server was not notifying the connection manager when a connection
request is available again.

The cases resolved are:
- Unable to initialize a server peer instance in response to the connection
- Failure to associate the connection with the server peer instance
- Disconnection of a non-persistent outbound peer

It also changes the log message to a debug in the former case because
it's not something that should be shown to the user as an error given
it's not due to anything the user has misconfigured nor is it even
unexpected if an invalid address is provided.
---
 server.go | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/server.go b/server.go
index afc8c959..dc869cc0 100644
--- a/server.go
+++ b/server.go
@@ -1245,7 +1245,7 @@ func (s *server) handleDonePeerMsg(state *peerState, sp *serverPeer) {
 		if !sp.Inbound() && sp.VersionKnown() {
 			state.outboundGroups[addrmgr.GroupKey(sp.NA())]--
 		}
-		if sp.persistent && sp.connReq != nil {
+		if !sp.Inbound() && sp.connReq != nil {
 			s.connManager.Disconnect(sp.connReq.ID())
 		}
 		delete(list, sp.ID())
@@ -1568,25 +1568,30 @@ func (s *server) listenHandler(listener net.Listener) {
 		}
 		sp := newServerPeer(s, false)
 		sp.Peer = peer.NewInboundPeer(newPeerConfig(sp))
-		go s.peerDoneHandler(sp)
 		sp.AssociateConnection(conn)
+		go s.peerDoneHandler(sp)
 	}
 	s.wg.Done()
 	srvrLog.Tracef("Listener handler done for %s", listener.Addr())
 }
 
-// newOutboundPeer initializes a new outbound peer and setups the message
-// listeners.
-func (s *server) newOutboundPeer(addr string, persistent bool) *serverPeer {
-	sp := newServerPeer(s, persistent)
-	p, err := peer.NewOutboundPeer(newPeerConfig(sp), addr)
+// outboundPeerConnected is invoked by the connection manager when a new
+// outbound connection is established.  It initializes a new outbound server
+// peer instance, associates it with the relevant state such as the connection
+// request instance and the connection itself, and finally notifies the address
+// manager of the attempt.
+func (s *server) outboundPeerConnected(c *connmgr.ConnReq, conn net.Conn) {
+	sp := newServerPeer(s, c.Permanent)
+	p, err := peer.NewOutboundPeer(newPeerConfig(sp), c.Addr)
 	if err != nil {
-		srvrLog.Errorf("Cannot create outbound peer %s: %v", addr, err)
-		return nil
+		srvrLog.Debugf("Cannot create outbound peer %s: %v", c.Addr, err)
+		s.connManager.Disconnect(c.ID())
 	}
 	sp.Peer = p
+	sp.connReq = c
+	sp.AssociateConnection(conn)
 	go s.peerDoneHandler(sp)
-	return sp
+	s.addrManager.Attempt(sp.NA())
 }
 
 // peerDoneHandler handles peer disconnects by notifiying the server that it's
@@ -2430,14 +2435,7 @@ func newServer(listenAddrs []string, db database.DB, chainParams *chaincfg.Param
 		RetryDuration: connectionRetryInterval,
 		MaxOutbound:   defaultMaxOutbound,
 		Dial:          btcdDial,
-		OnConnection: func(c *connmgr.ConnReq, conn net.Conn) {
-			sp := s.newOutboundPeer(c.Addr, c.Permanent)
-			if sp != nil {
-				sp.AssociateConnection(conn)
-				sp.connReq = c
-				s.addrManager.Attempt(sp.NA())
-			}
-		},
+		OnConnection:  s.outboundPeerConnected,
 		GetNewAddress: newAddressFunc,
 	})
 	if err != nil {