From 8d29f947c32343c9fa169f30ab5e7f787b03952c Mon Sep 17 00:00:00 2001 From: Vladislav Yarmak Date: Fri, 23 Feb 2024 19:28:02 +0200 Subject: [PATCH 1/2] add test for update of expired items Signed-off-by: Vladislav Yarmak --- cache_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/cache_test.go b/cache_test.go index 0c91512..2932edd 100644 --- a/cache_test.go +++ b/cache_test.go @@ -324,6 +324,36 @@ func Test_Cache_set(t *testing.T) { } }) } + + // finally, test proper expiration queue handling on expired item update. + // recreate situation when expired item gets updated + // and not auto-cleaned up yet. + c := New[string, struct{}]( + WithDisableTouchOnHit[string,struct{}](), + ) + + // insert an item and let it expire + c.Set("test", struct{}{}, 1) + time.Sleep(50*time.Millisecond) + + // update the expired item + updatedItem := c.Set("test", struct{}{}, 100*time.Millisecond) + + // eviction should not happen as we prolonged element + cl := c.OnEviction(func(_ context.Context, _ EvictionReason, item *Item[string, struct{}]){ + t.Errorf("eviction happened even though item has not expired yet: key=%s, evicted item expiresAt=%s, updated item expiresAt=%s, now=%s", + item.Key(), + item.ExpiresAt().String(), + updatedItem.ExpiresAt().String(), + time.Now().String()) + }) + // start of automatic cleanup process is delayed to allow update win the race + // and update expired before its removal + go c.Start() + + time.Sleep(90*time.Millisecond) + cl() + c.Stop() } func Test_Cache_get(t *testing.T) { From 44c3d34daabda58814edf94014c2bc508a9e54a1 Mon Sep 17 00:00:00 2001 From: Vladislav Yarmak Date: Fri, 23 Feb 2024 19:41:10 +0200 Subject: [PATCH 2/2] fix double insertion into expiration queue Signed-off-by: Vladislav Yarmak --- cache.go | 12 ++++++------ cache_test.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cache.go b/cache.go index 1ad3afb..4542e24 100644 --- a/cache.go +++ b/cache.go @@ -133,7 +133,7 @@ func (c *Cache[K, V]) set(key K, value V, ttl time.Duration) *Item[K, V] { ttl = c.options.ttl } - elem := c.get(key, false) + elem := c.get(key, false, true) if elem != nil { // update/overwrite an existing item item := elem.Value.(*Item[K, V]) @@ -176,14 +176,14 @@ func (c *Cache[K, V]) set(key K, value V, ttl time.Duration) *Item[K, V] { // It returns nil if the item is not found or is expired. // Not safe for concurrent use by multiple goroutines without additional // locking. -func (c *Cache[K, V]) get(key K, touch bool) *list.Element { +func (c *Cache[K, V]) get(key K, touch bool, includeExpired bool) *list.Element { elem := c.items.values[key] if elem == nil { return nil } item := elem.Value.(*Item[K, V]) - if item.isExpiredUnsafe() { + if !includeExpired && item.isExpiredUnsafe() { return nil } @@ -218,7 +218,7 @@ func (c *Cache[K, V]) getWithOpts(key K, lockAndLoad bool, opts ...Option[K, V]) c.items.mu.Lock() } - elem := c.get(key, !getOpts.disableTouchOnHit) + elem := c.get(key, !getOpts.disableTouchOnHit, false) if lockAndLoad { c.items.mu.Unlock() @@ -436,7 +436,7 @@ func (c *Cache[K, V]) DeleteExpired() { // If the item is not found, the method is no-op. func (c *Cache[K, V]) Touch(key K) { c.items.mu.Lock() - c.get(key, true) + c.get(key, true, false) c.items.mu.Unlock() } @@ -469,7 +469,7 @@ func (c *Cache[K, V]) Items() map[K]*Item[K, V] { items := make(map[K]*Item[K, V], len(c.items.values)) for k := range c.items.values { - item := c.get(k, false) + item := c.get(k, false, false) if item != nil { items[k] = item.Value.(*Item[K, V]) } diff --git a/cache_test.go b/cache_test.go index 2932edd..4cf83ba 100644 --- a/cache_test.go +++ b/cache_test.go @@ -404,7 +404,7 @@ func Test_Cache_get(t *testing.T) { oldItem.ttl = 0 } - elem := cache.get(c.Key, c.Touch) + elem := cache.get(c.Key, c.Touch, false) if c.Key == notFoundKey { assert.Nil(t, elem)