Skip to content

Commit

Permalink
Fix connect leak from clientpool
Browse files Browse the repository at this point in the history
It's also fixed upstream in thrift and will be part of thrift 0.16.0,
but the double fix is harmless and will help us when it's used on older
thrift versions.
  • Loading branch information
fishy committed Feb 4, 2022
1 parent 49770a8 commit b1ba649
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions clientpool/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ func (cp *channelPool) Get() (client Client, err error) {
if c.IsOpen() {
return c, nil
}
// For thrift connections, IsOpen could return false in both explicit and
// implicit closed situations.
// In implicit closed situation, IsOpen does a connectivity check and
// returns false if that check fails. In such case we should still close the
// connection explicitly to avoid resource leak.
// In explicit situation, calling Close again will just return an already
// closed error, which is harmless here.
c.Close()
default:
}

Expand All @@ -85,6 +93,11 @@ func (cp *channelPool) Release(c Client) error {
defer atomic.AddInt32(&cp.numActive, -1)

if !c.IsOpen() {
// Even when c.IsOpen reported false, still call Close explicitly to avoid
// connection leaks. At worst case scenario it just returns an already
// closed error, which is still harmless.
c.Close()

newC, err := cp.opener()
if err != nil {
return err
Expand Down

0 comments on commit b1ba649

Please sign in to comment.