erigontech / erigontech/erigon
cl: lru.NewWithTTL leaks an unstoppable cleanup goroutine per cache
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
`cl/phase1/core/state/lru.NewWithTTL` wraps `hashicorp/golang-lru/v2/expirable`, whose
`NewLRU` starts a `deleteExpired` goroutine and never closes its `done` channel. `Close()` is
commented out in the dependency (`expirable/expirable_lru.go:277`) with a note that the
functionality will only arrive in a version later than v2. Every TTL cache we construct
therefore keeps a goroutine for the lifetime of the process, with a ticker at `ttl/100`.
Nothing leaks in production today: all nine call sites are process-level singletons.
```
cl/aggregation/pool_impl.go:67
cl/validator/attestation_producer/attestation_producer.go:54
cl/sentinel/peers/peers_pool.go:76,77
cl/persistence/state/historical_states_reader/historical_states_reader.go:60
cl/p2p/p2p.go:131
cl/phase1/network/services/attestation_service.go:109
cl/phase1/network/services/execution_payload_bid_service.go:122
cl/rpc/peer_selection.go:60
```
The risk is that the constraint is invisible: any future per-request or per-peer construction
leaks a goroutine and a ticker with nothing in the type to stop it. It already showed up in
tests — see #23847, where a pool per test left eight goroutines behind per suite repetition.
Worth knowing for whoever picks this up: `expirable`'s `Get` already checks `ExpiresAt` and
returns a miss for an expired entry, so the background goroutine only reclaims memory. TTL read
semantics do not depend on it. That means we can own the sweep and give `CacheWithTTL` a real
`Close()`, either by driving expiry ourselves over a plain LRU or by constructing the upstream
cache with `noEvictionTTL` and sweeping on our own cancellable goroutine.
This touches consensus-adjacent caches (the aggregation pool, the attestation seen-cache), so it
wants care and its own tests rather than being folded into an unrelated PR.
Contributor guide
Assessment
This issue has not been assessed yet.