Browse Source

cache: fix a few minor goroutine leaks in leaf certs and the agent cache (#17636)

pull/17451/head
R.B. Boyer 1 year ago committed by GitHub
parent
commit
75451c1490
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      .changelog/17636.txt
  2. 20
      agent/cache-types/connect_ca_leaf.go
  3. 7
      agent/cache/cache.go

3
.changelog/17636.txt

@ -0,0 +1,3 @@
```release-note:bug
cache: fix a few minor goroutine leaks in leaf certs and the agent cache
```

20
agent/cache-types/connect_ca_leaf.go

@ -12,12 +12,11 @@ import (
"github.com/mitchellh/hashstructure"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/agent/cache"
"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/consul"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib"
)
// Recommended name for registration.
@ -424,20 +423,25 @@ func (c *ConnectCALeaf) Fetch(opts cache.FetchOptions, req cache.Request) (cache
// Setup the timeout chan outside the loop so we don't keep bumping the timeout
// later if we loop around.
timeoutCh := time.After(opts.Timeout)
timeoutTimer := time.NewTimer(opts.Timeout)
defer timeoutTimer.Stop()
// Setup initial expiry chan. We may change this if root update occurs in the
// loop below.
expiresCh := time.After(expiresAt.Sub(now))
expiresTimer := time.NewTimer(expiresAt.Sub(now))
defer func() {
// Resolve the timer reference at defer time, so we use the latest one each time.
expiresTimer.Stop()
}()
// Current cert is valid so just wait until it expires or we time out.
for {
select {
case <-timeoutCh:
case <-timeoutTimer.C:
// We timed out the request with same cert.
return lastResultWithNewState(), nil
case <-expiresCh:
case <-expiresTimer.C:
// Cert expired or was force-expired by a root change.
return c.generateNewLeaf(reqReal, lastResultWithNewState())
@ -478,7 +482,9 @@ func (c *ConnectCALeaf) Fetch(opts cache.FetchOptions, req cache.Request) (cache
// loop back around, we'll wait at most delay until generating a new cert.
if state.forceExpireAfter.Before(expiresAt) {
expiresAt = state.forceExpireAfter
expiresCh = time.After(delay)
// Stop the former one and create a new one.
expiresTimer.Stop()
expiresTimer = time.NewTimer(delay)
}
continue
}

7
agent/cache/cache.go vendored

@ -91,7 +91,7 @@ const (
// rate limiter settings.
// DefaultEntryFetchRate is the default rate at which cache entries can
// be fetch. This defaults to not being unlimited
// be fetch. This defaults to not being limited
DefaultEntryFetchRate = rate.Inf
// DefaultEntryFetchMaxBurst is the number of cache entry fetches that can
@ -533,7 +533,10 @@ RETRY_GET:
// Set our timeout channel if we must
if r.Info.Timeout > 0 && timeoutCh == nil {
timeoutCh = time.After(r.Info.Timeout)
timeoutTimer := time.NewTimer(r.Info.Timeout)
defer timeoutTimer.Stop()
timeoutCh = timeoutTimer.C
}
// At this point, we know we either don't have a value at all or the

Loading…
Cancel
Save