call onEvict after unlock to avoid deadlocks

Author: DracoLi

cache/lru/cache.go

@@ -41,16 +41,27 @@ func NewCache[K comparable, V any](size int) *Cache[K, V] {
 
 func (c *Cache[K, V]) Put(key K, value V) {
 	c.lock.Lock()
-	defer c.lock.Unlock()
+
+	var evictedKey K
+	var evictedValue V
+	evicted := false
 
 	if c.elements.Len() == c.size {
-		oldestKey, oldestValue, _ := c.elements.Oldest()
+		oldestKey, oldestValue, ok := c.elements.Oldest()
 		c.elements.Delete(oldestKey)
-		if c.onEvict != nil {
-			c.onEvict(oldestKey, oldestValue)
+		if ok {
+			evicted = true
+			evictedKey = oldestKey
+			evictedValue = oldestValue
 		}
 	}
 	c.elements.Put(key, value)
+	c.lock.Unlock()
+
+	// unlock before calling onEvict to avoid deadlocks
+	if evicted && c.onEvict != nil {
+		c.onEvict(evictedKey, evictedValue)
+	}
 }
 
 func (c *Cache[K, V]) Get(key K) (V, bool) {
@@ -65,29 +76,38 @@ func (c *Cache[K, V]) Get(key K) (V, bool) {
 	return val, true
 }
 
-func (c *Cache[K, _]) Evict(key K) {
+func (c *Cache[K, V]) Evict(key K) {
 	c.lock.Lock()
-	defer c.lock.Unlock()
-	c.elements.Delete(key)
+
+	var evictedValue V
+	var found bool
 	if c.onEvict != nil {
-		value, _ := c.elements.Get(key)
-		c.onEvict(key, value)
+		evictedValue, found = c.elements.Get(key)
+	}
+	c.elements.Delete(key)
+	c.lock.Unlock()
+
+	if found {
+		c.onEvict(key, evictedValue)
 	}
 }
 
-func (c *Cache[_, _]) Flush() {
+func (c *Cache[K, V]) Flush() {
 	c.lock.Lock()
-	defer c.lock.Unlock()
-
-	// Call onEvict for each element before clearing
+	var evicted map[K]V
 	if c.onEvict != nil {
+		evicted = make(map[K]V, c.elements.Len())
 		iter := c.elements.NewIterator()
 		for iter.Next() {
-			c.onEvict(iter.Key(), iter.Value())
+			evicted[iter.Key()] = iter.Value()
 		}
 	}
-
 	c.elements.Clear()
+	c.lock.Unlock()
+
+	for key, value := range evicted {
+		c.onEvict(key, value)
+	}
 }
 
 func (c *Cache[_, _]) Len() int {

cache/lru/cache_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/ava-labs/avalanchego/cache"
 	"github.com/ava-labs/avalanchego/cache/cachetest"
 	"github.com/ava-labs/avalanchego/ids"
 )
@@ -25,7 +26,6 @@ func TestCacheEviction(t *testing.T) {
 func TestCacheFlushWithOnEvict(t *testing.T) {
 	c := NewCache[ids.ID, int64](2)
 
-	// Track which elements were evicted
 	evicted := make(map[ids.ID]int64)
 	c.SetOnEvict(func(key ids.ID, value int64) {
 		evicted[key] = value
@@ -48,3 +48,40 @@ func TestCachePutWithOnEvict(t *testing.T) {
 	require.Len(t, evicted, 1)
 	require.Equal(t, int64(1), evicted[ids.ID{1}])
 }
+
+func TestCacheDeadlockPrevention(t *testing.T) {
+	tests := []struct {
+		name     string
+		size     int
+		testFunc func(t *testing.T, c cache.Cacher[ids.ID, int64])
+	}{
+		{
+			name:     "Basic",
+			size:     1,
+			testFunc: cachetest.Basic,
+		},
+		{
+			name:     "Eviction",
+			size:     2,
+			testFunc: cachetest.Eviction,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := NewCache[ids.ID, int64](tt.size)
+			callbackExecuted := false
+
+			c.SetOnEvict(func(key ids.ID, value int64) {
+				// This would deadlock if the lock wasn't released before calling the callback
+				c.Evict(ids.ID{99}) // Try to delete something from the callback
+				callbackExecuted = true
+			})
+
+			tt.testFunc(t, c)
+
+			// If we get here without deadlock, the test passes
+			require.True(t, callbackExecuted, "Callback should have been executed")
+		})
+	}
+}