Skip to content

feat: close/restart cleanup goroutine in runtime#214

Merged
ssagarverma merged 3 commits intohashicorp:mainfrom
jaime-amate:feat/close-restart-goroutine
Feb 24, 2026
Merged

feat: close/restart cleanup goroutine in runtime#214
ssagarverma merged 3 commits intohashicorp:mainfrom
jaime-amate:feat/close-restart-goroutine

Conversation

@jaime-amate
Copy link
Contributor

@jaime-amate jaime-amate commented Dec 23, 2025

Description

This PR adds the ability to stop and restart the cleanup goroutine at runtime for the expirable LRU cache. This feature provides better control over cache behavior and resource management, particularly useful in scenarios where you need to temporarily preserve expired entries or gracefully shutdown the cache.

New Methods

  • Close(): Stops the background cleanup goroutine that removes expired entries. After calling this method, expired items remain accessible in the cache until manually removed or the cleanup is restarted.
  • Restart(): Restarts the cleanup goroutine after it has been stopped with Close(). If the goroutine is already running, this method safely does nothing.

Implementation Details

  • Added a cleanupStopped boolean field to track the cleanup goroutine state
  • Modified read operations (Get, Peek, Keys, Values) to return expired items when the cleanup goroutine is stopped
  • Ensured thread-safety by protecting all state changes with the existing mutex
  • Prevented race conditions and duplicate goroutine creation

Behavior

When cleanup is running (default):

  • Expired items are automatically removed by the background goroutine
  • Get(), Peek(), etc. return false for expired items
  • Standard LRU with TTL behavior

When cleanup is stopped (after Close()):

  • Expired items remain in the cache
  • All read operations return expired items as if they were valid
  • No automatic cleanup occurs
  • Cache continues to enforce size limits through LRU eviction

Use Cases

  • Graceful Shutdown: Stop the cleanup goroutine before application shutdown to prevent goroutine leaks
  • Debugging: Temporarily stop cleanup to inspect expired entries
  • Cache Preservation: Pause automatic cleanup during maintenance windows or special operations
  • Testing: Better control over cache behavior in test scenarios

Breaking Changes

None. The changes are fully backward compatible. Existing code will continue to work as before, with the cleanup goroutine running by default.

cache := NewLRU[string, string](100, nil, time.Second*30)

// Normal operation
cache.Add("key1", "value1")

// Stop cleanup - expired items remain accessible
cache.Close()

// ... expired items can still be retrieved ...

// Resume normal operation with cleanup
cache.Restart()

Related Issue

How Has This Been Tested?

Added comprehensive test coverage including:

  • TestCache_CloseGoRoutine: Verifies expired items remain accessible after Close()
  • TestCache_RestartGoRoutine: Confirms cleanup resumes after Restart()

@hashicorp-cla-app
Copy link

hashicorp-cla-app bot commented Dec 23, 2025

CLA assistant check
All committers have signed the CLA.

@hashicorp-cla-app
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@jaime-amate jaime-amate marked this pull request as ready for review December 23, 2025 12:20
@jaime-amate jaime-amate requested review from a team as code owners December 23, 2025 12:20
@schmichael
Copy link
Member

I added cf37413 because the cleanup goroutine might have been sleeping when Close() was called. The cleanup goroutine lacked any checks for Close calls after sleeping, so I believe entries could have been deleted even after Close() was called.

Comment on lines +309 to +313
select {
case <-time.After(timeToExpire):
case <-done:
return
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is merely an optimization to exit this goroutine ASAP instead of letting it sleep up to the ttl.

Comment on lines +316 to +321
select {
case <-done:
// Done channel closed while sleeping, return without deleting entries
return
default:
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the correctness fix from cf37413. Without checking the done channel after sleeping, if Close() was called during time.Sleep (now time.After) then the cleanup goroutine would have made one more deletion pass after Close() was called.

}

// startGoroutine starts the cleanup goroutine for expired entries.
func (c *LRU[K, V]) startGoroutine() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: might be nice for this function to be named what it does and not how it's implemented.

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mismithhisler
Copy link
Member

Just a thought that I don't needs to be required here but if you wrapped the check on the done chan in a function, you could probably get rid of the cleanupStopped bool? The upside being you don't need to worry about keeping done and cleanupStopped in sync.

@Skarlso
Copy link

Skarlso commented Feb 2, 2026

Would want this too, because testing the cache is a bit difficult without the ability to stop it. And clearing things, and this just generally leaks the cleanup go routine without a proper stopping.

@jaime-amate
Copy link
Contributor Author

@tgross could we get review from hashicorp/team-ip-compliance?

@tgross
Copy link
Member

tgross commented Feb 23, 2026

@tgross could we get review from hashicorp/team-ip-compliance?

Sorry about that, I'm not in that group so I can't merge this myself. I've given that group a gentle nudge.

@ssagarverma ssagarverma merged commit 5015efc into hashicorp:main Feb 24, 2026
5 checks passed
@Skarlso
Copy link

Skarlso commented Feb 24, 2026

Yay 🎉 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants