From a29a42d8b718f6770537cd3cf9f7805ae10ca916 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 13:50:25 +0200 Subject: [PATCH 01/14] execution/cache: fence GenericCache generation swaps and make size accounting exact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split from #22159 (StateCache review findings #22120). - maybeGrow copies and swaps the generation with every put stripe held, so a striped put can no longer land in a retired generation (older value resurfacing as live) and a conditional put can no longer fill a mid-resize gap with a stale value. Grow detection moves inside the stripe, the grow itself outside it; the triggering insert (and racers until the swap) evict at the pre-grow cap — a bounded transient. - Clear performs its counter reset, coherence re-init and generation swap under the same fence. - Delete and the lazy stale-drop run under the key's put stripe, and currentSize is subtracted solely via the OnEvict callback: freelru picks eviction victims per shard (hash bits 16+), which the stripes (bits 0-7) don't cover, so any subtraction computed outside the callback double-counts against a racing capacity eviction. - One Debug line per grow (caps, copied count, alloc/fenced split) so the once-per-lifetime writer stall is self-explaining in logs. - growLRU documents its intentionally unfenced swap contract: safe only for content-addressed layers; counters approximate across grows. --- execution/cache/cache_test.go | 69 ++++++- .../cache/code_cache_concurrency_test.go | 3 +- execution/cache/generic_cache.go | 134 ++++++++++---- .../cache/generic_cache_concurrency_test.go | 169 ++++++++++++++++++ execution/cache/grow_lru.go | 10 +- 5 files changed, 350 insertions(+), 35 deletions(-) diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index 3051a4f365c..8af3f077737 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -866,11 +866,10 @@ func TestCodeCache_PutWithCodeHashIfAbsent(t *testing.T) { // clobber it — the prefetch-vs-flush staleness this cache guards against. func TestDomainCache_PutIfAbsentAtomicWithPut(t *testing.T) { c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) - addr := makeAddr(1) fresh := []byte("fresh") stale := []byte("stale") for round := 0; round < 20000; round++ { - c.Delete(addr) + addr := makeAddr(round) var wg sync.WaitGroup wg.Add(2) go func() { defer wg.Done(); c.Put(addr, fresh, 20) }() @@ -881,3 +880,69 @@ func TestDomainCache_PutIfAbsentAtomicWithPut(t *testing.T) { require.Equal(t, fresh, v, "round %d: PutIfAbsent raced past a concurrent Put", round) } } + +// A Delete racing an update-in-place put must not double-subtract the +// displaced entry's size: freelru's OnEvict subtracts it for the Remove, and +// put's update delta subtracts it again unless the two writers share the +// key's stripe. +func TestDomainCache_DeleteAtomicWithPut_NoSizeDrift(t *testing.T) { + c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + addr := makeAddr(1) + v1 := []byte("value-one") + v2 := []byte("value-two") + for round := 0; round < 20000; round++ { + c.Put(addr, v1, 10) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(addr, v2, 20) }() + go func() { defer wg.Done(); c.Delete(addr) }() + wg.Wait() + c.Delete(addr) + require.Zero(t, c.SizeBytes(), "round %d: size accounting drifted", round) + } +} + +// The lazy stale-drop inside GetWithTxNum removes entries; an unstriped +// Remove racing put's read-modify-write double-subtracts the displaced +// entry's size. Exactly one live entry remains after every round, so drift +// shows as a size mismatch. +func TestDomainCache_StaleDropAtomicWithPut_NoSizeDrift(t *testing.T) { + c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + addr := makeAddr(1) + v1 := []byte("value-one") + v2 := []byte("value-two") + wantSize := int64(len(addr) + len(v1) + 24) + for round := 0; round < 20000; round++ { + c.Put(addr, v1, 10) + c.Unwind(5) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(addr, v2, 20) }() + go func() { defer wg.Done(); c.Get(addr) }() + wg.Wait() + require.Equal(t, wantSize, c.SizeBytes(), "round %d: size accounting drifted", round) + } +} + +// A Clear racing a put must not leave phantom bytes: unless Clear excludes +// writers via the put stripes, a put that loaded the retiring generation +// lands its entry where no reader sees it and adds the entry's size after +// Clear zeroed the counter — inflating SizeBytes for an invisible entry. +func TestDomainCache_ClearAtomicWithPut_NoSizeDrift(t *testing.T) { + c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + addr := makeAddr(1) + v1 := []byte("value-one") + entrySize := int64(len(addr) + len(v1) + 24) + for round := 0; round < 20000; round++ { + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(addr, v1, 10) }() + go func() { defer wg.Done(); c.Clear() }() + wg.Wait() + wantSize := int64(0) + if _, ok := c.Get(addr); ok { + wantSize = entrySize + } + require.Equal(t, wantSize, c.SizeBytes(), "round %d: size accounting drifted", round) + } +} diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 109a01874fa..fb2a63fc12d 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -17,6 +17,7 @@ package cache import ( + "encoding/binary" "sync" "testing" @@ -130,7 +131,7 @@ func TestCodeCache_PutIfAbsentAtomicWithPut(t *testing.T) { fresh := []byte{0xaa, 1, 2, 3} stale := []byte{0xbb, 4, 5, 6} for round := 0; round < 20000; round++ { - cc.Delete(addr) + binary.BigEndian.PutUint64(addr[1:], uint64(round)) var wg sync.WaitGroup wg.Add(2) go func() { defer wg.Done(); cc.Put(addr, fresh, 20) }() diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 4d261f686f7..e3545978c72 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -20,6 +20,7 @@ import ( "bytes" "sync" "sync/atomic" + "time" "github.com/c2h5oh/datasize" "github.com/elastic/go-freelru" @@ -59,9 +60,10 @@ type entry[T any] struct { // GenericCache is a sharded, LRU-evicting bounded cache for key-value // data. Eviction mode is fixed at construction (see policy.go). type GenericCache[T any] struct { - // data is the sharded LRU, replaced wholesale on a jump-grow. Load it once per - // operation; a write racing a resize may land in the LRU about to be replaced - // and be dropped — a benign miss (the value is re-read from the domain). + // data is the sharded LRU, replaced wholesale only with every put stripe + // held — on a jump-grow (fully copied generation) and on Clear (fresh + // empty one) — so no write lands in a retired generation and no reader + // sees a partial copy (see maybeGrow, Clear). data atomic.Pointer[freelru.ShardedLRU[uint64, entry[T]]] capacityB datasize.ByteSize mode Mode @@ -172,7 +174,11 @@ func newGenericCacheEntries[T any](capacityBytes datasize.ByteSize, capacityEntr } // newShards builds a sharded LRU of the given capacity with this cache's evict -// callback wired, so currentSize follows capacity-driven eviction and Remove. +// callback wired. The callback is the sole subtractor of currentSize — every +// removal (capacity eviction, Remove) accounts through it. Freelru picks +// eviction victims per shard (hash bits 16+), which the put stripes (bits 0-7) +// don't cover, so any subtraction computed outside the callback races a +// cross-stripe eviction of the same entry. func (c *GenericCache[T]) newShards(capacity uint32) *freelru.ShardedLRU[uint64, entry[T]] { lru, err := freelru.NewSharded[uint64, entry[T]](capacity, u64identity) if err != nil { @@ -187,7 +193,15 @@ func (c *GenericCache[T]) newShards(capacity uint32) *freelru.ShardedLRU[uint64, // maybeGrow jump-resizes the LRU one step larger when it is full, the ceiling // hasn't been reached, and the shared envelope can fund the step. Otherwise the -// LRU keeps its size and freelru evicts within it. Called with no lock held. +// LRU keeps its size and freelru evicts within it. Must not be called with a +// stripe held (it takes them all). +// +// The copy runs with every put stripe held: writers (and the striped +// stale-drop) are excluded, so no write can land in the generation being +// retired and a conditional put never sees a mid-resize gap it could fill +// with a stale value; readers stay on the retiring generation until the swap +// and never miss. Grows are a handful of steps per cache lifetime, so the +// writer stall is a bounded one-off. func (c *GenericCache[T]) maybeGrow() { c.resizeMu.Lock() defer c.resizeMu.Unlock() @@ -202,15 +216,27 @@ func (c *GenericCache[T]) maybeGrow() { if !cachebudget.Global.Reserve(delta) { return } - next := c.newShards(newCap) + start := time.Now() + next := c.newShards(newCap) // allocate before excluding writers + fenceStart := time.Now() + for i := range c.putStripes { + c.putStripes[i].Lock() + } + copied := 0 for _, k := range old.Keys() { if v, ok := old.Get(k); ok { next.Add(k, v) + copied++ } } c.data.Store(next) c.curCap.Store(newCap) + for i := range c.putStripes { + c.putStripes[i].Unlock() + } c.reservedBytes += delta + log.Debug("[cache] jump-grow", "fromSlots", curCap, "toSlots", newCap, "copied", copied, + "alloc", fenceStart.Sub(start), "fenced", time.Since(fenceStart)) } // DomainCache wraps GenericCache[[]byte] to implement the Cache interface. @@ -271,7 +297,7 @@ func (c *GenericCache[T]) GetWithTxNum(key []byte) (T, uint64, bool) { // tx — and must be dropped; >= not > (the surviving block's last txNum is // floor-1, so this never drops a live entry). if c.coh.IsStale(e.txNum, e.epoch) { - lru.Remove(h) + c.dropStale(h, key) c.staleEvicted.Add(1) c.misses.Add(1) var zero T @@ -296,6 +322,17 @@ func (c *GenericCache[T]) PutIfAbsent(key []byte, value T, txNum uint64) { } func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) { + if c.putLocked(key, value, txNum, overwrite) { + // Grow outside the stripe — maybeGrow takes every stripe. + c.maybeGrow() + } +} + +// putLocked performs the write under the key's stripe and reports whether the +// insert landed in a full LRU with ceiling headroom, i.e. the caller should +// grow. Detection stays on the insert path — Len locks every shard, too costly +// per warm update. +func (c *GenericCache[T]) putLocked(key []byte, value T, txNum uint64, overwrite bool) bool { h := maphash.Hash(key) valBytes := c.sizeFunc(value) newSize := len(key) + valBytes + 24 @@ -308,16 +345,17 @@ func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) lru := c.data.Load() existing, hasExisting := lru.Get(h) - // Existing key — update in place. Reuse the stored key buffer to - // avoid an extra allocation; the freshly-decoded value replaces the - // old one. + // Existing key — update by remove-then-add (see newShards for why a size + // delta would be wrong). Reuse the stored key buffer to avoid an extra + // allocation; the freshly-decoded value replaces the old one. if hasExisting && bytes.Equal(existing.key, key) { if !overwrite && !c.coh.IsStale(existing.txNum, existing.epoch) { - return + return false } + c.removeLocked(lru, h) lru.Add(h, entry[T]{key: existing.key, val: value, size: newSize, txNum: txNum, epoch: ep}) - c.currentSize.Add(int64(newSize - existing.size)) - return + c.currentSize.Add(int64(newSize)) + return false } if c.mode == ModeNoOp { @@ -325,16 +363,16 @@ func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) // entry-count cap, which ModeNoOp ("drop new keys when full") must not do. if c.currentSize.Load()+int64(newSize) > int64(c.capacityB) || lru.Len() >= int(c.maxCap) { c.dropped.Add(1) - return + return false } } - // ModeEvictLRU: grow toward the ceiling before inserting into a full LRU, so a - // busy cache expands into its budget rather than evicting at the start size. - if curCap := c.curCap.Load(); c.mode != ModeNoOp && curCap < c.maxCap && lru.Len() >= int(curCap) { - c.maybeGrow() - lru = c.data.Load() - } + curCap := c.curCap.Load() + // The insert lands before the grow (which must run outside the stripe), so + // it and any racers until the swap evict at the pre-grow cap — a transient + // bounded by the grow window, not a regression of the grow-first ordering. + needGrow := c.mode != ModeNoOp && curCap < c.maxCap && lru.Len() >= int(curCap) + // In ModeEvictLRU the byte budget is enforced through the entry-count cap, // not a separate currentSize check: capacityEntries is derived from // capacityB (capacityB/avgBytesPerEntry, see NewGenericCache / @@ -348,24 +386,51 @@ func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) // balcache.go / db/state/cache.go accept. // hasExisting here means a 64-bit maphash collision (different key, same - // hash): freelru.Add replaces the colliding entry in place WITHOUT firing - // OnEvict, so subtract the displaced size now — otherwise currentSize drifts - // up by it permanently. + // hash): remove the colliding entry first so OnEvict accounts for it — + // freelru.Add would replace it in place without firing OnEvict. if hasExisting { - c.currentSize.Add(-int64(existing.size)) + c.removeLocked(lru, h) } keyCopy := common.Copy(key) lru.Add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) c.currentSize.Add(int64(newSize)) c.inserts.Add(1) + return needGrow } -// Delete removes the data for the given key. +// removeLocked removes h under the caller-held stripe, deferring the size +// subtraction to OnEvict (see newShards). The evictions metric is compensated: +// an intentional removal is not a capacity eviction. +func (c *GenericCache[T]) removeLocked(lru *freelru.ShardedLRU[uint64, entry[T]], h uint64) { + if lru.Remove(h) { + c.evictions.Add(^uint64(0)) + } +} + +// Delete removes the data for the given key. Runs under the key's put stripe +// so the check-then-remove is atomic against same-key puts and excluded from +// generation swaps (maybeGrow, Clear), which fence via the stripes. func (c *GenericCache[T]) Delete(key []byte) { h := maphash.Hash(key) + mu := &c.putStripes[h&(putStripeCount-1)] + mu.Lock() + defer mu.Unlock() lru := c.data.Load() if existing, ok := lru.Get(h); ok && bytes.Equal(existing.key, key) { - lru.Remove(h) + c.removeLocked(lru, h) + } +} + +// dropStale removes key's entry under its put stripe: the re-check keeps an +// entry a concurrent put revived, and the stripe keeps the removal out of +// generation swaps. +func (c *GenericCache[T]) dropStale(h uint64, key []byte) { + mu := &c.putStripes[h&(putStripeCount-1)] + mu.Lock() + defer mu.Unlock() + lru := c.data.Load() + if e, ok := lru.Get(h); ok && bytes.Equal(e.key, key) && c.coh.IsStale(e.txNum, e.epoch) { + c.removeLocked(lru, h) } } @@ -373,10 +438,10 @@ func (c *GenericCache[T]) Delete(key []byte) { // unwindFloor) coherence pair: with no entries left, no stale (txNum, epoch) // can survive, so a fresh floor keeps subsequent Puts at the live epoch // serviceable. Mirrors CodeCache.Clear (which already did this — the two had -// drifted). +// drifted). The counter reset and the generation swap run with every put +// stripe held — like maybeGrow's — so a racing put can neither land in the +// retired generation nor add its size after the reset. func (c *GenericCache[T]) Clear() { - c.currentSize.Store(0) - c.coh.Init() // Shrink back to the start size and return the grown budget to the envelope, // keeping the cache adaptive across fork-validation/reset (it regrows on // demand). A no-op Purge would leave the grown slot array resident. @@ -386,8 +451,17 @@ func (c *GenericCache[T]) Clear() { cachebudget.Global.Release(c.reservedBytes - int64(c.startCap)*c.avgEntryBytes) c.reservedBytes = int64(c.startCap) * c.avgEntryBytes } + next := c.newShards(c.startCap) // allocate before excluding writers + for i := range c.putStripes { + c.putStripes[i].Lock() + } + c.currentSize.Store(0) + c.coh.Init() c.curCap.Store(c.startCap) - c.data.Store(c.newShards(c.startCap)) + c.data.Store(next) + for i := range c.putStripes { + c.putStripes[i].Unlock() + } } // Close returns this cache's envelope reservation so later caches can grow into diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index 6e27e997385..e3e7a9d6d35 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -19,9 +19,13 @@ package cache import ( "encoding/binary" "sync" + "sync/atomic" "testing" "github.com/c2h5oh/datasize" + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/maphash" ) // TestGenericCache_ConcurrentPutAcrossGrow guards the jump-grow data race: @@ -51,3 +55,168 @@ func TestGenericCache_ConcurrentPutAcrossGrow(t *testing.T) { } wg.Wait() } + +// A same-key put serialized by its stripe must never be undone by a grow: with +// copy-then-swap migration, a writer that loaded the old generation before the +// swap landed its write in the abandoned generation, and the migrated (older) +// value resurfaced as live — a stale serve, not a benign miss. The writer +// self-verifies each put and a reader checks the hot key's monotonically +// increasing value never goes backward. +func TestGenericCache_PutNotLostAcrossGrow(t *testing.T) { + value := func(n uint64) []byte { + b := make([]byte, 8) + binary.BigEndian.PutUint64(b, n) + return b + } + for round := 0; round < 50; round++ { + c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + hot := []byte("hot-key") + c.Put(hot, value(0), 1) + + stop := make(chan struct{}) + var regressed atomic.Bool + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for n := uint64(1); ; n++ { + select { + case <-stop: + return + default: + } + c.Put(hot, value(n), n) + if v, ok := c.Get(hot); ok { + if got := binary.BigEndian.Uint64(v); got < n { + regressed.Store(true) + return + } + } + } + }() + go func() { + defer wg.Done() + last := uint64(0) + for { + select { + case <-stop: + return + default: + } + if v, ok := c.Get(hot); ok { + if n := binary.BigEndian.Uint64(v); n < last { + regressed.Store(true) + return + } else { + last = n + } + } + } + }() + + // Cross the grow threshold so maybeGrow swaps the generation while the + // hot-key writer runs. + key := make([]byte, 8) + for i := 0; i < 3*genericCacheStartCapacity; i++ { + binary.BigEndian.PutUint64(key, uint64(1+i)) + c.Put(key, []byte{1}, 1) + } + + close(stop) + wg.Wait() + c.Close() + require.False(t, regressed.Load(), "round %d: a striped put was lost across a grow (older value resurfaced)", round) + } +} + +// A conditional put must keep deferring to a live entry across a grow: if the +// resize ever publishes a generation the entry hasn't reached yet, a +// PutIfAbsent arriving in that gap finds the key absent and inserts its +// (stale) value — the writer class the if-absent semantics exist to close. +// The prober watches for the generation swap and bursts conditional puts the +// moment it lands, mimicking a fill thread that starts a put mid-resize. +// +// The cache is seeded below any capacity pressure with the hot key inserted +// last — the LRU victim is always an older seed key, so the hot key cannot be +// evicted and a stale value at the end can only have come through a resize +// gap. The grow is forced by lowering curCap: reaching Len >= startCap +// organically needs every freelru shard full, which would make the hot key +// evictable and the signal ambiguous. +func TestGenericCache_PutIfAbsentDefersAcrossGrow(t *testing.T) { + fresh := []byte("fresh-value") + stale := []byte("stale-value") + for round := 0; round < 50; round++ { + c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + key := make([]byte, 8) + for i := 0; i < 512; i++ { + binary.BigEndian.PutUint64(key, uint64(1+i)) + c.Put(key, []byte{1}, 1) + } + hot := []byte("hot-key") + c.Put(hot, fresh, 10) + + stop := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + before := c.data.Load() + for { + select { + case <-stop: + return + default: + } + if c.data.Load() != before { + for j := 0; j < 4096; j++ { + c.PutIfAbsent(hot, stale, 5) + } + return + } + } + }() + + c.curCap.Store(uint32(c.Len())) + binary.BigEndian.PutUint64(key, 0) + c.Put(key, []byte{1}, 1) // insert at the lowered cap → triggers the grow + + close(stop) + wg.Wait() + v, ok := c.Get(hot) + require.True(t, ok, "round %d: hot key missing", round) + require.Equal(t, fresh, v, "round %d: PutIfAbsent bypassed the live entry across a grow", round) + c.Close() + } +} + +// A capacity eviction is a size-subtracting writer the put stripes cannot +// serialize: freelru picks its victim per shard (hash bits 16+), so an insert +// on one stripe can evict a key whose own update — on another stripe — is +// between its Get and Add; delta accounting against the pre-eviction size then +// double-subtracts. Capacity 1 collapses freelru to a single shard, making any +// two keys same-shard; the keys are chosen to differ in their put stripe. Each +// hit leaks negative size; drift accumulates and shows after the settle +// deletes. +func TestGenericCache_CapacityEvictionAtomicWithPut_NoSizeDrift(t *testing.T) { + c := newGenericCacheEntries(1*datasize.MB, 1, func(v []byte) int { return len(v) }, ModeEvictLRU) + a := makeAddr(1) + var b []byte + for i := 2; ; i++ { + b = makeAddr(i) + if maphash.Hash(a)&(putStripeCount-1) != maphash.Hash(b)&(putStripeCount-1) { + break + } + } + v := []byte("value-one") + for round := 0; round < 100000; round++ { + c.Put(b, v, 10) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(a, v, 10) }() // insert → evicts b (cap 1) + go func() { defer wg.Done(); c.Put(b, v, 20) }() // same-key update path + wg.Wait() + } + c.Delete(a) + c.Delete(b) + require.Zero(t, c.SizeBytes(), "capacity eviction raced the update-path delta") +} diff --git a/execution/cache/grow_lru.go b/execution/cache/grow_lru.go index b2455f5f5c1..b0e3725028d 100644 --- a/execution/cache/grow_lru.go +++ b/execution/cache/grow_lru.go @@ -33,8 +33,14 @@ import ( // pre-commits its full configured capacity — the same demand-growth the state // caches use — reused across the CodeCache's content and size layers. // -// A write racing a resize may land in the LRU about to be replaced and be -// dropped; that is a benign cache miss (the value is re-read from the DB). +// Generation swaps (maybeGrow, Purge) are not fenced against writers — safe +// only for content-addressed layers, where a key's payload never changes: a +// write lost in a retired generation is a benign miss, and an entry whose +// removal a racing copy undid serves correct bytes until its stale stamp +// drops it on the next read. Do not reuse for mutable-per-key values — those +// need GenericCache's fenced swap. The onEvict-maintained counters are +// approximate across grow windows (a lost write is counted but never +// evicted; a raced removal can subtract twice). type growLRU[V any] struct { cur atomic.Pointer[freelru.ShardedLRU[uint64, V]] onEvict func(uint64, V) From 81150303477b60811ad45a65ed338c4f188199c6 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 15:23:40 +0200 Subject: [PATCH 02/14] execution/cache: sample coherence epoch under the put stripe putLocked read coh.Epoch() before acquiring the key's stripe, so a put losing the stripe race to Clear (whose coh.Init resets the epoch counter) stamped a pre-Clear epoch onto an entry landing in the post-Clear generation. Once a later unwind re-reached that epoch value, the entry aliased the live epoch and survived IsStale despite its txNum being at or above the floor - serving dead-fork state. Move the epoch sample inside the stripe, next to the generation load the fence already synchronizes. The new test parks Clear on the key's stripe pre-Init and the put behind it (starvation-mode FIFO hands the stripe to Clear first); it failed deterministically before the fix. --- execution/cache/generic_cache.go | 5 ++- .../cache/generic_cache_concurrency_test.go | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index e3545978c72..8f4086015ce 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -336,12 +336,15 @@ func (c *GenericCache[T]) putLocked(key []byte, value T, txNum uint64, overwrite h := maphash.Hash(key) valBytes := c.sizeFunc(value) newSize := len(key) + valBytes + 24 - ep := c.coh.Epoch() mu := &c.putStripes[h&(putStripeCount-1)] mu.Lock() defer mu.Unlock() + // Sample the epoch under the stripe: Clear resets the epoch counter inside + // the fence, so a stamp read outside could alias a future epoch and let a + // dead-fork entry survive a later unwind. + ep := c.coh.Epoch() lru := c.data.Load() existing, hasExisting := lru.Get(h) diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index e3e7a9d6d35..0d3f2abf3d0 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -21,6 +21,7 @@ import ( "sync" "sync/atomic" "testing" + "time" "github.com/c2h5oh/datasize" "github.com/stretchr/testify/require" @@ -220,3 +221,38 @@ func TestGenericCache_CapacityEvictionAtomicWithPut_NoSizeDrift(t *testing.T) { c.Delete(b) require.Zero(t, c.SizeBytes(), "capacity eviction raced the update-path delta") } + +// A put samples the coherence epoch and then contends for its stripe; a Clear +// that wins the stripe first resets the epoch counter, so the put would stamp +// a pre-Clear epoch onto an entry landing in the post-Clear generation. Once +// a later unwind re-reaches that epoch value, the entry aliases the live +// epoch and serves dead-fork state despite its txNum being at or above the +// floor. +// +// The test holds the key's stripe to park Clear on it (before the reset, +// which runs inside the fence) and then the put behind it; waits beyond 1ms +// put the mutex in starvation mode, so unlocking hands the stripe FIFO to +// Clear first. +func TestGenericCache_ClearRacingPut_EpochAlias(t *testing.T) { + c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + defer c.Close() + c.Unwind(300) // epoch 0 -> 1 + + key := []byte("epoch-alias-key") + mu := &c.putStripes[maphash.Hash(key)&(putStripeCount-1)] + mu.Lock() + + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Clear() }() + time.Sleep(5 * time.Millisecond) + go func() { defer wg.Done(); c.Put(key, []byte("dead-fork-value"), 200) }() + time.Sleep(5 * time.Millisecond) + mu.Unlock() + wg.Wait() + + c.Unwind(150) // epoch 0 -> 1 again, floor 150 + + _, ok := c.Get(key) + require.False(t, ok, "entry at txNum 200 outlived an unwind to 150: its pre-Clear epoch stamp aliases the live epoch") +} From d61ddba3185e6ba6d52c1fe9bbe54f09a309a6ee Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 15:31:15 +0200 Subject: [PATCH 03/14] execution/cache: make the PutIfAbsent grow test exercise the lost-insert window The prober only burst conditional puts after observing the generation swap, but every design publishes the new generation after the copy completes, so the hot key was always present and the test could not fail - with or without the fence. Replace the body: a writer hammers puts of brand-new keys while an insert at a lowered curCap triggers the grow, then every key that straddled the swap is probed with a stale PutIfAbsent. Unfenced, a new-key put landing in the retiring generation after the Keys() snapshot is lost on the swap and the conditional put installs the stale value as live - the test now fails in the first rounds when the fence is removed (20/20 runs). The candidate presence asserts also give a stronger migration-completeness check than the old single hot key. --- .../cache/generic_cache_concurrency_test.go | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index 0d3f2abf3d0..919652e0062 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -130,62 +130,68 @@ func TestGenericCache_PutNotLostAcrossGrow(t *testing.T) { } } -// A conditional put must keep deferring to a live entry across a grow: if the -// resize ever publishes a generation the entry hasn't reached yet, a -// PutIfAbsent arriving in that gap finds the key absent and inserts its -// (stale) value — the writer class the if-absent semantics exist to close. -// The prober watches for the generation swap and bursts conditional puts the -// moment it lands, mimicking a fill thread that starts a put mid-resize. +// A conditional put must keep deferring to a live entry across a grow. The +// vulnerable writer class: a put of a brand-new key that lands in the +// retiring generation after the copy snapshotted Keys() is lost on the swap, +// and a follow-up PutIfAbsent finds the key absent and installs its stale +// value as live. With the fence the put either lands pre-fence (and is +// migrated — Keys() is taken with every stripe held) or lands in the new +// generation; either way the conditional put defers. // -// The cache is seeded below any capacity pressure with the hot key inserted -// last — the LRU victim is always an older seed key, so the hot key cannot be -// evicted and a stale value at the end can only have come through a resize -// gap. The grow is forced by lowering curCap: reaching Len >= startCap -// organically needs every freelru shard full, which would make the hot key -// evictable and the signal ambiguous. +// A writer hammers fresh keys while the grow swaps generations; every key +// that straddled the swap is then probed with a stale conditional put. The +// grow is forced by lowering curCap over a lightly-populated cache, so +// capacity eviction cannot explain a missing key. func TestGenericCache_PutIfAbsentDefersAcrossGrow(t *testing.T) { fresh := []byte("fresh-value") stale := []byte("stale-value") - for round := 0; round < 50; round++ { + for round := 0; round < 100; round++ { c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) key := make([]byte, 8) - for i := 0; i < 512; i++ { + for i := 0; i < 256; i++ { binary.BigEndian.PutUint64(key, uint64(1+i)) c.Put(key, []byte{1}, 1) } - hot := []byte("hot-key") - c.Put(hot, fresh, 10) + before := c.data.Load() + c.curCap.Store(uint32(c.Len())) + var candidates [][]byte stop := make(chan struct{}) var wg sync.WaitGroup wg.Add(1) go func() { defer wg.Done() - before := c.data.Load() - for { + for j := 0; ; j++ { select { case <-stop: return default: } + k := make([]byte, 9) + k[0] = 0xfe + binary.BigEndian.PutUint64(k[1:], uint64(j)) + c.Put(k, fresh, 10) + candidates = append(candidates, k) if c.data.Load() != before { - for j := 0; j < 4096; j++ { - c.PutIfAbsent(hot, stale, 5) - } return } } }() - c.curCap.Store(uint32(c.Len())) binary.BigEndian.PutUint64(key, 0) c.Put(key, []byte{1}, 1) // insert at the lowered cap → triggers the grow - close(stop) wg.Wait() - v, ok := c.Get(hot) - require.True(t, ok, "round %d: hot key missing", round) - require.Equal(t, fresh, v, "round %d: PutIfAbsent bypassed the live entry across a grow", round) + + for _, k := range candidates { + c.PutIfAbsent(k, stale, 5) + } + for i, k := range candidates { + v, ok := c.Get(k) + require.True(t, ok, "round %d: candidate %d missing", round, i) + require.Equal(t, fresh, v, + "round %d: candidate %d: PutIfAbsent installed a stale value over a put lost in the retiring generation", round, i) + } c.Close() } } From 3577e23e58c481bc6020dceab937999c758da536 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 15:45:48 +0200 Subject: [PATCH 04/14] execution/cache: net intentional removals out of evictions at print time removeLocked compensated the OnEvict increment by decrementing the evictions counter; a PrintStatsAndReset Swap(0) landing between the two zeroed the counter first, so the decrement underflowed it to ~1.8e19 for the next interval. The remove-then-add update path fires this on every warm update, so the window is open a measurable fraction of wall time on a busy node. Count intentional removals in their own counter instead - both counters are increment-only between resets, so no interleaving with a reset can underflow either - and net them at print time. Removals are snapshotted before evictions: OnEvict bumps evictions before the removal is counted, so this order keeps every captured removal paired with a captured eviction; the clamp absorbs a removal deferred to the next interval. Reported semantics are unchanged: evictions still means capacity evictions only. The new test plays the stats reset against a Delete hammer; it underflowed within milliseconds before the fix. --- execution/cache/generic_cache.go | 18 ++++++++-- .../cache/generic_cache_concurrency_test.go | 33 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 8f4086015ce..526e5183140 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -104,6 +104,7 @@ type GenericCache[T any] struct { misses atomic.Uint64 inserts atomic.Uint64 evictions atomic.Uint64 + removals atomic.Uint64 // intentional removals, netted out of evictions at print time dropped atomic.Uint64 staleEvicted atomic.Uint64 // entries dropped lazily on read after an unwind @@ -402,11 +403,12 @@ func (c *GenericCache[T]) putLocked(key []byte, value T, txNum uint64, overwrite } // removeLocked removes h under the caller-held stripe, deferring the size -// subtraction to OnEvict (see newShards). The evictions metric is compensated: -// an intentional removal is not a capacity eviction. +// subtraction to OnEvict (see newShards). The removal is counted separately so +// PrintStatsAndReset can net intentional removals out of the evictions metric — +// decrementing evictions here would underflow across a concurrent stats reset. func (c *GenericCache[T]) removeLocked(lru *freelru.ShardedLRU[uint64, entry[T]], h uint64) { if lru.Remove(h) { - c.evictions.Add(^uint64(0)) + c.removals.Add(1) } } @@ -507,7 +509,17 @@ func (c *GenericCache[T]) PrintStatsAndReset(name string) { hits := c.hits.Swap(0) misses := c.misses.Swap(0) inserts := c.inserts.Swap(0) + // Snapshot removals before evictions: OnEvict bumps evictions before the + // removal is counted, so this order keeps every captured removal paired with + // a captured eviction. The clamp absorbs a removal deferred to the next + // interval. + removals := c.removals.Swap(0) evictions := c.evictions.Swap(0) + if evictions >= removals { + evictions -= removals + } else { + evictions = 0 + } dropped := c.dropped.Swap(0) staleEvicted := c.staleEvicted.Swap(0) total := hits + misses diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index 919652e0062..a4b21341d98 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -262,3 +262,36 @@ func TestGenericCache_ClearRacingPut_EpochAlias(t *testing.T) { _, ok := c.Get(key) require.False(t, ok, "entry at txNum 200 outlived an unwind to 150: its pre-Clear epoch stamp aliases the live epoch") } + +// Intentional removals are netted out of the evictions metric; doing that by +// decrementing the shared counter races PrintStatsAndReset's Swap(0) — the +// swap can land between OnEvict's increment and the decrement, underflowing +// the counter to ~1.8e19 for the next interval. The sampler plays the stats +// reset against a Delete hammer and must never observe an absurd value. +func TestGenericCache_StatsResetAtomicWithDelete_NoEvictionsUnderflow(t *testing.T) { + c := NewGenericCache[[]byte](1*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + defer c.Close() + key := []byte("metrics-key") + stop := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-stop: + return + default: + } + c.Put(key, []byte{1}, 1) + c.Delete(key) + } + }() + defer wg.Wait() + defer close(stop) + for i := 0; i < 1_000_000; i++ { + if ev := c.evictions.Swap(0); ev > 1<<40 { + t.Fatalf("stats reset racing an intentional removal underflowed the evictions counter: %d", ev) + } + } +} From 213ed94fe390fb9847fc2345c63b5060d8a956ba Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 15:57:36 +0200 Subject: [PATCH 05/14] execution/cache: rename putLocked to putStriped The -Locked suffix conventionally means the caller holds the lock, which is removeLocked's contract; putStriped acquires the stripe itself. --- execution/cache/generic_cache.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 526e5183140..b39d95d0e83 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -323,17 +323,17 @@ func (c *GenericCache[T]) PutIfAbsent(key []byte, value T, txNum uint64) { } func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) { - if c.putLocked(key, value, txNum, overwrite) { + if c.putStriped(key, value, txNum, overwrite) { // Grow outside the stripe — maybeGrow takes every stripe. c.maybeGrow() } } -// putLocked performs the write under the key's stripe and reports whether the +// putStriped performs the write under the key's stripe and reports whether the // insert landed in a full LRU with ceiling headroom, i.e. the caller should // grow. Detection stays on the insert path — Len locks every shard, too costly // per warm update. -func (c *GenericCache[T]) putLocked(key []byte, value T, txNum uint64, overwrite bool) bool { +func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrite bool) bool { h := maphash.Hash(key) valBytes := c.sizeFunc(value) newSize := len(key) + valBytes + 24 From d7598d7cd00902b81ee56fe29db56f765f51f815 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 16:20:23 +0200 Subject: [PATCH 06/14] execution/cache: clarify that staleEvicted counts stale reads, not removals dropStale's stripe-held re-check keeps an entry a racing put revived, so the counter tracks detections; the old comment implied actual drops. --- execution/cache/generic_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index b39d95d0e83..e17e14733da 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -106,7 +106,7 @@ type GenericCache[T any] struct { evictions atomic.Uint64 removals atomic.Uint64 // intentional removals, netted out of evictions at print time dropped atomic.Uint64 - staleEvicted atomic.Uint64 // entries dropped lazily on read after an unwind + staleEvicted atomic.Uint64 // stale entries detected on read after an unwind; dropped unless a racing put revived them sizeFunc func(T) int } From aa65992a1f0a08fb569509fe38ca4fa4ecc88c2e Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 17:02:25 +0200 Subject: [PATCH 07/14] execution/cache: count capacity evictions from Add's return OnEvict fires for intentional Removes too, so any scheme that routes them through the evictions counter - decrement compensation or netting against a removals counter at print time - races a concurrent stats reset: the swap straddles the paired updates and reports phantom evictions (or underflows). Count evictions where they happen instead, from freelru.Add's evicted return, and leave OnEvict with size accounting only. This deletes the removals counter and the print-time netting, and surfaces copy evictions in the jump-grow log line. --- execution/cache/generic_cache.go | 53 +++++++------------ .../cache/generic_cache_concurrency_test.go | 25 +++++---- 2 files changed, 34 insertions(+), 44 deletions(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index e17e14733da..5298eaa9779 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -103,8 +103,7 @@ type GenericCache[T any] struct { hits atomic.Uint64 misses atomic.Uint64 inserts atomic.Uint64 - evictions atomic.Uint64 - removals atomic.Uint64 // intentional removals, netted out of evictions at print time + evictions atomic.Uint64 // capacity evictions only, counted from Add's evicted return (see newShards) dropped atomic.Uint64 staleEvicted atomic.Uint64 // stale entries detected on read after an unwind; dropped unless a racing put revived them @@ -179,7 +178,9 @@ func newGenericCacheEntries[T any](capacityBytes datasize.ByteSize, capacityEntr // removal (capacity eviction, Remove) accounts through it. Freelru picks // eviction victims per shard (hash bits 16+), which the put stripes (bits 0-7) // don't cover, so any subtraction computed outside the callback races a -// cross-stripe eviction of the same entry. +// cross-stripe eviction of the same entry. The callback must not feed the +// evictions metric — it also fires for intentional Removes — so capacity +// evictions are counted from Add's evicted return at the call sites. func (c *GenericCache[T]) newShards(capacity uint32) *freelru.ShardedLRU[uint64, entry[T]] { lru, err := freelru.NewSharded[uint64, entry[T]](capacity, u64identity) if err != nil { @@ -187,7 +188,6 @@ func (c *GenericCache[T]) newShards(capacity uint32) *freelru.ShardedLRU[uint64, } lru.SetOnEvict(func(_ uint64, e entry[T]) { c.currentSize.Add(-int64(e.size)) - c.evictions.Add(1) }) return lru } @@ -223,10 +223,12 @@ func (c *GenericCache[T]) maybeGrow() { for i := range c.putStripes { c.putStripes[i].Lock() } - copied := 0 + copied, evicted := 0, 0 for _, k := range old.Keys() { if v, ok := old.Get(k); ok { - next.Add(k, v) + if next.Add(k, v) { + evicted++ + } copied++ } } @@ -235,8 +237,9 @@ func (c *GenericCache[T]) maybeGrow() { for i := range c.putStripes { c.putStripes[i].Unlock() } + c.evictions.Add(uint64(evicted)) c.reservedBytes += delta - log.Debug("[cache] jump-grow", "fromSlots", curCap, "toSlots", newCap, "copied", copied, + log.Debug("[cache] jump-grow", "fromSlots", curCap, "toSlots", newCap, "copied", copied, "evicted", evicted, "alloc", fenceStart.Sub(start), "fenced", time.Since(fenceStart)) } @@ -356,8 +359,10 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit if !overwrite && !c.coh.IsStale(existing.txNum, existing.epoch) { return false } - c.removeLocked(lru, h) - lru.Add(h, entry[T]{key: existing.key, val: value, size: newSize, txNum: txNum, epoch: ep}) + lru.Remove(h) + if lru.Add(h, entry[T]{key: existing.key, val: value, size: newSize, txNum: txNum, epoch: ep}) { + c.evictions.Add(1) + } c.currentSize.Add(int64(newSize)) return false } @@ -393,25 +398,17 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit // hash): remove the colliding entry first so OnEvict accounts for it — // freelru.Add would replace it in place without firing OnEvict. if hasExisting { - c.removeLocked(lru, h) + lru.Remove(h) } keyCopy := common.Copy(key) - lru.Add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) + if lru.Add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) { + c.evictions.Add(1) + } c.currentSize.Add(int64(newSize)) c.inserts.Add(1) return needGrow } -// removeLocked removes h under the caller-held stripe, deferring the size -// subtraction to OnEvict (see newShards). The removal is counted separately so -// PrintStatsAndReset can net intentional removals out of the evictions metric — -// decrementing evictions here would underflow across a concurrent stats reset. -func (c *GenericCache[T]) removeLocked(lru *freelru.ShardedLRU[uint64, entry[T]], h uint64) { - if lru.Remove(h) { - c.removals.Add(1) - } -} - // Delete removes the data for the given key. Runs under the key's put stripe // so the check-then-remove is atomic against same-key puts and excluded from // generation swaps (maybeGrow, Clear), which fence via the stripes. @@ -422,7 +419,7 @@ func (c *GenericCache[T]) Delete(key []byte) { defer mu.Unlock() lru := c.data.Load() if existing, ok := lru.Get(h); ok && bytes.Equal(existing.key, key) { - c.removeLocked(lru, h) + lru.Remove(h) } } @@ -435,7 +432,7 @@ func (c *GenericCache[T]) dropStale(h uint64, key []byte) { defer mu.Unlock() lru := c.data.Load() if e, ok := lru.Get(h); ok && bytes.Equal(e.key, key) && c.coh.IsStale(e.txNum, e.epoch) { - c.removeLocked(lru, h) + lru.Remove(h) } } @@ -509,17 +506,7 @@ func (c *GenericCache[T]) PrintStatsAndReset(name string) { hits := c.hits.Swap(0) misses := c.misses.Swap(0) inserts := c.inserts.Swap(0) - // Snapshot removals before evictions: OnEvict bumps evictions before the - // removal is counted, so this order keeps every captured removal paired with - // a captured eviction. The clamp absorbs a removal deferred to the next - // interval. - removals := c.removals.Swap(0) evictions := c.evictions.Swap(0) - if evictions >= removals { - evictions -= removals - } else { - evictions = 0 - } dropped := c.dropped.Swap(0) staleEvicted := c.staleEvicted.Swap(0) total := hits + misses diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index a4b21341d98..d882435bf62 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -263,12 +263,14 @@ func TestGenericCache_ClearRacingPut_EpochAlias(t *testing.T) { require.False(t, ok, "entry at txNum 200 outlived an unwind to 150: its pre-Clear epoch stamp aliases the live epoch") } -// Intentional removals are netted out of the evictions metric; doing that by -// decrementing the shared counter races PrintStatsAndReset's Swap(0) — the -// swap can land between OnEvict's increment and the decrement, underflowing -// the counter to ~1.8e19 for the next interval. The sampler plays the stats -// reset against a Delete hammer and must never observe an absurd value. -func TestGenericCache_StatsResetAtomicWithDelete_NoEvictionsUnderflow(t *testing.T) { +// The evictions counter must carry capacity evictions only. Routing +// intentional removals through it — decrement-compensated or netted against a +// removal counter at print time — races a concurrent stats reset: the swap +// straddles the paired updates, underflowing the counter or reporting phantom +// evictions that a later interval cannot retract. A Delete hammer with zero +// capacity pressure must therefore never surface a nonzero count, concurrent +// resets included. +func TestGenericCache_StatsResetAtomicWithDelete_NoPhantomEvictions(t *testing.T) { c := NewGenericCache[[]byte](1*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) defer c.Close() key := []byte("metrics-key") @@ -287,11 +289,12 @@ func TestGenericCache_StatsResetAtomicWithDelete_NoEvictionsUnderflow(t *testing c.Delete(key) } }() - defer wg.Wait() - defer close(stop) + total := uint64(0) for i := 0; i < 1_000_000; i++ { - if ev := c.evictions.Swap(0); ev > 1<<40 { - t.Fatalf("stats reset racing an intentional removal underflowed the evictions counter: %d", ev) - } + total += c.evictions.Swap(0) } + close(stop) + wg.Wait() + total += c.evictions.Swap(0) + require.Zero(t, total, "intentional removals surfaced in the evictions metric") } From dc2be257a777dd58b9396a9425660e53f797e18c Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 20:33:07 +0200 Subject: [PATCH 08/14] execution/cache: keep grow migration lossless by owning shard geometry Left to pick its own geometry per generation, freelru chooses more, smaller shards as capacity rises (it derives the count from GOMAXPROCS and table size), so a new shard receiving more entries than its capacity evicts during the migration copy - keys clustered on the shard-selection bits silently vanish across a grow, and a follow-up conditional put can install a stale value in the hole. Generations now get an explicit shard count: a lineage starts at ~64 entries per shard and the count doubles across grows only while per-shard capacity does not shrink, bounded by freelru's own GOMAXPROCS-derived ceiling. Power-of-two counts make the selection bits nest, so each new shard receives a subset of exactly one old shard and the copy can never overfill one. Steady-state shard counts at full size match what freelru would choose; the jump-grow log line now also reports the shard count. --- execution/cache/generic_cache.go | 69 +++++++++++++++---- .../cache/generic_cache_concurrency_test.go | 50 ++++++++++++++ 2 files changed, 105 insertions(+), 14 deletions(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 5298eaa9779..ef56a8b458c 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -18,6 +18,8 @@ package cache import ( "bytes" + "math/bits" + "runtime" "sync" "sync/atomic" "time" @@ -82,6 +84,14 @@ type GenericCache[T any] struct { resizeMu sync.Mutex reservedBytes int64 + // shardCount is the live generation's freelru shard count, bounded by + // shardCeil (freelru's own GOMAXPROCS-derived choice). Left to freelru, a + // grown generation could pick more, smaller shards and evict entries during + // the migration copy; instead shards double across grows only while + // per-shard capacity does not shrink (see maybeGrow). Mutated under resizeMu. + shardCount uint32 + shardCeil uint32 + currentSize atomic.Int64 // enveloped is set only when the cache draws from the shared envelope (via @@ -112,6 +122,19 @@ type GenericCache[T any] struct { func u64identity(k uint64) uint32 { return uint32(k) } +func nextPow2(v uint32) uint32 { + if v <= 1 { + return 1 + } + return 1 << bits.Len32(v-1) +} + +// initialShardCount starts a lineage at ~64 entries per shard (freelru's own +// small-cache geometry), bounded by ceil. +func initialShardCount(capacity, ceil uint32) uint32 { + return min(nextPow2(capacity/64), ceil) +} + const ( // genericCacheStartCapacity is the slot count a jump-grow cache is born with. // A cache whose working set never exceeds it (a test fixture) stays this small @@ -166,23 +189,28 @@ func newGenericCacheEntries[T any](capacityBytes datasize.ByteSize, capacityEntr sizeFunc: sizeFunc, } c.curCap.Store(capacityEntries) + c.shardCeil = nextPow2(uint32(runtime.GOMAXPROCS(0) * 16)) + c.shardCount = initialShardCount(capacityEntries, c.shardCeil) // Before any unwind every entry predates the (nonexistent) floor, so all // reads are valid; the floor only drops once an unwind happens. c.coh.Init() - c.data.Store(c.newShards(capacityEntries)) + c.data.Store(c.newShards(capacityEntries, c.shardCount)) return c } -// newShards builds a sharded LRU of the given capacity with this cache's evict -// callback wired. The callback is the sole subtractor of currentSize — every -// removal (capacity eviction, Remove) accounts through it. Freelru picks -// eviction victims per shard (hash bits 16+), which the put stripes (bits 0-7) -// don't cover, so any subtraction computed outside the callback races a -// cross-stripe eviction of the same entry. The callback must not feed the -// evictions metric — it also fires for intentional Removes — so capacity -// evictions are counted from Add's evicted return at the call sites. -func (c *GenericCache[T]) newShards(capacity uint32) *freelru.ShardedLRU[uint64, entry[T]] { - lru, err := freelru.NewSharded[uint64, entry[T]](capacity, u64identity) +// newShards builds a sharded LRU of the given capacity and shard count (see +// shardCount; the 1.25 slack mirrors freelru.NewSharded, and per-shard sizes +// stay large enough that freelru's internal shard clamp never overrides the +// count) with this cache's evict callback wired. The callback is the sole +// subtractor of currentSize — every removal (capacity eviction, Remove) +// accounts through it. Freelru picks eviction victims per shard (hash bits +// 16+), which the put stripes (bits 0-7) don't cover, so any subtraction +// computed outside the callback races a cross-stripe eviction of the same +// entry. The callback must not feed the evictions metric — it also fires for +// intentional Removes — so capacity evictions are counted from Add's evicted +// return at the call sites. +func (c *GenericCache[T]) newShards(capacity, shards uint32) *freelru.ShardedLRU[uint64, entry[T]] { + lru, err := freelru.NewShardedWithSize[uint64, entry[T]](shards, capacity, capacity+capacity/4, u64identity) if err != nil { panic(err) } @@ -217,8 +245,18 @@ func (c *GenericCache[T]) maybeGrow() { if !cachebudget.Global.Reserve(delta) { return } + // Shards double with capacity only while per-shard capacity does not + // shrink. The selection bits nest across power-of-two counts, so each new + // shard receives a subset of exactly one old shard and the copy below can + // never overfill one — freelru's own geometry for the larger capacity + // would pick more, smaller shards and evict during the copy. + perShardOld := (curCap + c.shardCount - 1) / c.shardCount + shards := c.shardCount + for shards*2 <= c.shardCeil && (newCap+shards*2-1)/(shards*2) >= perShardOld { + shards *= 2 + } start := time.Now() - next := c.newShards(newCap) // allocate before excluding writers + next := c.newShards(newCap, shards) // allocate before excluding writers fenceStart := time.Now() for i := range c.putStripes { c.putStripes[i].Lock() @@ -234,12 +272,13 @@ func (c *GenericCache[T]) maybeGrow() { } c.data.Store(next) c.curCap.Store(newCap) + c.shardCount = shards for i := range c.putStripes { c.putStripes[i].Unlock() } c.evictions.Add(uint64(evicted)) c.reservedBytes += delta - log.Debug("[cache] jump-grow", "fromSlots", curCap, "toSlots", newCap, "copied", copied, "evicted", evicted, + log.Debug("[cache] jump-grow", "fromSlots", curCap, "toSlots", newCap, "shards", shards, "copied", copied, "evicted", evicted, "alloc", fenceStart.Sub(start), "fenced", time.Since(fenceStart)) } @@ -453,12 +492,14 @@ func (c *GenericCache[T]) Clear() { cachebudget.Global.Release(c.reservedBytes - int64(c.startCap)*c.avgEntryBytes) c.reservedBytes = int64(c.startCap) * c.avgEntryBytes } - next := c.newShards(c.startCap) // allocate before excluding writers + shards := initialShardCount(c.startCap, c.shardCeil) + next := c.newShards(c.startCap, shards) // allocate before excluding writers for i := range c.putStripes { c.putStripes[i].Lock() } c.currentSize.Store(0) c.coh.Init() + c.shardCount = shards c.curCap.Store(c.startCap) c.data.Store(next) for i := range c.putStripes { diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index d882435bf62..517fe7c00a7 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -196,6 +196,56 @@ func TestGenericCache_PutIfAbsentDefersAcrossGrow(t *testing.T) { } } +// A grow must migrate every entry. Left to pick its own geometry per +// generation, freelru chooses more, smaller shards as capacity rises, and a +// new shard that overfills during the copy silently evicts — keys clustered +// on the shard-selection bits vanish across a "grow", and a follow-up +// conditional put can install a stale value in the hole. Seeding writes the +// clustered keys after the pad so they are the newest in their shard and +// cannot be seeding-eviction victims; only the migration can lose them. +func TestGenericCache_GrowMigrationLossless(t *testing.T) { + c := NewGenericCacheWithAvg[[]byte](4*datasize.MB, 256, func(v []byte) int { return len(v) }, ModeEvictLRU) + defer c.Close() + + // Keys sharing hash bits 16-23 land in one shard of any generation with up + // to 256 shards. + target := (maphash.Hash([]byte("cluster-seed")) >> 16) & 255 + var clustered [][]byte + for i := 0; len(clustered) < 24; i++ { + k := make([]byte, 8) + binary.BigEndian.PutUint64(k, uint64(i)) + if (maphash.Hash(k)>>16)&255 == target { + clustered = append(clustered, k) + } + } + pad := make([]byte, 9) + for j := 0; c.Len() < genericCacheStartCapacity-len(clustered); j++ { + binary.BigEndian.PutUint64(pad[1:], uint64(j)) + c.Put(pad, []byte{1}, 1) + } + for _, k := range clustered { + c.Put(k, []byte("fresh"), 10) + } + for j := 1 << 20; c.Len() < genericCacheStartCapacity; j++ { + binary.BigEndian.PutUint64(pad[1:], uint64(j)) + c.Put(pad, []byte{1}, 1) + if j > 1<<21 { + t.Fatal("seeding could not fill the cache to the grow threshold") + } + } + before := c.data.Load() + c.Put([]byte("grow-trigger"), []byte{1}, 1) + require.NotEqual(t, before, c.data.Load(), "grow did not happen") + + lost := 0 + for _, k := range clustered { + if _, ok := c.Get(k); !ok { + lost++ + } + } + require.Zero(t, lost, "grow migration evicted clustered entries: per-shard capacity shrank across the swap") +} + // A capacity eviction is a size-subtracting writer the put stripes cannot // serialize: freelru picks its victim per shard (hash bits 16+), so an insert // on one stripe can evict a key whose own update — on another stripe — is From 6ba18ed321a14572baf8dce8ee1f4cb772ce2365 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 20:39:16 +0200 Subject: [PATCH 09/14] execution/cache: reserve entry size before remove-then-add The update and collision paths removed the old entry before adding the new one and only then adjusted the byte counter, so a concurrent ModeNoOp admission on another stripe could observe the transient dip and admit a key past a full budget - breaking "drop new keys when full". Reserve the new size before the removal instead: the counter transiently over-states usage, which at worst drops a new key, and the settled sums are unchanged. --- execution/cache/generic_cache.go | 11 +++++-- .../cache/generic_cache_concurrency_test.go | 33 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index ef56a8b458c..4b1503435d7 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -398,11 +398,15 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit if !overwrite && !c.coh.IsStale(existing.txNum, existing.epoch) { return false } + // Reserve the new size before the removal: the byte counter must never + // transiently under-state usage, or a concurrent ModeNoOp admission on + // another stripe over-admits past the budget. Over-stating is safe — at + // worst a new key is dropped, which is within "drop new keys when full". + c.currentSize.Add(int64(newSize)) lru.Remove(h) if lru.Add(h, entry[T]{key: existing.key, val: value, size: newSize, txNum: txNum, epoch: ep}) { c.evictions.Add(1) } - c.currentSize.Add(int64(newSize)) return false } @@ -435,7 +439,9 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit // hasExisting here means a 64-bit maphash collision (different key, same // hash): remove the colliding entry first so OnEvict accounts for it — - // freelru.Add would replace it in place without firing OnEvict. + // freelru.Add would replace it in place without firing OnEvict. The size + // is reserved before the removal (see the update path above). + c.currentSize.Add(int64(newSize)) if hasExisting { lru.Remove(h) } @@ -443,7 +449,6 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit if lru.Add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) { c.evictions.Add(1) } - c.currentSize.Add(int64(newSize)) c.inserts.Add(1) return needGrow } diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index 517fe7c00a7..001a6ca56ea 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -196,6 +196,39 @@ func TestGenericCache_PutIfAbsentDefersAcrossGrow(t *testing.T) { } } +// A ModeNoOp admission must never observe the byte counter mid-update: the +// update path removes the old entry before adding the new one, and a +// concurrent insert on another stripe that reads the transient dip passes the +// budget check and lands over capacity — breaking "drop new keys when full" +// with a key that should never have been admitted. The counter is reserved +// before the removal, so the budget is transiently over-stated (at worst +// dropping a new key) and never under-stated. +func TestGenericCache_ModeNoOpAdmissionAtomicWithUpdate(t *testing.T) { + a := []byte("key-a-aaaaaaaaaaaaaa") + var b []byte + for i := 0; ; i++ { + cand := []byte("key-b-bbbbbbbbbbbbb" + string(rune('a'+i%26))) + if maphash.Hash(a)&(putStripeCount-1) != maphash.Hash(cand)&(putStripeCount-1) { + b = cand + break + } + } + v := []byte("valuevalu") // entry size 20+9+24 = 53: the budget fits exactly one entry + c := newGenericCacheEntries(datasize.ByteSize(53), 8, func(v []byte) int { return len(v) }, ModeNoOp) + c.Put(a, v, 1) + for round := 0; round < 200000; round++ { + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(a, v, 2) }() + go func() { defer wg.Done(); c.Put(b, v, 1) }() + wg.Wait() + if _, ok := c.Get(b); ok { + t.Fatalf("round %d: ModeNoOp admitted a key past a full budget (SizeBytes=%d, capacityB=%d)", + round, c.SizeBytes(), c.CapacityBytes()) + } + } +} + // A grow must migrate every entry. Left to pick its own geometry per // generation, freelru chooses more, smaller shards as capacity rises, and a // new shard that overfills during the copy silently evicts — keys clustered From b5c6b361c6794b45971cc3fece288c2b2d09b597 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 21:20:56 +0200 Subject: [PATCH 10/14] execution/cache: modernize test loops for the rangeint linter Main now enforces modernize's rangeint; the tests added on this branch predate that. --- execution/cache/cache_test.go | 6 +++--- execution/cache/generic_cache_concurrency_test.go | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index aeabbe9efed..0559e23aa92 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -890,7 +890,7 @@ func TestDomainCache_DeleteAtomicWithPut_NoSizeDrift(t *testing.T) { addr := makeAddr(1) v1 := []byte("value-one") v2 := []byte("value-two") - for round := 0; round < 20000; round++ { + for round := range 20000 { c.Put(addr, v1, 10) var wg sync.WaitGroup wg.Add(2) @@ -912,7 +912,7 @@ func TestDomainCache_StaleDropAtomicWithPut_NoSizeDrift(t *testing.T) { v1 := []byte("value-one") v2 := []byte("value-two") wantSize := int64(len(addr) + len(v1) + 24) - for round := 0; round < 20000; round++ { + for round := range 20000 { c.Put(addr, v1, 10) c.Unwind(5) var wg sync.WaitGroup @@ -933,7 +933,7 @@ func TestDomainCache_ClearAtomicWithPut_NoSizeDrift(t *testing.T) { addr := makeAddr(1) v1 := []byte("value-one") entrySize := int64(len(addr) + len(v1) + 24) - for round := 0; round < 20000; round++ { + for round := range 20000 { var wg sync.WaitGroup wg.Add(2) go func() { defer wg.Done(); c.Put(addr, v1, 10) }() diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index beaded957a0..1e80c38388a 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -69,7 +69,7 @@ func TestGenericCache_PutNotLostAcrossGrow(t *testing.T) { binary.BigEndian.PutUint64(b, n) return b } - for round := 0; round < 50; round++ { + for round := range 50 { c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) hot := []byte("hot-key") c.Put(hot, value(0), 1) @@ -118,7 +118,7 @@ func TestGenericCache_PutNotLostAcrossGrow(t *testing.T) { // Cross the grow threshold so maybeGrow swaps the generation while the // hot-key writer runs. key := make([]byte, 8) - for i := 0; i < 3*genericCacheStartCapacity; i++ { + for i := range 3 * genericCacheStartCapacity { binary.BigEndian.PutUint64(key, uint64(1+i)) c.Put(key, []byte{1}, 1) } @@ -145,10 +145,10 @@ func TestGenericCache_PutNotLostAcrossGrow(t *testing.T) { func TestGenericCache_PutIfAbsentDefersAcrossGrow(t *testing.T) { fresh := []byte("fresh-value") stale := []byte("stale-value") - for round := 0; round < 100; round++ { + for round := range 100 { c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) key := make([]byte, 8) - for i := 0; i < 256; i++ { + for i := range 256 { binary.BigEndian.PutUint64(key, uint64(1+i)) c.Put(key, []byte{1}, 1) } @@ -216,7 +216,7 @@ func TestGenericCache_ModeNoOpAdmissionAtomicWithUpdate(t *testing.T) { v := []byte("valuevalu") // entry size 20+9+24 = 53: the budget fits exactly one entry c := newGenericCacheEntries(datasize.ByteSize(53), 8, func(v []byte) int { return len(v) }, ModeNoOp) c.Put(a, v, 1) - for round := 0; round < 200000; round++ { + for round := range 200000 { var wg sync.WaitGroup wg.Add(2) go func() { defer wg.Done(); c.Put(a, v, 2) }() @@ -298,7 +298,7 @@ func TestGenericCache_CapacityEvictionAtomicWithPut_NoSizeDrift(t *testing.T) { } } v := []byte("value-one") - for round := 0; round < 100000; round++ { + for range 100000 { c.Put(b, v, 10) var wg sync.WaitGroup wg.Add(2) @@ -373,7 +373,7 @@ func TestGenericCache_StatsResetAtomicWithDelete_NoPhantomEvictions(t *testing.T } }() total := uint64(0) - for i := 0; i < 1_000_000; i++ { + for range 1_000_000 { total += c.evictions.Swap(0) } close(stop) From 4b548f0939a2158f64e238c5fff32252a8df740c Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 21:45:53 +0200 Subject: [PATCH 11/14] execution/cache: keep dead entries dead for readers racing Clear GetWithTxNum judged a captured entry against live coherence, so a Clear completing between the generation load and the staleness check re-inited coherence (fresh epoch, lifted floor) and revalidated an unwind-invalidated entry from the retiring generation - serving dead-fork state. The fix needs both orderings: the reader snapshots coherence before loading the generation, and Clear re-inits coherence only after its swap. Either alone leaves a window - a snapshot taken in Clear's (init, swap) gap still pairs post-init coherence with the retiring generation; the new test caught exactly that on the first fix attempt. An old-generation entry is now always judged by a pre-init snapshot that still carries the unwind, and a live entry judged by a pre-Clear snapshot degrades to a safe miss via dropStale's re-check. The test parks a reader on the fence reaching the key's stripe so its Get lands next to the re-init; it served the dead value within tens of rounds before the fix. --- execution/cache/coherence/coherence.go | 23 +++++++- execution/cache/generic_cache.go | 17 +++++- .../cache/generic_cache_concurrency_test.go | 52 +++++++++++++++++++ 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/execution/cache/coherence/coherence.go b/execution/cache/coherence/coherence.go index 44f50c80d8a..6410f69793d 100644 --- a/execution/cache/coherence/coherence.go +++ b/execution/cache/coherence/coherence.go @@ -64,9 +64,30 @@ func (g *Gen) load() *gen { } // IsStale reports whether an entry stamped (txNum, epoch) reflects dead-fork -// state after an unwind. +// state after an unwind, judged by the live coherence state. func (g *Gen) IsStale(txNum uint64, epoch uint32) bool { + return g.Snapshot().IsStale(txNum, epoch) +} + +// Snapshot is an immutable (epoch, floor) pair for judging entries against +// the coherence state captured at a chosen point — e.g. before loading a +// cache generation, so a concurrent Clear's re-init (fresh epoch, lifted +// floor) cannot revalidate a dead entry captured from the retiring +// generation. +type Snapshot struct { + epoch uint32 + floor uint64 +} + +// Snapshot returns the current (epoch, floor) pair. +func (g *Gen) Snapshot() Snapshot { s := g.load() + return Snapshot{epoch: s.epoch, floor: s.floor} +} + +// IsStale reports whether an entry stamped (txNum, epoch) reflects dead-fork +// state under this snapshot. +func (s Snapshot) IsStale(txNum uint64, epoch uint32) bool { return epoch != s.epoch && txNum >= s.floor } diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 4b1503435d7..125b0df6233 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -324,6 +324,15 @@ func (c *GenericCache[T]) Get(key []byte) (T, bool) { // maxStep — the same coherence the BranchCache read applies for commitment. func (c *GenericCache[T]) GetWithTxNum(key []byte) (T, uint64, bool) { h := maphash.Hash(key) + // Snapshot coherence before loading the generation: judged against the live + // state instead, a Clear landing between the load and the staleness check + // re-inits coherence (fresh epoch, lifted floor) and revalidates a dead + // entry captured from the retiring generation. Paired with Clear re-initing + // only after its swap, an old-generation entry is always judged by a + // pre-init snapshot that still carries the unwind. A live entry judged by a + // pre-Clear snapshot only degrades to a miss (dropStale re-checks and keeps + // it). + coh := c.coh.Snapshot() lru := c.data.Load() e, ok := lru.Get(h) if !ok || !bytes.Equal(e.key, key) { @@ -339,7 +348,7 @@ func (c *GenericCache[T]) GetWithTxNum(key []byte) (T, uint64, bool) { // dead block — e.g. an EIP-4788 beacon-root write in the block-begin system // tx — and must be dropped; >= not > (the surviving block's last txNum is // floor-1, so this never drops a live entry). - if c.coh.IsStale(e.txNum, e.epoch) { + if coh.IsStale(e.txNum, e.epoch) { c.dropStale(h, key) c.staleEvicted.Add(1) c.misses.Add(1) @@ -503,10 +512,14 @@ func (c *GenericCache[T]) Clear() { c.putStripes[i].Lock() } c.currentSize.Store(0) - c.coh.Init() c.shardCount = shards c.curCap.Store(c.startCap) c.data.Store(next) + // Re-init coherence only after the swap: paired with GetWithTxNum's + // snapshot-before-load ordering, an entry captured from the retiring + // generation is then always judged by pre-init coherence that still + // carries the unwind. + c.coh.Init() for i := range c.putStripes { c.putStripes[i].Unlock() } diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index 1e80c38388a..ce267a2f856 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -346,6 +346,58 @@ func TestGenericCache_ClearRacingPut_EpochAlias(t *testing.T) { require.False(t, ok, "entry at txNum 200 outlived an unwind to 150: its pre-Clear epoch stamp aliases the live epoch") } +// A reader that captures a dead (unwind-invalidated) entry from the retiring +// generation must not have it revalidated by Clear's coherence re-init: +// judged against the post-Init state (fresh epoch, lifted floor), the entry +// passes IsStale and dead-fork state is served. Coherence is snapshotted +// before the generation load, so an old-generation entry is always judged by +// coherence that still carries the unwind. +// +// The reader gates on the fence reaching the key's stripe — the last one the +// sweep locks — so its Get lands next to the Init that follows. +func TestGenericCache_ClearRacingGet_DeadEntryStaysDead(t *testing.T) { + var key []byte + for i := 0; ; i++ { + k := make([]byte, 8) + binary.BigEndian.PutUint64(k, uint64(i)) + if maphash.Hash(k)&(putStripeCount-1) == putStripeCount-1 { + key = k + break + } + } + dead := []byte("dead-fork-value") + c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + defer c.Close() + for round := range 2000 { + c.Put(key, dead, 200) + c.Unwind(150) // the entry is dead-fork state; it must never be served again + var served atomic.Bool + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Clear() }() + go func() { + defer wg.Done() + mu := &c.putStripes[putStripeCount-1] + for range 1 << 16 { + if mu.TryLock() { + mu.Unlock() + continue + } + break + } + for range 4 { + if _, ok := c.Get(key); ok { + served.Store(true) + return + } + } + }() + wg.Wait() + require.False(t, served.Load(), + "round %d: Clear revalidated an unwind-invalidated entry for a concurrent reader", round) + } +} + // The evictions counter must carry capacity evictions only. Routing // intentional removals through it — decrement-compensated or netted against a // removal counter at print time — races a concurrent stats reset: the swap From cd62936a686919d457b66333920b5875f29b4733 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 21:47:58 +0200 Subject: [PATCH 12/14] execution/cache: use full-width rounds in the PutIfAbsent atomicity test makeAddr stores only byte(round), so the test cycled through 256 addresses and every round past the first cycle found a live leftover - PutIfAbsent deferred to it and the absent-key insert race went unexercised. Encode the full round, as the CodeCache twin does. --- execution/cache/cache_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index 0559e23aa92..da5f3615ebd 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -18,6 +18,7 @@ package cache import ( "bytes" + "encoding/binary" "sync" "testing" @@ -868,8 +869,11 @@ func TestDomainCache_PutIfAbsentAtomicWithPut(t *testing.T) { c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) fresh := []byte("fresh") stale := []byte("stale") + addr := make([]byte, 20) for round := range 20000 { - addr := makeAddr(round) + // Full-width round: the race only has teeth on a never-seen key, and + // makeAddr would truncate it to a byte. + binary.BigEndian.PutUint64(addr[1:], uint64(round)) var wg sync.WaitGroup wg.Add(2) go func() { defer wg.Done(); c.Put(addr, fresh, 20) }() From a0fa50f2c0855d8f26f72b2eb6dc9ed436268227 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Tue, 14 Jul 2026 22:15:39 +0200 Subject: [PATCH 13/14] execution/cache: trim putStriped grow comment to the invariant --- execution/cache/generic_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 125b0df6233..2dc3435ed37 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -431,7 +431,7 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit curCap := c.curCap.Load() // The insert lands before the grow (which must run outside the stripe), so // it and any racers until the swap evict at the pre-grow cap — a transient - // bounded by the grow window, not a regression of the grow-first ordering. + // bounded by the grow window. needGrow := c.mode != ModeNoOp && curCap < c.maxCap && lru.Len() >= int(curCap) // In ModeEvictLRU the byte budget is enforced through the entry-count cap, From 4f2f172a03c45c65ebbddf5c2e291ff67deab823 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Wed, 15 Jul 2026 12:05:42 +0200 Subject: [PATCH 14/14] execution/cache: release test cache reservations --- execution/cache/cache_test.go | 98 ++++++++++--------- execution/cache/code_cache_codehash_test.go | 28 +++--- .../cache/code_cache_concurrency_test.go | 8 +- .../cache/generic_cache_concurrency_test.go | 2 +- 4 files changed, 71 insertions(+), 65 deletions(-) diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index da5f3615ebd..6e2009d0cba 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -32,6 +32,12 @@ import ( ) // Test helpers +func closeOnCleanup[T interface{ Close() }](tb testing.TB, c T) T { + tb.Helper() + tb.Cleanup(c.Close) + return c +} + func makeAddr(i int) []byte { addr := make([]byte, 20) addr[19] = byte(i) @@ -57,7 +63,7 @@ func makeValue(i int) []byte { // ============================================================================= func TestDomainCache_NewWithByteCapacity(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) // 1MB + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) // 1MB require.NotNil(t, c) assert.Equal(t, 0, c.Len()) assert.Equal(t, int64(0), c.SizeBytes()) @@ -65,7 +71,7 @@ func TestDomainCache_NewWithByteCapacity(t *testing.T) { } func TestDomainCache_GetPut(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) addr := makeAddr(1) value := makeValue(1) @@ -84,7 +90,7 @@ func TestDomainCache_GetPut(t *testing.T) { } func TestDomainCache_PutUpdateValue(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) addr := makeAddr(1) value1 := []byte{1, 2, 3, 4, 5, 6, 7, 8} // 8 bytes @@ -105,7 +111,7 @@ func TestDomainCache_PutCapacityLimit_NoOpMode(t *testing.T) { // full, new keys are silently dropped. Counted via the dropped metric. // Entry overhead is 20 (addr key) + 3 (value) + 24 = 47 bytes per entry. // Two entries take 94 bytes; cap at 100 leaves no room for a third. - c := NewDomainCacheMode(100, ModeNoOp) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeNoOp)) c.Put(makeAddr(1), makeValue(1), 0) c.Put(makeAddr(2), makeValue(2), 0) @@ -156,7 +162,7 @@ func TestDomainCache_PutEvictsWhenFull_EvictMode(t *testing.T) { } func TestDomainCache_Delete(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) addr := makeAddr(1) c.Put(addr, makeValue(1), 0) @@ -170,7 +176,7 @@ func TestDomainCache_Delete(t *testing.T) { } func TestDomainCache_Clear(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) c.Put(makeAddr(1), makeValue(1), 0) c.Put(makeAddr(2), makeValue(2), 0) @@ -181,7 +187,7 @@ func TestDomainCache_Clear(t *testing.T) { } func TestDomainCache_PrintStatsAndReset(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) // Generate some hits and misses c.Put(makeAddr(1), makeValue(1), 0) @@ -197,7 +203,7 @@ func TestDomainCache_PrintStatsAndReset(t *testing.T) { } func TestDomainCache_PrintStatsAndReset_NoOps(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) // No operations - should handle zero total gracefully c.PrintStatsAndReset("test") } @@ -211,14 +217,14 @@ func TestDomainCache_ImplementsInterface(t *testing.T) { // ============================================================================= func TestCodeCache_NewDefaultCodeCache(t *testing.T) { - c := NewDefaultCodeCache() + c := closeOnCleanup(t, NewDefaultCodeCache()) require.NotNil(t, c) assert.Equal(t, 0, c.Len()) assert.Equal(t, 0, c.CodeLen()) } func TestCodeCache_GetPut(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) addr := makeAddr(1) code := makeCode(1) @@ -238,7 +244,7 @@ func TestCodeCache_GetPut(t *testing.T) { } func TestCodeCache_PutEmptyCode(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) addr := makeAddr(1) c.Put(addr, []byte{}, 0) @@ -249,7 +255,7 @@ func TestCodeCache_PutEmptyCode(t *testing.T) { } func TestCodeCache_CodeDeduplication(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) code := makeCode(1) addr1 := makeAddr(1) @@ -289,7 +295,7 @@ func TestCodeCache_AddrCapacityLimit(t *testing.T) { return []byte{0x60, byte(i >> 8), byte(i)} } - c := NewCodeCache(1024*1024, 1024*28) // 1MB code, ~1024 addr LRU entries + c := closeOnCleanup(t, NewCodeCache(1024*1024, 1024*28)) // 1MB code, ~1024 addr LRU entries for i := range 1100 { c.Put(wideAddr(i), wideCode(i), 0) } @@ -323,7 +329,7 @@ func TestCodeCache_AddrCapacityLimit(t *testing.T) { func TestCodeCache_CodeCapacityLimit(t *testing.T) { // Tiny byte budget → a 1-entry code layer cap. Successive distinct codes // LRU-evict the coldest rather than freezing the layer. - c := NewCodeCache(25, 1024*1024) // 25 bytes code, 1MB addr + c := closeOnCleanup(t, NewCodeCache(25, 1024*1024)) // 25 bytes code, 1MB addr c.Put(makeAddr(1), makeCode(1), 0) c.Put(makeAddr(2), makeCode(2), 0) @@ -343,7 +349,7 @@ func TestCodeCache_CodeCapacityLimit(t *testing.T) { } func TestCodeCache_Delete(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) addr := makeAddr(1) code := makeCode(1) @@ -359,7 +365,7 @@ func TestCodeCache_Delete(t *testing.T) { } func TestCodeCache_Clear(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) c.Put(makeAddr(1), makeCode(1), 0) c.Put(makeAddr(2), makeCode(2), 0) @@ -372,7 +378,7 @@ func TestCodeCache_Clear(t *testing.T) { } func TestCodeCache_PrintStatsAndReset(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) c.Put(makeAddr(1), makeCode(1), 0) c.Get(makeAddr(1)) // hit @@ -383,13 +389,13 @@ func TestCodeCache_PrintStatsAndReset(t *testing.T) { } func TestCodeCache_PrintStatsAndReset_NoOps(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) // No operations - should handle zero total gracefully c.PrintStatsAndReset() } func TestCodeCache_GetMissingCode(t *testing.T) { - c := NewCodeCache(1024*1024, 1024*1024) // 1MB each + c := closeOnCleanup(t, NewCodeCache(1024*1024, 1024*1024)) // 1MB each // Manually set addr mapping without code (simulates capacity limit scenario) addr := makeAddr(1) @@ -414,7 +420,7 @@ func TestCodeCache_ImplementsInterface(t *testing.T) { // ============================================================================= func TestStateCache_NewStateCache(t *testing.T) { - c := NewStateCache(10, 20, 30, 40) + c := closeOnCleanup(t, NewStateCache(10, 20, 30, 40)) require.NotNil(t, c) // Account, Storage, Code, Commitment should be initialized @@ -428,7 +434,7 @@ func TestStateCache_NewStateCache(t *testing.T) { } func TestStateCache_NewDefaultStateCache(t *testing.T) { - c := NewDefaultStateCache() + c := closeOnCleanup(t, NewDefaultStateCache()) require.NotNil(t, c) assert.NotNil(t, c.GetCache(kv.AccountsDomain)) @@ -437,7 +443,7 @@ func TestStateCache_NewDefaultStateCache(t *testing.T) { } func TestStateCache_GetPut_Account(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) addr := makeAddr(1) value := makeValue(1) @@ -455,7 +461,7 @@ func TestStateCache_GetPut_Account(t *testing.T) { } func TestStateCache_GetPut_Storage(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) key := make([]byte, 52) // addr(20) + slot(32) copy(key, makeAddr(1)) @@ -469,7 +475,7 @@ func TestStateCache_GetPut_Storage(t *testing.T) { } func TestStateCache_GetPut_Code(t *testing.T) { - c := NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) code := makeCode(1) @@ -481,7 +487,7 @@ func TestStateCache_GetPut_Code(t *testing.T) { } func TestStateCache_GetPut_UnsupportedDomain(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) // ReceiptDomain is not supported c.Put(kv.ReceiptDomain, makeAddr(1), makeValue(1), 0) @@ -491,7 +497,7 @@ func TestStateCache_GetPut_UnsupportedDomain(t *testing.T) { } func TestStateCache_Delete(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) addr := makeAddr(1) c.Put(kv.AccountsDomain, addr, makeValue(1), 0) @@ -505,7 +511,7 @@ func TestStateCache_Delete(t *testing.T) { // caches deleted keys via Put(key, nil); if Get treats that as "not found", // the caller unnecessarily falls through to the DB on every read. func TestStateCache_PutEmpty_ThenGet_IsCacheHit(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) key := make([]byte, 52) // addr(20) + slot(32) key[0] = 0x1d @@ -520,7 +526,7 @@ func TestStateCache_PutEmpty_ThenGet_IsCacheHit(t *testing.T) { // Same test for []byte{} (zero-length but non-nil). func TestStateCache_PutEmptySlice_ThenGet_IsCacheHit(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) key := make([]byte, 52) key[0] = 0x1d @@ -534,14 +540,14 @@ func TestStateCache_PutEmptySlice_ThenGet_IsCacheHit(t *testing.T) { } func TestStateCache_Delete_UnsupportedDomain(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) // Should not panic c.Delete(kv.ReceiptDomain, makeAddr(1)) } func TestStateCache_Clear(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) c.Put(kv.AccountsDomain, makeAddr(1), makeValue(1), 0) c.Put(kv.StorageDomain, makeAddr(2), makeValue(2), 0) @@ -559,7 +565,7 @@ func TestStateCache_Clear(t *testing.T) { } func TestStateCache_GetCache_OutOfBounds(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) // Domain >= DomainLen should return nil cache := c.GetCache(kv.DomainLen) @@ -574,7 +580,7 @@ func TestStateCache_GetCache_OutOfBounds(t *testing.T) { // ============================================================================= func TestDomainCache_ConcurrentAccess(t *testing.T) { - c := NewDomainCacheMode(10000, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(10000, ModeEvictLRU)) done := make(chan bool) @@ -599,7 +605,7 @@ func TestDomainCache_ConcurrentAccess(t *testing.T) { } func TestCodeCache_ConcurrentAccess(t *testing.T) { - c := NewCodeCache(1000, 1000) + c := closeOnCleanup(t, NewCodeCache(1000, 1000)) done := make(chan bool) @@ -628,7 +634,7 @@ func TestCodeCache_ConcurrentAccess(t *testing.T) { // ============================================================================= func TestStateCache_DomainIsolation(t *testing.T) { - c := NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) accountData := []byte("account") @@ -689,7 +695,7 @@ func makeDiffKey(baseKey []byte, step uint64) string { // Entries stamped at/below the unwind point survive (warm hot set kept); entries // above it from the now-dead epoch are dropped lazily on read. func TestUnwind_KeepsBelowFloor_EvictsAbove(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) below := makeAddr(1) above := makeAddr(2) c.Put(below, makeValue(1), 50) // predates the unwind @@ -710,7 +716,7 @@ func TestUnwind_KeepsBelowFloor_EvictsAbove(t *testing.T) { // so an entry stamped at exactly that txNum is dead-fork state and must be // evicted — the drop rule is txNum >= floor, not txNum > floor. func TestUnwind_EvictsEntryAtFloor(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) atFloor := makeAddr(1) belowFloor := makeAddr(2) c.Put(atFloor, makeValue(1), 100) // first txNum of the first unwound block @@ -730,7 +736,7 @@ func TestUnwind_EvictsEntryAtFloor(t *testing.T) { // SAME txNum as the dead fork's write. The epoch — not the txNum — distinguishes // them, so the dead entry reads stale and the re-written one reads valid. func TestUnwind_ReusedTxNumDisambiguatedByEpoch(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) k := makeAddr(1) c.Put(k, makeValue(1), 150) // dead fork, epoch 0 @@ -749,7 +755,7 @@ func TestUnwind_ReusedTxNumDisambiguatedByEpoch(t *testing.T) { // dead epoch above the floor and reads stale no matter how far execution // advances afterwards (there is no rising high-water mark to re-validate it). func TestUnwind_StragglerNeverResurrects(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) straggler := makeAddr(1) c.Put(straggler, makeValue(1), 150) // epoch 0 @@ -766,7 +772,7 @@ func TestUnwind_StragglerNeverResurrects(t *testing.T) { // A second, shallower unwind must not resurrect entries a deeper earlier unwind // invalidated (floor only moves down). func TestUnwind_FloorOnlyMovesDown(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) k := makeAddr(1) c.Put(k, makeValue(1), 70) // epoch 0 @@ -778,7 +784,7 @@ func TestUnwind_FloorOnlyMovesDown(t *testing.T) { } func TestDomainCache_PutIfAbsent(t *testing.T) { - c := NewDomainCacheMode(1*datasize.KB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.KB, ModeEvictLRU)) addr := makeAddr(1) fresh := []byte("fresh") stale := []byte("stale") @@ -813,7 +819,7 @@ func TestDomainCache_PutIfAbsent(t *testing.T) { } func TestCodeCache_PutIfAbsentKeepsLiveAddrBinding(t *testing.T) { - cc := NewCodeCache(1*datasize.MB, 1*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) fresh := []byte{0xaa, 1, 2, 3} stale := []byte{0xbb, 4, 5, 6} @@ -838,7 +844,7 @@ func TestCodeCache_PutIfAbsentKeepsLiveAddrBinding(t *testing.T) { } func TestCodeCache_PutWithCodeHashIfAbsent(t *testing.T) { - cc := NewCodeCache(1*datasize.MB, 1*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) fresh := []byte{0xaa, 1, 2, 3} stale := []byte{0xbb, 4, 5, 6} @@ -866,7 +872,7 @@ func TestCodeCache_PutWithCodeHashIfAbsent(t *testing.T) { // check (absent), lose the CPU to the authoritative writer's insert, then // clobber it — the prefetch-vs-flush staleness this cache guards against. func TestDomainCache_PutIfAbsentAtomicWithPut(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) fresh := []byte("fresh") stale := []byte("stale") addr := make([]byte, 20) @@ -890,7 +896,7 @@ func TestDomainCache_PutIfAbsentAtomicWithPut(t *testing.T) { // put's update delta subtracts it again unless the two writers share the // key's stripe. func TestDomainCache_DeleteAtomicWithPut_NoSizeDrift(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) addr := makeAddr(1) v1 := []byte("value-one") v2 := []byte("value-two") @@ -911,7 +917,7 @@ func TestDomainCache_DeleteAtomicWithPut_NoSizeDrift(t *testing.T) { // entry's size. Exactly one live entry remains after every round, so drift // shows as a size mismatch. func TestDomainCache_StaleDropAtomicWithPut_NoSizeDrift(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) addr := makeAddr(1) v1 := []byte("value-one") v2 := []byte("value-two") @@ -933,7 +939,7 @@ func TestDomainCache_StaleDropAtomicWithPut_NoSizeDrift(t *testing.T) { // lands its entry where no reader sees it and adds the entry's size after // Clear zeroed the counter — inflating SizeBytes for an invisible entry. func TestDomainCache_ClearAtomicWithPut_NoSizeDrift(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) addr := makeAddr(1) v1 := []byte("value-one") entrySize := int64(len(addr) + len(v1) + 24) diff --git a/execution/cache/code_cache_codehash_test.go b/execution/cache/code_cache_codehash_test.go index dd7feee771a..a57664f1025 100644 --- a/execution/cache/code_cache_codehash_test.go +++ b/execution/cache/code_cache_codehash_test.go @@ -34,7 +34,7 @@ func makeCodeHash(i int) []byte { } func TestCodeCache_GetByCodeHash_HitAfterPut(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) code := []byte{0x60, 0x80, 0x60, 0x40, 0x52} // small contract preamble codeHash := makeCodeHash(0xab) @@ -61,7 +61,7 @@ func TestCodeCache_GetByCodeHash_HitAfterPut(t *testing.T) { func TestCodeCache_GetByCodeHash_DistinctAddrsSameCode(t *testing.T) { // The point of codeHashToCode: many addresses sharing one codeHash all hit a // single entry once any one of them has been populated. - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) code := []byte{0x60, 0x80, 0x60, 0x40, 0x52} codeHash := makeCodeHash(0xcd) @@ -82,7 +82,7 @@ func TestCodeCache_GetByCodeHash_DistinctAddrsSameCode(t *testing.T) { } func TestCodeCache_PutWithCodeHash_EmptyHashOrCodeIsNoOp(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) code := []byte{0x60, 0x00} @@ -101,7 +101,7 @@ func TestCodeCache_PutWithCodeHash_EvictsColdestWhenFull(t *testing.T) { // Tiny byte budget → a 1-entry freelru cap. The second put must EVICT the // coldest entry (LRU), not freeze the layer: the newest code is retrievable // and the oldest is gone. - c := NewCodeCache(8, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(8, 1*datasize.MB)) c.PutWithCodeHash(makeAddr(1), []byte{1, 2, 3, 4}, makeCodeHash(1), 0) c.PutWithCodeHash(makeAddr(2), []byte{5, 6, 7, 8}, makeCodeHash(2), 0) @@ -112,7 +112,7 @@ func TestCodeCache_PutWithCodeHash_EvictsColdestWhenFull(t *testing.T) { } func TestCodeCache_CodeSize_PopulatedAlongsideBytes(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) code := []byte{0x60, 0x80, 0x60, 0x40, 0x52, 0x60, 0x10} codeHash := makeCodeHash(0xee) @@ -129,7 +129,7 @@ func TestCodeCache_CodeSize_PopulatedAlongsideBytes(t *testing.T) { } func TestCodeCache_CodeSize_DirectPutAndGet(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) codeHash := makeCodeHash(0xff) // Direct Put without going through the bytes layer. @@ -141,7 +141,7 @@ func TestCodeCache_CodeSize_DirectPutAndGet(t *testing.T) { } func TestCodeCache_CodeSize_EmptyHashOrNegativeIsNoOp(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) c.PutCodeSizeByCodeHash(nil, 100, 0) c.PutCodeSizeByCodeHash(makeCodeHash(1), -1, 0) _, ok := c.GetCodeSizeByCodeHash(makeCodeHash(1)) @@ -153,7 +153,7 @@ func TestCodeCache_CodeSize_EmptyHashOrNegativeIsNoOp(t *testing.T) { // ============================================================================= func BenchmarkCodeCache_GetByCodeHash_Hit(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := bytes.Repeat([]byte{0x5b}, 2048) // 2 KiB typical contract size codeHash := makeCodeHash(0x11) c.PutWithCodeHash(makeAddr(1), code, codeHash, 0) @@ -168,7 +168,7 @@ func BenchmarkCodeCache_GetByCodeHash_Hit(b *testing.B) { } func BenchmarkCodeCache_GetByCodeHash_Miss(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) missHash := makeCodeHash(0x22) b.ResetTimer() @@ -181,7 +181,7 @@ func BenchmarkCodeCache_GetByCodeHash_Miss(b *testing.B) { // path. Compare against GetByCodeHash to verify the codeHashToCode lookup is at least // as fast (one map probe vs two: addr→hash then hash→code). func BenchmarkCodeCache_Get_AddrLevel_Hit(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := bytes.Repeat([]byte{0x5b}, 2048) addr := makeAddr(1) c.PutWithCodeHash(addr, code, makeCodeHash(0x33), 0) @@ -200,7 +200,7 @@ func BenchmarkCodeCache_Get_AddrLevel_Hit(b *testing.B) { // Without codeHashToCode every fresh addr would pay a file read. With codeHashToCode every // caller that already knows the hash hits one shared entry. func BenchmarkCodeCache_GetByCodeHash_ManyAddrs_OneCode(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := bytes.Repeat([]byte{0x5b}, 2048) codeHash := makeCodeHash(0x44) c.PutWithCodeHash(makeAddr(1), code, codeHash, 0) // populate once @@ -221,7 +221,7 @@ func BenchmarkCodeCache_GetByCodeHash_ManyAddrs_OneCode(b *testing.B) { // content-addressed codeHash→code, and the size layer — not just the addr // layer. The code's value is invariant for a hash, but its existence is not. func TestCodeCache_Unwind_DropsUnwoundCodeEverywhere(t *testing.T) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := makeAddr(1) code := bytes.Repeat([]byte{0x60}, 64) @@ -254,7 +254,7 @@ func TestCodeCache_Unwind_DropsUnwoundCodeEverywhere(t *testing.T) { // TestCodeCache_Unwind_BelowFloorSurvives verifies code deployed below the // unwind floor (still live after the unwind) stays warm on all layers. func TestCodeCache_Unwind_BelowFloorSurvives(t *testing.T) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := makeAddr(2) code := bytes.Repeat([]byte{0x61}, 32) @@ -274,7 +274,7 @@ func TestCodeCache_Unwind_BelowFloorSurvives(t *testing.T) { // on the live fork (current epoch) after an unwind makes it discoverable again, // even though a stale entry at the same txNum was left behind. func TestCodeCache_Unwind_RedeployRevives(t *testing.T) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := makeAddr(3) code := bytes.Repeat([]byte{0x62}, 48) diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 8e2a5827897..9cf56b7508f 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -36,7 +36,7 @@ import ( // goroutine that actually inserts accounts the size, so the counters must equal // exactly one entry regardless of how many concurrent Puts raced. func TestCodeCache_ConcurrentPutSameCode_NoSizeDrift(t *testing.T) { - cc := NewCodeCache(64*datasize.MB, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := make([]byte, 20) addr[0] = 0xab @@ -70,7 +70,7 @@ func TestCodeCache_ConcurrentPutSameCode_NoSizeDrift(t *testing.T) { // an entry whose stored keyHash differs from the requested codeHash is treated // as a miss, so a 64-bit maphash collision can never serve the wrong code. func TestCodeCache_ByteCheckRejectsForeignKeyHash(t *testing.T) { - cc := NewCodeCache(64*datasize.MB, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := []byte("contract A bytecode") realHash := crypto.Keccak256(code) @@ -98,7 +98,7 @@ func TestCodeCache_ByteCheckRejectsForeignKeyHash(t *testing.T) { // OnEvict-maintained byte counter must never drift negative under concurrency. func TestCodeCache_ConcurrentDistinctPuts_RespectCap(t *testing.T) { const codeCap = 4 * datasize.KB - cc := NewCodeCache(codeCap, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(codeCap, 16*datasize.MB)) const workers = 128 var wg sync.WaitGroup @@ -125,7 +125,7 @@ func TestCodeCache_ConcurrentDistinctPuts_RespectCap(t *testing.T) { // authoritative Put must win over a conditional prefetch put in every // interleaving. func TestCodeCache_PutIfAbsentAtomicWithPut(t *testing.T) { - cc := NewCodeCache(64*datasize.MB, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := make([]byte, 20) addr[0] = 0xcd fresh := []byte{0xaa, 1, 2, 3} diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index ce267a2f856..0639c8e1131 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -37,7 +37,7 @@ import ( // -race, this must stay clean. func TestGenericCache_ConcurrentPutAcrossGrow(t *testing.T) { // Budget well above the start size (1024 slots) so maybeGrow fires repeatedly. - c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + c := closeOnCleanup(t, NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU)) const workers = 8 const perWorker = 20_000