From 92ee21ba925117281a5623cd83613656dc4ae513 Mon Sep 17 00:00:00 2001
From: "R.B. Boyer" <4903+rboyer@users.noreply.github.com>
Date: Fri, 9 Jun 2023 11:27:04 -0500
Subject: [PATCH] cache: fix a few minor goroutine leaks in leaf certs and the
 agent cache (#17636)

---
 .changelog/17636.txt                 |  3 +++
 agent/cache-types/connect_ca_leaf.go | 20 +++++++++++++-------
 agent/cache/cache.go                 |  7 +++++--
 3 files changed, 21 insertions(+), 9 deletions(-)
 create mode 100644 .changelog/17636.txt

diff --git a/.changelog/17636.txt b/.changelog/17636.txt
new file mode 100644
index 00000000000..aa06f9191b9
--- /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 7fff8e6c226..e53097021e5 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.
@@ -426,20 +425,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())
 
@@ -480,7 +484,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 e31d8040fd2..30be0507e1c 100644
--- a/agent/cache/cache.go
+++ b/agent/cache/cache.go
@@ -71,7 +71,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
@@ -510,7 +510,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