peer: refactor main event loop for ping handler

The error was never used as the init couldn't return an error, so we do
away with that. We also modify the main event loop dispatch to more
closely match other areas of the codebase.

peer/ping_manager.go

@@ -98,16 +98,20 @@ func NewPingManager(cfg *PingManagerConfig) *PingManager {
 func (m *PingManager) Start() error {
 	var err error
 	m.started.Do(func() {
-		err = m.start()
+		m.pingTicker = time.NewTicker(m.cfg.IntervalDuration)
+		m.pingTimeout = time.NewTimer(0)
+
+		m.wg.Add(1)
+		go m.pingHandler()
 	})
 
 	return err
 }
 
-func (m *PingManager) start() error {
+// pingHandler is the main goroutine responsible for enforcing the ping/pong
-	m.pingTicker = time.NewTicker(m.cfg.IntervalDuration)
+// protocol.
-
+func (m *PingManager) pingHandler() {
-	m.pingTimeout = time.NewTimer(0)
+	defer m.wg.Done()
 	defer m.pingTimeout.Stop()
 
 	// Ensure that the pingTimeout channel is empty.
@@ -115,17 +119,13 @@ func (m *PingManager) start() error {
 		<-m.pingTimeout.C
 	}
 
-	m.wg.Add(1)
-	go func() {
-		defer m.wg.Done()
 	for {
 		select {
 		case <-m.pingTicker.C:
-				// If this occurs it means that the new ping
+			// If this occurs it means that the new ping cycle has
-				// cycle has begun while there is still an
+			// begun while there is still an outstanding ping
-				// outstanding ping awaiting a pong response.
+			// awaiting a pong response.  This should never occur,
-				// This should never occur, but if it does, it
+			// but if it does, it implies a timeout.
-				// implies a timeout.
 			if m.outstandingPongSize >= 0 {
 				e := errors.New("impossible: new ping" +
 					"in unclean state",
@@ -143,7 +143,6 @@ func (m *PingManager) start() error {
 
 			// Set up our bookkeeping for the new Ping.
 			if err := m.setPingState(pongSize); err != nil {
-
 				m.cfg.OnPongFailure(err)
 
 				return
@@ -165,15 +164,15 @@ func (m *PingManager) start() error {
 		case pong := <-m.pongChan:
 			pongSize := int32(len(pong.PongBytes))
 
-				// Save off values we are about to override
+			// Save off values we are about to override when we
-				// when we call resetPingState.
+			// call resetPingState.
 			expected := m.outstandingPongSize
 			lastPing := m.pingLastSend
 
 			m.resetPingState()
 
-				// If the pong we receive doesn't match the
+			// If the pong we receive doesn't match the ping we
-				// ping we sent out, then we fail out.
+			// sent out, then we fail out.
 			if pongSize != expected {
 				e := errors.New("pong response does " +
 					"not match expected size",
@@ -195,9 +194,6 @@ func (m *PingManager) start() error {
 			return
 		}
 	}
-	}()
-
-	return nil
 }
 
 // Stop interrupts the goroutines that the PingManager owns. Can only be called