From 3bc4f52ef9e663a61d1e7b54227ca2e23b2e7f8a Mon Sep 17 00:00:00 2001 From: YACOVM Date: Fri, 23 Dec 2016 22:22:27 +0200 Subject: [PATCH] Close Gossip comm server-side connection in defer In the gossip comm layer, if the connection is a server-side one, and the conn.serviceConnection() method returns, the connection needs to terminate. The goroutines that service that connection are terminated by asking the connectionStore to close that connection. But, if a new connection has been created (i.e from the client-side) in the meantime before the connStore deletes the connection, the connStore would close the new connection instead of the old one, and the goroutines that serve the old connection would still be alive until the process is shut down. This is a possible goroutine leak. The simplest way to fix this, is to call conn.close() regardless, which is safe because if a connection is closed twice, one of the attempts to close it is aborted because we have a CAS to ensure only 1 successfull invocation of conn.close. Change-Id: I2b9f2378cf07a547dbf900bb75aa1b196e7093b3 Signed-off-by: Yacov Manevich --- gossip/comm/comm_impl.go | 1 + 1 file changed, 1 insertion(+) diff --git a/gossip/comm/comm_impl.go b/gossip/comm/comm_impl.go index 850f11f8385..5eaf02651ce 100644 --- a/gossip/comm/comm_impl.go +++ b/gossip/comm/comm_impl.go @@ -476,6 +476,7 @@ func (c *commImpl) GossipStream(stream proto.Gossip_GossipStreamServer) error { defer func() { c.logger.Info("Client", extractRemoteAddress(stream), " disconnected") c.connStore.closeByPKIid(PKIID) + conn.close() }() return conn.serviceConnection()