From fcc7a229d9d56112eda955f59e7c8de85fa17bbe Mon Sep 17 00:00:00 2001 From: AshSgDe29071999 Date: Wed, 2 Sep 2026 17:46:15 +0530 Subject: [PATCH] Call OnEvicted when Set replaces an expired item Get misses expired keys, but Set still overwrote the slot without running OnEvicted, so resources the callback was meant to release leaked. Live overwrites still do not evict. See #176 --- cache.go | 39 +++++++++++++++++++++------------------ cache_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/cache.go b/cache.go index db88d2f..71902fe 100644 --- a/cache.go +++ b/cache.go @@ -49,25 +49,15 @@ type cache struct { // (DefaultExpiration), the cache's default expiration time is used. If it is -1 // (NoExpiration), the item never expires. func (c *cache) Set(k string, x interface{}, d time.Duration) { - // "Inlining" of set - var e int64 - if d == DefaultExpiration { - d = c.defaultExpiration - } - if d > 0 { - e = time.Now().Add(d).UnixNano() - } c.mu.Lock() - c.items[k] = Item{ - Object: x, - Expiration: e, - } - // TODO: Calls to mu.Unlock are currently not deferred because defer - // adds ~200 ns (as of go1.) + ov, evicted := c.set(k, x, d) c.mu.Unlock() + if evicted { + c.onEvicted(k, ov) + } } -func (c *cache) set(k string, x interface{}, d time.Duration) { +func (c *cache) set(k string, x interface{}, d time.Duration) (interface{}, bool) { var e int64 if d == DefaultExpiration { d = c.defaultExpiration @@ -75,10 +65,19 @@ func (c *cache) set(k string, x interface{}, d time.Duration) { if d > 0 { e = time.Now().Add(d).UnixNano() } + var evictedValue interface{} + evicted := false + if c.onEvicted != nil { + if old, found := c.items[k]; found && old.Expired() { + evictedValue = old.Object + evicted = true + } + } c.items[k] = Item{ Object: x, Expiration: e, } + return evictedValue, evicted } // Add an item to the cache, replacing any existing item, using the default @@ -96,8 +95,11 @@ func (c *cache) Add(k string, x interface{}, d time.Duration) error { c.mu.Unlock() return fmt.Errorf("Item %s already exists", k) } - c.set(k, x, d) + ov, evicted := c.set(k, x, d) c.mu.Unlock() + if evicted { + c.onEvicted(k, ov) + } return nil } @@ -948,8 +950,9 @@ func (c *cache) DeleteExpired() { } // Sets an (optional) function that is called with the key and value when an -// item is evicted from the cache. (Including when it is deleted manually, but -// not when it is overwritten.) Set to nil to disable. +// item is evicted from the cache. (Including when it is deleted manually, and +// when an expired item is replaced. A live overwrite does not evict.) Set to +// nil to disable. func (c *cache) OnEvicted(f func(string, interface{})) { c.mu.Lock() c.onEvicted = f diff --git a/cache_test.go b/cache_test.go index de3e9d6..cb6d0f6 100644 --- a/cache_test.go +++ b/cache_test.go @@ -1247,6 +1247,30 @@ func TestOnEvicted(t *testing.T) { } } +func TestOnEvictedExpiredOverwrite(t *testing.T) { + tc := New(DefaultExpiration, 0) + var gotKey string + var gotVal interface{} + tc.OnEvicted(func(k string, v interface{}) { + gotKey = k + gotVal = v + }) + tc.Set("foo", "old", 1*time.Millisecond) + <-time.After(5 * time.Millisecond) + if _, found := tc.Get("foo"); found { + t.Fatal("expected expired get to miss") + } + tc.Set("foo", "new", DefaultExpiration) + if gotKey != "foo" || gotVal.(string) != "old" { + t.Fatalf("OnEvicted = (%q, %#v), want (foo, old)", gotKey, gotVal) + } + gotKey = "" + tc.Set("foo", "newer", DefaultExpiration) + if gotKey != "" { + t.Fatal("live overwrite must not call OnEvicted") + } +} + func TestCacheSerialization(t *testing.T) { tc := New(DefaultExpiration, 0) testFillAndSerialize(t, tc)