TT-17147: fix: remove unconditional break in memorycache cleanup timer#8180
Conversation
|
This PR addresses a critical memory leak in the The fix ensures the cleanup timer runs continuously as intended. To validate this and prevent regressions, the PR introduces comprehensive tests: a unit test ( Files Changed Analysis
Architecture & Impact Assessment
graph TD
subgraph "Before"
A[Start Timer] --> B{Select};
B -- ticker.C --> C["cleanup()"];
C --> D[break];
D --> E[Exit Loop];
end
subgraph "After"
F[Start Timer] --> G{Select};
G -- ticker.C --> H["cleanup()"];
H --> G;
G -- ctx.Done --> I[Exit Loop];
end
Scope Discovery & Context ExpansionThe bug in Further analysis confirms that this generic caching component is also used by:
By fixing the underlying cache implementation, this PR provides a crucial stability improvement across multiple high-throughput features of the gateway, preventing potential memory-related outages. Metadata
Powered by Visor from Probelabs Last updated: 2026-05-15T10:42:24.170Z | Triggered by: pr_updated | Commit: 8549fa3 💡 TIP: You can chat with Visor using |
✅ Security Check PassedNo security issues found – changes LGTM. ✅ Security Check PassedNo security issues found – changes LGTM. \n\n✅ Architecture Check PassedNo architecture issues found – changes LGTM. ✅ Performance Check PassedNo performance issues found – changes LGTM. Quality Issues (1)
Powered by Visor from Probelabs Last updated: 2026-05-15T10:42:13.456Z | Triggered by: pr_updated | Commit: 8549fa3 💡 TIP: You can chat with Visor using |
🎯 Recommended Merge TargetsBased on JIRA ticket TT-17147: Memory Leak in internal/memorycache.Cache.startCleanupTimer Fix Version: Tyk 5.8.14Required:
Fix Version: Tyk 5.13.0Required:
Recommended:
📋 Workflow
|
Remove trailing whitespace on two blank lines and add the missing final newline. Fixes golangci-lint gci finding flagged on PR #8180.
🚨 Jira Linter FailedCommit: The Jira linter failed to validate your PR. Please check the error details below: 🔍 Click to view error detailsNext Steps
This comment will be automatically deleted once the linter passes. |
|
|
/release to release-5.13 |
|
/release to release-5.13.0 |
|
✅ Cherry-pick successful. A PR was created: #8227 |
|
✅ Cherry-pick successful. A PR was created: #8228 |
|
/release to release-5.8 |
|
/release to release-5.8.14 |
|
✅ Cherry-pick successful. A PR was created: #8229 |
|
✅ Cherry-pick successful. A PR was created: #8230 |
…in memorycache cleanup timer (#8180) (#8228) TT-17147: fix: remove unconditional break in memorycache cleanup timer (#8180) ## Problem / Task The `internal/memorycache.Cache.startCleanupTimer` exits after one cleanup due to an unconditional `break` inside the for/select loop. This causes `BucketStorage` entries used by `SessionLimiter.limitDRL` to accumulate indefinitely, leading to a massive memory leak (e.g., 1.9 GB live heap) when using rate limiting by client IP. ## Who is affected? Users are potentially affected by this memory leak if they meet **all** of the following conditions: - Running Tyk Gateway version **5.8.x** (or any version where `internal/memorycache` was introduced/embedded). - Using **Rate Limiting** (specifically the default distributed rate limiter `limitDRL`, which relies on `BucketStorage` backed by `memorycache`). - Experiencing a high cardinality of rate limit keys (e.g., rate limiting by **Client IP** or using custom gRPC plugins that generate many unique rate limit keys). ## Changes - Removed the unconditional `break` statement in `internal/memorycache/cache.go` inside `startCleanupTimer` so the loop continues to run and clean up expired items. - Added `TestCache_CleanupTimer` in `internal/memorycache/cache_test.go` to verify the cleanup timer runs multiple times and correctly removes expired items. - Added `BenchmarkCache_MemoryLeak` in `internal/memorycache/cache_test.go` to validate memory usage and performance of the cache over time, simulating the memory leak scenario. ## Benchmark Results To validate the fix, we ran `BenchmarkCache_MemoryLeak` which reports an `items_left` metric showing how many items remain in the cache at the end of the run. **Before the fix (with the unconditional break):** ```text BenchmarkCache_MemoryLeak-16 3703683 1384 ns/op 456112 items_left 101 B/op 3 allocs/op ``` *Result: 456,112 items left in the cache. The cleanup goroutine was dead, causing items to accumulate indefinitely.* **After the fix (unconditional break removed):** ```text BenchmarkCache_MemoryLeak-16 3742993 1387 ns/op 1.000 items_left 90 B/op 3 allocs/op ``` *Result: Only 1 item left in the cache. The cleanup timer runs continuously and successfully evicts expired items.* ## Testing - Ran `go test -v -run TestCache_CleanupTimer ./internal/memorycache` which passes. - Ran `go test -bench=BenchmarkCache_MemoryLeak -benchmem -run=^$ ./internal/memorycache` which passes and shows stable memory usage. - Verified the build passes with `make build`. --------- Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]> <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17147" title="TT-17147" target="_blank">TT-17147</a> </summary> | | | |---------|----| | Status | Closed | | Summary | Memory Leak in internal/memorycache.Cache.startCleanupTimer | Generated at: 2026-05-15 11:47:30 </details> <!---TykTechnologies/jira-linter ends here--> Co-authored-by: probelabs[bot] <219682034+probelabs[bot]@users.noreply.github.com> Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]>
… memorycache cleanup timer (#8180) (#8227) TT-17147: fix: remove unconditional break in memorycache cleanup timer (#8180) ## Problem / Task The `internal/memorycache.Cache.startCleanupTimer` exits after one cleanup due to an unconditional `break` inside the for/select loop. This causes `BucketStorage` entries used by `SessionLimiter.limitDRL` to accumulate indefinitely, leading to a massive memory leak (e.g., 1.9 GB live heap) when using rate limiting by client IP. ## Who is affected? Users are potentially affected by this memory leak if they meet **all** of the following conditions: - Running Tyk Gateway version **5.8.x** (or any version where `internal/memorycache` was introduced/embedded). - Using **Rate Limiting** (specifically the default distributed rate limiter `limitDRL`, which relies on `BucketStorage` backed by `memorycache`). - Experiencing a high cardinality of rate limit keys (e.g., rate limiting by **Client IP** or using custom gRPC plugins that generate many unique rate limit keys). ## Changes - Removed the unconditional `break` statement in `internal/memorycache/cache.go` inside `startCleanupTimer` so the loop continues to run and clean up expired items. - Added `TestCache_CleanupTimer` in `internal/memorycache/cache_test.go` to verify the cleanup timer runs multiple times and correctly removes expired items. - Added `BenchmarkCache_MemoryLeak` in `internal/memorycache/cache_test.go` to validate memory usage and performance of the cache over time, simulating the memory leak scenario. ## Benchmark Results To validate the fix, we ran `BenchmarkCache_MemoryLeak` which reports an `items_left` metric showing how many items remain in the cache at the end of the run. **Before the fix (with the unconditional break):** ```text BenchmarkCache_MemoryLeak-16 3703683 1384 ns/op 456112 items_left 101 B/op 3 allocs/op ``` *Result: 456,112 items left in the cache. The cleanup goroutine was dead, causing items to accumulate indefinitely.* **After the fix (unconditional break removed):** ```text BenchmarkCache_MemoryLeak-16 3742993 1387 ns/op 1.000 items_left 90 B/op 3 allocs/op ``` *Result: Only 1 item left in the cache. The cleanup timer runs continuously and successfully evicts expired items.* ## Testing - Ran `go test -v -run TestCache_CleanupTimer ./internal/memorycache` which passes. - Ran `go test -bench=BenchmarkCache_MemoryLeak -benchmem -run=^$ ./internal/memorycache` which passes and shows stable memory usage. - Verified the build passes with `make build`. --------- Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]> <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17147" title="TT-17147" target="_blank">TT-17147</a> </summary> | | | |---------|----| | Status | Closed | | Summary | Memory Leak in internal/memorycache.Cache.startCleanupTimer | Generated at: 2026-05-15 11:47:23 </details> <!---TykTechnologies/jira-linter ends here--> Co-authored-by: probelabs[bot] <219682034+probelabs[bot]@users.noreply.github.com> Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]>
…in memorycache cleanup timer (#8180) (#8230) TT-17147: fix: remove unconditional break in memorycache cleanup timer (#8180) ## Problem / Task The `internal/memorycache.Cache.startCleanupTimer` exits after one cleanup due to an unconditional `break` inside the for/select loop. This causes `BucketStorage` entries used by `SessionLimiter.limitDRL` to accumulate indefinitely, leading to a massive memory leak (e.g., 1.9 GB live heap) when using rate limiting by client IP. ## Who is affected? Users are potentially affected by this memory leak if they meet **all** of the following conditions: - Running Tyk Gateway version **5.8.x** (or any version where `internal/memorycache` was introduced/embedded). - Using **Rate Limiting** (specifically the default distributed rate limiter `limitDRL`, which relies on `BucketStorage` backed by `memorycache`). - Experiencing a high cardinality of rate limit keys (e.g., rate limiting by **Client IP** or using custom gRPC plugins that generate many unique rate limit keys). ## Changes - Removed the unconditional `break` statement in `internal/memorycache/cache.go` inside `startCleanupTimer` so the loop continues to run and clean up expired items. - Added `TestCache_CleanupTimer` in `internal/memorycache/cache_test.go` to verify the cleanup timer runs multiple times and correctly removes expired items. - Added `BenchmarkCache_MemoryLeak` in `internal/memorycache/cache_test.go` to validate memory usage and performance of the cache over time, simulating the memory leak scenario. ## Benchmark Results To validate the fix, we ran `BenchmarkCache_MemoryLeak` which reports an `items_left` metric showing how many items remain in the cache at the end of the run. **Before the fix (with the unconditional break):** ```text BenchmarkCache_MemoryLeak-16 3703683 1384 ns/op 456112 items_left 101 B/op 3 allocs/op ``` *Result: 456,112 items left in the cache. The cleanup goroutine was dead, causing items to accumulate indefinitely.* **After the fix (unconditional break removed):** ```text BenchmarkCache_MemoryLeak-16 3742993 1387 ns/op 1.000 items_left 90 B/op 3 allocs/op ``` *Result: Only 1 item left in the cache. The cleanup timer runs continuously and successfully evicts expired items.* ## Testing - Ran `go test -v -run TestCache_CleanupTimer ./internal/memorycache` which passes. - Ran `go test -bench=BenchmarkCache_MemoryLeak -benchmem -run=^$ ./internal/memorycache` which passes and shows stable memory usage. - Verified the build passes with `make build`. --------- Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]> <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17147" title="TT-17147" target="_blank">TT-17147</a> </summary> | | | |---------|----| | Status | Closed | | Summary | Memory Leak in internal/memorycache.Cache.startCleanupTimer | Generated at: 2026-05-15 11:48:26 </details> <!---TykTechnologies/jira-linter ends here--> Co-authored-by: probelabs[bot] <219682034+probelabs[bot]@users.noreply.github.com> Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]>
…memorycache cleanup timer (#8180) (#8229) TT-17147: fix: remove unconditional break in memorycache cleanup timer (#8180) ## Problem / Task The `internal/memorycache.Cache.startCleanupTimer` exits after one cleanup due to an unconditional `break` inside the for/select loop. This causes `BucketStorage` entries used by `SessionLimiter.limitDRL` to accumulate indefinitely, leading to a massive memory leak (e.g., 1.9 GB live heap) when using rate limiting by client IP. ## Who is affected? Users are potentially affected by this memory leak if they meet **all** of the following conditions: - Running Tyk Gateway version **5.8.x** (or any version where `internal/memorycache` was introduced/embedded). - Using **Rate Limiting** (specifically the default distributed rate limiter `limitDRL`, which relies on `BucketStorage` backed by `memorycache`). - Experiencing a high cardinality of rate limit keys (e.g., rate limiting by **Client IP** or using custom gRPC plugins that generate many unique rate limit keys). ## Changes - Removed the unconditional `break` statement in `internal/memorycache/cache.go` inside `startCleanupTimer` so the loop continues to run and clean up expired items. - Added `TestCache_CleanupTimer` in `internal/memorycache/cache_test.go` to verify the cleanup timer runs multiple times and correctly removes expired items. - Added `BenchmarkCache_MemoryLeak` in `internal/memorycache/cache_test.go` to validate memory usage and performance of the cache over time, simulating the memory leak scenario. ## Benchmark Results To validate the fix, we ran `BenchmarkCache_MemoryLeak` which reports an `items_left` metric showing how many items remain in the cache at the end of the run. **Before the fix (with the unconditional break):** ```text BenchmarkCache_MemoryLeak-16 3703683 1384 ns/op 456112 items_left 101 B/op 3 allocs/op ``` *Result: 456,112 items left in the cache. The cleanup goroutine was dead, causing items to accumulate indefinitely.* **After the fix (unconditional break removed):** ```text BenchmarkCache_MemoryLeak-16 3742993 1387 ns/op 1.000 items_left 90 B/op 3 allocs/op ``` *Result: Only 1 item left in the cache. The cleanup timer runs continuously and successfully evicts expired items.* ## Testing - Ran `go test -v -run TestCache_CleanupTimer ./internal/memorycache` which passes. - Ran `go test -bench=BenchmarkCache_MemoryLeak -benchmem -run=^$ ./internal/memorycache` which passes and shows stable memory usage. - Verified the build passes with `make build`. --------- Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]> <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17147" title="TT-17147" target="_blank">TT-17147</a> </summary> | | | |---------|----| | Status | Closed | | Summary | Memory Leak in internal/memorycache.Cache.startCleanupTimer | Generated at: 2026-05-15 11:48:19 </details> <!---TykTechnologies/jira-linter ends here--> Co-authored-by: probelabs[bot] <219682034+probelabs[bot]@users.noreply.github.com> Co-authored-by: Leonid Bugaev <[email protected]> Co-authored-by: Matias <[email protected]>



Problem / Task
The
internal/memorycache.Cache.startCleanupTimerexits after one cleanup due to an unconditionalbreakinside the for/select loop. This causesBucketStorageentries used bySessionLimiter.limitDRLto accumulate indefinitely, leading to a massive memory leak (e.g., 1.9 GB live heap) when using rate limiting by client IP.Who is affected?
Users are potentially affected by this memory leak if they meet all of the following conditions:
internal/memorycachewas introduced/embedded).limitDRL, which relies onBucketStoragebacked bymemorycache).Changes
breakstatement ininternal/memorycache/cache.goinsidestartCleanupTimerso the loop continues to run and clean up expired items.TestCache_CleanupTimerininternal/memorycache/cache_test.goto verify the cleanup timer runs multiple times and correctly removes expired items.BenchmarkCache_MemoryLeakininternal/memorycache/cache_test.goto validate memory usage and performance of the cache over time, simulating the memory leak scenario.Benchmark Results
To validate the fix, we ran
BenchmarkCache_MemoryLeakwhich reports anitems_leftmetric showing how many items remain in the cache at the end of the run.Before the fix (with the unconditional break):
Result: 456,112 items left in the cache. The cleanup goroutine was dead, causing items to accumulate indefinitely.
After the fix (unconditional break removed):
Result: Only 1 item left in the cache. The cleanup timer runs continuously and successfully evicts expired items.
Testing
go test -v -run TestCache_CleanupTimer ./internal/memorycachewhich passes.go test -bench=BenchmarkCache_MemoryLeak -benchmem -run=^$ ./internal/memorycachewhich passes and shows stable memory usage.make build.