From 0691b21a30380973bee4fcf3130cb0afb63c5fdf Mon Sep 17 00:00:00 2001
From: Conner Fromknecht <conner@mit.edu>
Date: Mon, 7 May 2018 18:32:00 -0700
Subject: [PATCH 1/2] peer: improves disconnect handling

This commit attempts to resolve some potential deadlock
scenarios during a peer disconnect.

Currently, writeMessage returns a nil error when disconnecting.
This should have minimal impact on the writeHanlder, as the
subsequent loop selects on the quit chan, and will cause it to
exit. However, if this happens when sending the init message,
the Start() method will attempt to proceed even though the peer
has been disconnected.

In addition, this commit changes the behavior of synchronous
write errors, by using a non-blocking select. Though unlikely,
this prevents any cases where multiple errors are returned, and
the errors are not being pulled from the other side of the errChan.
This removes any naked sends on the errChan from stalling the peer's
shutdown.
---
 peer.go | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/peer.go b/peer.go
index ad9dc946..41c1fc16 100644
--- a/peer.go
+++ b/peer.go
@@ -30,6 +30,9 @@ import (
 
 var (
 	numNodes int32
+
+	// ErrPeerExiting signals that the peer received a disconnect request.
+	ErrPeerExiting = errors.Errorf("peer exiting")
 )
 
 const (
@@ -1105,7 +1108,7 @@ func (p *peer) logWireMessage(msg lnwire.Message, read bool) {
 func (p *peer) writeMessage(msg lnwire.Message) error {
 	// Simply exit if we're shutting down.
 	if atomic.LoadInt32(&p.disconnect) != 0 {
-		return nil
+		return ErrPeerExiting
 	}
 
 	// TODO(roasbeef): add message summaries
@@ -1154,8 +1157,8 @@ out:
 				atomic.StoreInt64(&p.pingLastSend, now)
 			}
 
-			// Write out the message to the socket, closing the
-			// 'sentChan' if it's non-nil, The 'sentChan' allows
+			// Write out the message to the socket, responding with
+			// error if `errChan` is non-nil. The `errChan` allows
 			// callers to optionally synchronize sends with the
 			// writeHandler.
 			err := p.writeMessage(outMsg.msg)
@@ -1169,7 +1172,7 @@ out:
 			}
 
 		case <-p.quit:
-			exitErr = errors.Errorf("peer exiting")
+			exitErr = ErrPeerExiting
 			break out
 		}
 	}
@@ -1263,7 +1266,7 @@ func (p *peer) queueMsg(msg lnwire.Message, errChan chan error) {
 	case <-p.quit:
 		peerLog.Tracef("Peer shutting down, could not enqueue msg.")
 		if errChan != nil {
-			errChan <- fmt.Errorf("peer shutting down")
+			errChan <- ErrPeerExiting
 		}
 	}
 }

From d20adc8e0e37d6221ca79d8951466b1210c95e59 Mon Sep 17 00:00:00 2001
From: Conner Fromknecht <conner@mit.edu>
Date: Mon, 7 May 2018 18:36:08 -0700
Subject: [PATCH 2/2] server: use nil errChan for broadcasts

This skips creating errChans when sending messages to
peer during broadcast. This should be a minor memory
optimization, as well as not requiring channel sends
on those which will never be read.
---
 server.go | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/server.go b/server.go
index 9c259f44..9783dc51 100644
--- a/server.go
+++ b/server.go
@@ -1091,7 +1091,7 @@ func (s *server) SendToPeer(target *btcec.PublicKey,
 		case err := <-errChan:
 			return err
 		case <-targetPeer.quit:
-			return fmt.Errorf("peer shutting down")
+			return ErrPeerExiting
 		case <-s.quit:
 			return ErrServerShuttingDown
 		}
@@ -1183,7 +1183,8 @@ func (s *server) sendPeerMessages(
 	// event, we defer a call to Done on both WaitGroups to 1) ensure that
 	// server will be able to shutdown after its go routines exit, and 2)
 	// so the server can return to the caller of BroadcastMessage.
-	if wg != nil {
+	isBroadcast := wg != nil
+	if isBroadcast {
 		defer s.wg.Done()
 		defer wg.Done()
 	}
@@ -1193,9 +1194,16 @@ func (s *server) sendPeerMessages(
 	// the queue.
 	var errChans []chan error
 	for _, msg := range msgs {
-		errChan := make(chan error, 1)
+		// If this is not broadcast, create error channels to provide
+		// synchronous feedback regarding the delivery of the message to
+		// a specific peer.
+		var errChan chan error
+		if !isBroadcast {
+			errChan = make(chan error, 1)
+			errChans = append(errChans, errChan)
+		}
+
 		targetPeer.queueMsg(msg, errChan)
-		errChans = append(errChans, errChan)
 	}
 
 	return errChans