Bug: collection.Cache can retain entries without expiry timers after concurrent Set calls

Description

collection.Cache can retain entries indefinitely after concurrent Set calls: entries remain in Cache.data even though their keys are absent from the timing wheel's timer registry. Consequently, Get keeps returning a hit beyond the configured TTL.

The reproduction uses only NewCache(time.Second) and concurrent Set calls. No explicit Del, Drain, Redis, or application-specific wrapper is involved. The test is placed in package collection solely to inspect whether surviving entries still have registered expiry timers.

Versions and environment

• Linux amd64
• Upstream go-zero v1.10.3, Go 1.24.11: reproduced in all 3 runs.
• Upstream go-zero v1.8.2, Go 1.23.12: reproduced in all 3 runs.
• Unmodified production source from the upstream Go modules; only the following test was added.

Reproduction

Save the following as core/collection/cache_expiry_regression_test.go in an upstream checkout:

package collection

import (
	"fmt"
	"sync"
	"testing"
	"time"
)

// Run in the upstream collection package so the timer registry can be checked
// without unsafe/reflection or changes to production code.
func TestCacheExpiryConcurrentWritesRegression(t *testing.T) {
	c, err := NewCache(time.Second)
	if err != nil {
		t.Fatal(err)
	}
	t.Cleanup(c.timingWheel.Stop)
	const count = 2000
	keys := make([]string, count)
	for i := range keys {
		keys[i] = fmt.Sprintf("key-%d", i)
	}
	var wg sync.WaitGroup
	deadline := time.Now().Add(3 * time.Second)
	for worker := 0; worker < 8; worker++ {
		wg.Add(1)
		go func(offset int) {
			defer wg.Done()
			for i := offset; time.Now().Before(deadline); i++ {
				c.Set(keys[i%count], 1)
			}
		}(worker * 251)
	}
	wg.Wait()
	time.Sleep(3 * time.Second)
	var survivors []string
	for _, key := range keys {
		if _, ok := c.Get(key); ok {
			survivors = append(survivors, key)
		}
	}
	t.Logf("3 seconds after writes stopped: %d/%d entries remain", len(survivors), count)
	// Allow any already dispatched expiry callbacks to finish before checking
	// whether the surviving entries still have registered timers.
	time.Sleep(2 * time.Second)
	orphans, scheduled := 0, 0
	for _, key := range survivors {
		if _, ok := c.Get(key); !ok {
			continue
		}
		if _, ok := c.timingWheel.timers.Get(key); ok {
			scheduled++
		} else {
			orphans++
		}
	}
	t.Logf("5 seconds after writes stopped: entries without timers=%d, entries with timers=%d", orphans, scheduled)
	if orphans != 0 {
		t.Fatalf("cache retained %d entries with no expiry timer", orphans)
	}
	if scheduled != 0 {
		t.Fatalf("cache retained %d entries beyond TTL, but their timers still exist", scheduled)
	}
}

Run:

go test ./core/collection -run '^TestCacheExpiryConcurrentWritesRegression$' -count=3 -v

Actual result

With v1.10.3, the three runs retained 210, 265, and 208 entries respectively, after writes had stopped for 5 seconds. Every surviving entry had no registered timer; the number of surviving entries with a registered timer was zero.

Example output:

3 seconds after writes stopped: 210/2000 entries remain
5 seconds after writes stopped: entries without timers=210, entries with timers=0
cache retained 210 entries with no expiry timer
--- FAIL: TestCacheExpiryConcurrentWritesRegression (8.00s)

The corresponding v1.8.2 runs retained 303, 320, and 273 entries, also with no registered timers. Counts vary with scheduling.

Expected result

After writes stop and the 1-second TTL plus cleanup allowance passes, entries should no longer be readable. In particular, an entry should not be left without a registered expiry timer.

Relevant implementation / suspected race

Cache.Del deletes the map entry under c.lock, then calls RemoveTimer after unlocking. SetWithExpire likewise updates the map under c.lock, then registers or moves its timer after unlocking.

One possible interleaving is:

1. An expiry callback calls Del, removes the old data, and unlocks.
2. A concurrent Set inserts new data and registers a new timer for the same key.
3. The old Del resumes and removes the newly registered timer.

Get does not independently validate an expiry deadline. Subsequent overwrites call MoveTimer because the data entry still exists; moveTask returns without creating a timer when the timer registry has no entry for the key.

This interleaving is a source-level explanation to investigate; the test above confirms the orphaned-entry condition, but does not force or instrument that exact interleaving.

Relation to #5797

#5797 describes a timer firing one wheel revolution early or late, and explicitly says tasks are not lost. This report checks the registry after writers stop and finds no timers for the surviving cache entries, so it appears distinct from that scheduling-arithmetic issue.

Impact

Users relying on TTL to stop serving stale data, release deduplication/suppression markers, or bound cache lifetime may retain stale entries until explicit invalidation, LRU eviction (if configured), or process restart.