diff --git a/.changelog/17636.txt b/.changelog/17636.txt new file mode 100644 index 0000000000..aa06f9191b --- /dev/null +++ b/.changelog/17636.txt @@ -0,0 +1,3 @@ +```release-note:bug +cache: fix a few minor goroutine leaks in leaf certs and the agent cache +``` diff --git a/agent/cache-types/connect_ca_leaf.go b/agent/cache-types/connect_ca_leaf.go index f12ce1ece6..4d146c586c 100644 --- a/agent/cache-types/connect_ca_leaf.go +++ b/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 } diff --git a/agent/cache/cache.go b/agent/cache/cache.go index ea537cc9e6..5871d826fd 100644 --- a/agent/cache/cache.go +++ b/agent/cache/cache.go @@ -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