Skip to content

Commit

Permalink
Close Gossip comm server-side connection in defer
Browse files Browse the repository at this point in the history
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 <yacovm@il.ibm.com>
  • Loading branch information
yacovm committed Jan 5, 2017
1 parent 830ef1c commit 3bc4f52
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions gossip/comm/comm_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 3bc4f52

Please sign in to comment.