From dfa93436c24f6f9e8d9d52746b06587650d34cec Mon Sep 17 00:00:00 2001 From: David Wragg Date: Fri, 22 May 2015 11:26:30 +0100 Subject: [PATCH] Fix use of "break" within "select" in LocalConnection actorLoop From the golang spec: 'A "break" statement terminates execution of the innermost "for", "switch", or "select" statement within the same function.' But the LocalConnection actorLoop seems to assume it skips the "select" and terminates the enclosing "for". As a result, if something did conn.Shutdown(nil), it would not actually shutdown the connection. It turns out this is never a problem, because Shutdown is always called with a non-nil error, even though there is code in a few places to handle the nil case. By cleaning that up, the dubious "break" is eliminated. --- router/connection.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/router/connection.go b/router/connection.go index 7e380730..2baa2233 100644 --- a/router/connection.go +++ b/router/connection.go @@ -196,13 +196,14 @@ func (conn *LocalConnection) SendProtocolMsg(m ProtocolMsg) { // Async func (conn *LocalConnection) Shutdown(err error) { + // err should always be a real error, even if only io.EOF + if err == nil { + panic("nil error") + } + // Run on its own goroutine in case the channel is backed up go func() { - if err == nil { - conn.sendAction(nil) - } else { - conn.sendAction(func() error { return err }) - } + conn.sendAction(func() error { return err }) }() } @@ -379,9 +380,6 @@ func (conn *LocalConnection) actorLoop(actionChan <-chan ConnectionAction) (err for err == nil { select { case action := <-actionChan: - if action == nil { - break - } err = action() case <-conn.heartbeatTCP.C: err = conn.sendSimpleProtocolMsg(ProtocolHeartbeat) @@ -398,13 +396,10 @@ func (conn *LocalConnection) actorLoop(actionChan <-chan ConnectionAction) (err } func (conn *LocalConnection) shutdown(err error) { - switch { - case err != nil && conn.remote == nil: + if conn.remote == nil { log.Printf("->[%s] connection shutting down due to error during handshake: %v\n", conn.remoteTCPAddr, err) - case err != nil: + } else { conn.Log("connection shutting down due to error:", err) - default: - conn.Log("connection shutting down") } if conn.TCPConn != nil {