erigontech / erigontech/erigon

cl: lru.NewWithTTL leaks an unstoppable cleanup goroutine per cache

Open
#23,848 0 comments 0 reactions 1 assignee Claimed by @lystopad View on GitHub
Caplin type:bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.