Skip to content

Commit 5991a64

Browse files
committed
add note on deadlock
1 parent 2bbc7d8 commit 5991a64

File tree

2 files changed

+14
-10
lines changed

2 files changed

+14
-10
lines changed

cache/lru/cache.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ type Cache[K comparable, V any] struct {
2121
elements *linked.Hashmap[K, V]
2222
size int
2323

24-
// onEvict is called with the key and value of an evicted entry, if set.
24+
// onEvict is called with the key and value of an entry before eviction, if set.
2525
onEvict func(K, V)
2626
}
2727

28-
// SetOnEvict sets a callback to be called with the key and value of an evicted entry.
28+
// SetOnEvict sets a callback to be called with the key and value of an entry before eviction.
29+
// The onEvict callback is called while holding the cache lock.
30+
// Do not call any cache methods (Get, Put, Evict, Flush) from within the callback
31+
// as this will cause a deadlock. The callback should only be used for cleanup
32+
// operations like closing files or releasing resources.
2933
func (c *Cache[K, V]) SetOnEvict(cb func(K, V)) {
3034
c.lock.Lock()
3135
defer c.lock.Unlock()
@@ -44,11 +48,11 @@ func (c *Cache[K, V]) Put(key K, value V) {
4448
defer c.lock.Unlock()
4549

4650
if c.elements.Len() == c.size {
47-
oldestKey, oldestValue, _ := c.elements.Oldest()
48-
c.elements.Delete(oldestKey)
49-
if c.onEvict != nil {
51+
oldestKey, oldestValue, found := c.elements.Oldest()
52+
if c.onEvict != nil && found {
5053
c.onEvict(oldestKey, oldestValue)
5154
}
55+
c.elements.Delete(oldestKey)
5256
}
5357
c.elements.Put(key, value)
5458
}
@@ -68,11 +72,12 @@ func (c *Cache[K, V]) Get(key K) (V, bool) {
6872
func (c *Cache[K, _]) Evict(key K) {
6973
c.lock.Lock()
7074
defer c.lock.Unlock()
71-
c.elements.Delete(key)
7275
if c.onEvict != nil {
73-
value, _ := c.elements.Get(key)
74-
c.onEvict(key, value)
76+
if value, found := c.elements.Get(key); found {
77+
c.onEvict(key, value)
78+
}
7579
}
80+
c.elements.Delete(key)
7681
}
7782

7883
func (c *Cache[_, _]) Flush() {

cache/lru/cache_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ func TestCacheEviction(t *testing.T) {
2525
func TestCacheFlushWithOnEvict(t *testing.T) {
2626
c := NewCache[ids.ID, int64](2)
2727

28-
// Track which elements were evicted
2928
evicted := make(map[ids.ID]int64)
3029
c.SetOnEvict(func(key ids.ID, value int64) {
3130
evicted[key] = value
@@ -46,5 +45,5 @@ func TestCachePutWithOnEvict(t *testing.T) {
4645

4746
cachetest.Basic(t, c)
4847
require.Len(t, evicted, 1)
49-
require.Equal(t, evicted[ids.ID{1}], int64(1))
48+
require.Equal(t, int64(1), evicted[ids.ID{1}])
5049
}

0 commit comments

Comments
 (0)