Skip to content

Commit

Permalink
VAULT-4240 time.After() in a select statement can lead to memory leak (
Browse files Browse the repository at this point in the history
…#14814)

* VAULT-4240 time.After() in a select statement can lead to memory leak

* CL
  • Loading branch information
hghaf099 committed Apr 1, 2022
1 parent eb79ba0 commit b1012f2
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 3 deletions.
3 changes: 3 additions & 0 deletions changelog/14814.txt
@@ -0,0 +1,3 @@
```release-note:bug
core: time.After() used in a select statement can lead to memory leak
```
7 changes: 6 additions & 1 deletion helper/fairshare/jobmanager.go
Expand Up @@ -265,6 +265,10 @@ func (j *JobManager) assignWork() {
j.wg.Add(1)

go func() {
// ticker is used to prevent memory leak of using time.After in
// for - select pattern.
ticker := time.NewTicker(50 * time.Millisecond)
defer ticker.Stop()
for {
for {
// assign work while there are jobs to distribute
Expand All @@ -291,13 +295,14 @@ func (j *JobManager) assignWork() {
}
}

ticker.Reset(50 * time.Millisecond)
select {
case <-j.quit:
j.wg.Done()
return
case <-j.newWork:
// listen for wake-up when an empty job manager has been given work
case <-time.After(50 * time.Millisecond):
case <-ticker.C:
// periodically check if new workers can be assigned. with the
// fairsharing worker distribution it can be the case that there
// is work waiting, but no queues are eligible for another worker
Expand Down
7 changes: 6 additions & 1 deletion physical/raft/raft.go
Expand Up @@ -860,11 +860,16 @@ func (b *RaftBackend) SetupCluster(ctx context.Context, opts SetupOpts) error {
// StartAsLeader is only set during init, recovery mode, storage migration,
// and tests.
if opts.StartAsLeader {
// ticker is used to prevent memory leak of using time.After in
// for - select pattern.
ticker := time.NewTicker(10 * time.Millisecond)
defer ticker.Stop()
for {
if raftObj.State() == raft.Leader {
break
}

ticker.Reset(10 * time.Millisecond)
select {
case <-ctx.Done():
future := raftObj.Shutdown()
Expand All @@ -873,7 +878,7 @@ func (b *RaftBackend) SetupCluster(ctx context.Context, opts SetupOpts) error {
}

return errors.New("shutdown while waiting for leadership")
case <-time.After(10 * time.Millisecond):
case <-ticker.C:
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion vault/raft.go
Expand Up @@ -481,6 +481,10 @@ func (c *Core) raftTLSRotatePhased(ctx context.Context, logger hclog.Logger, raf
defer keyCheckInterval.Stop()

var backoff bool
// ticker is used to prevent memory leak of using time.After in
// for - select pattern.
ticker := time.NewTicker(time.Until(nextRotationTime))
defer ticker.Stop()
for {
// If we encountered and error we should try to create the key
// again.
Expand All @@ -489,13 +493,14 @@ func (c *Core) raftTLSRotatePhased(ctx context.Context, logger hclog.Logger, raf
backoff = false
}

ticker.Reset(time.Until(nextRotationTime))
select {
case <-keyCheckInterval.C:
err := checkCommitted()
if err != nil {
logger.Error("failed to activate TLS key", "error", err)
}
case <-time.After(time.Until(nextRotationTime)):
case <-ticker.C:
// It's time to rotate the keys
next, err := rotateKeyring()
if err != nil {
Expand Down

0 comments on commit b1012f2

Please sign in to comment.