Skip to content

Commit 0b36e7a

Browse files
committed
agent: fix Connect CA data race and cache watcher timer leak
- leader_connect_ca.go: Hold stateLock across primaryRoots.Roots iteration in secondaryGetActivePrimaryCARoot() to avoid data race. - watch.go: Replace time.After with time.NewTimer and timer.Stop() in notifyBlockingQuery and notifyPollingQuery to prevent memory leaks on context cancellation.
1 parent 681197a commit 0b36e7a

3 files changed

Lines changed: 24 additions & 10 deletions

File tree

.changelog/23854.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
```release-note:bug
2+
connect: Fix data race in secondaryGetActivePrimaryCARoot by holding stateLock during primary root slice iteration.
3+
agent/cache: Fix memory leak in notifyBlockingQuery and notifyPollingQuery timer loops by replacing time.After with time.NewTimer and explicit timer.Stop().
4+
```

agent/cache/watch.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,11 @@ func (c *Cache) notifyBlockingQuery(ctx context.Context, r getOptions, correlati
141141
}
142142

143143
if wait > 0 {
144+
timer := time.NewTimer(wait)
144145
select {
145-
case <-time.After(wait):
146+
case <-timer.C:
146147
case <-ctx.Done():
148+
timer.Stop()
147149
return
148150
}
149151
}
@@ -239,10 +241,20 @@ func (c *Cache) notifyPollingQuery(ctx context.Context, r getOptions, correlatio
239241
wait += lib.RandomStagger(r.Info.MaxAge / 16)
240242
}
241243

242-
select {
243-
case <-time.After(wait):
244-
case <-ctx.Done():
245-
return
244+
if wait > 0 {
245+
timer := time.NewTimer(wait)
246+
select {
247+
case <-timer.C:
248+
case <-ctx.Done():
249+
timer.Stop()
250+
return
251+
}
252+
} else {
253+
select {
254+
case <-ctx.Done():
255+
return
256+
default:
257+
}
246258
}
247259
}
248260
}

agent/consul/leader_connect_ca.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,13 +203,11 @@ func (c *CAManager) secondarySetPrimaryRoots(newRoots structs.IndexedCARoots) {
203203
}
204204

205205
func (c *CAManager) secondaryGetActivePrimaryCARoot() (*structs.CARoot, error) {
206-
// TODO: this could be a different lock, as long as its the same lock in secondarySetPrimaryRoots
207206
c.stateLock.Lock()
208-
primaryRoots := c.primaryRoots
209-
c.stateLock.Unlock()
207+
defer c.stateLock.Unlock()
210208

211-
for _, root := range primaryRoots.Roots {
212-
if root.ID == primaryRoots.ActiveRootID && root.Active {
209+
for _, root := range c.primaryRoots.Roots {
210+
if root.ID == c.primaryRoots.ActiveRootID && root.Active {
213211
return root, nil
214212
}
215213
}

0 commit comments

Comments
 (0)