Skip to content

fix(ttlcache): refresh clobber, deadlocks and ttl precision - #1

Open
zze0s wants to merge 29 commits into
mainfrom
fix/ttlcache-refresh
Open

fix(ttlcache): refresh clobber, deadlocks and ttl precision#1
zze0s wants to merge 29 commits into
mainfrom
fix/ttlcache-refresh

Conversation

@zze0s

@zze0s zze0s commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What

  • fix a Get TTL refresh clobbering a concurrent Set — the refresh now re-reads under the write lock instead of writing back a stale copy
  • fix Set deadlocking against the expiration loop on a full wake channel — the loop is now driven by a coalesced signal plus an earliest-deadline field guarded by the lock, so a dropped signal can never lose or delay an expiration
  • fix reads observing expired entries — an entry past its deadline now reads as missing everywhere (Get, GetOrSet, Keys, All) instead of being returned, or resurrected by the sliding refresh, while the sweep lagged; a store or delete landing on such an entry still hands it to the deallocation callback as timed out
  • fix Close returning while the expiration goroutine was still running — it now waits for the loop, including any in-flight deallocation callback
  • fix deallocation callbacks deadlocking the cache — they run after removal, outside the lock, and may call back into the cache
  • fix short TTLs being honored at clock resolution — ttlcache now stamps and sweeps with real time.Now() and no longer uses timecache (a 200ms item in a SetDefaultTTL(10s) cache lived 2.6–5.2s, now ~201ms); Get refreshes are batched to the resolution, capped at half the item's TTL
  • replace the options builders with functional options: SetDefaultTTL, SetTimerResolution, DisableUpdateTime, SetDeallocationFunc on ttlcache, Round on timecache
  • add Keys()/All() iterators; GetKeys stays as a deprecated shim
  • fix Set over a live key silently dropping the displaced value — the deallocation callback now fires with ReasonReplaced, and GetOrSet's bool now reports whether the key was already present (it was always true before), so callers can release the loser of a store race
  • drop pre-1.23 timer workarounds, run timing tests under testing/synctest, and gate the suite on the Go 1.27 goroutineleak profile
  • add a test workflow: vet, gofmt, go test, go test -race
  • add LICENSE and a README covering the three packages with runnable examples

Why

  • adversarial review of the package surfaced lost writes, two deadlock classes, and TTLs arriving 13–26x late under a coarse default resolution
  • a second review found the follow-on liveness gaps: a blocked timeout callback let writers fill the wake channel and strand a short-TTL item behind far deadlines, reads could return or resurrect expired entries while the sweep lagged, and Close returned before the loop stopped
  • the replacement-deallocation and GetOrSet changes close a prepared-statement leak in qui's database/sql caches ahead of its migration to this module
  • the module was already on Go 1.27, so the tests and timer code can use what it provides

Breaking

  • constructing a cache with the old Options builders no longer compiles — pass the functional options to New instead: ttlcache.New(ttlcache.SetDefaultTTL(...)), timecache.New(timecache.Round(...))
  • the module requires Go 1.27
  • GetKeys is deprecated but keeps working — migrate to Keys() when convenient
  • GetOrSet/GetOrSetItem return true only when the key was already present (previously always true), and a Set over an existing key now runs the deallocation callback for the displaced value
  • Close now blocks until the expiration goroutine exits; calling it from a timeout deallocation callback would deadlock (previously it returned immediately)

Testing

  • go test -race ./... green; the timing tests are deterministic under synctest
  • new regression tests are mutation-verified: lock-holding iterators, a removed skip-guard, a never-re-arm timer, an under-lock replacement callback, and a refresh routed through the replacement path each fail their test
  • the second review's findings were reproduced first (blocked-callback scenarios), then fixed red-green: stranded wake-up, expired reads via Get/GetItem/GetKeys/All, corpse displacement reasons, and Close returning early
  • TTL accuracy measured before/after: default-TTL items unchanged (~1.001s on a 1s TTL), short explicit TTLs on time instead of seconds late
  • suite end re-checks the goroutineleak profile; the pipeline was validated against deliberately leaked caches

@zze0s zze0s added the enhancement New feature or request label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant