close OnReplaced

common/lru/basiclru.go

@@ -22,10 +22,11 @@ package lru
 // This type is not safe for concurrent use.
 // The zero value is not valid, instances must be created using NewCache.
 type BasicLRU[K comparable, V any] struct {
-	list      *list[K]
-	items     map[K]cacheItem[K, V]
-	cap       int
-	onEvicted func(key K, value V)
+	list       *list[K]
+	items      map[K]cacheItem[K, V]
+	cap        int
+	onEvicted  func(key K, value V)
+	onReplaced func(key K, value V)
 }
 
 type cacheItem[K any, V any] struct {
@@ -51,6 +52,9 @@ func (c *BasicLRU[K, V]) Add(key K, value V) (evicted bool) {
 	item, ok := c.items[key]
 	if ok {
 		// Already exists in cache.
+		if c.onReplaced != nil {
+			c.onReplaced(key, item.value)
+		}
 		item.value = value
 		c.items[key] = item
 		c.list.moveToFront(item.elem)
@@ -152,6 +156,11 @@ func (c *BasicLRU[K, V]) OnEvicted(fn func(k K, v V)) {
 	c.onEvicted = fn
 }
 
+// OnReplaced sets a callback function to be called when an item is replaced in the cache.
+func (c *BasicLRU[K, V]) OnReplaced(fn func(k K, v V)) {
+	c.onReplaced = fn
+}
+
 // Keys returns all keys in the cache.
 func (c *BasicLRU[K, V]) Keys() []K {
 	keys := make([]K, 0, len(c.items))

common/lru/lru.go

@@ -101,3 +101,11 @@ func (c *Cache[K, V]) OnEvicted(fn func(key K, value V)) {
 
 	c.cache.OnEvicted(fn)
 }
+
+// OnReplaced sets a callback function to be called when an item is replaced in the cache.
+func (c *Cache[K, V]) OnReplaced(fn func(key K, value V)) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	c.cache.OnReplaced(fn)
+}

core/rawdb/eradb/eradb.go

@@ -27,12 +27,6 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
-/**
-* TODO:
-* - FD leak possible on cache eviction.
-* - FD leak possible on concurrent access to GetRaw*.
- */
-
 const (
 	openFileLimit = 64
 )
@@ -61,17 +55,20 @@ func New(datadir string) (*EraDatabase, error) {
 		return nil, err
 	}
 	db := &EraDatabase{datadir: datadir, cache: lru.NewCache[uint64, *era.Era](openFileLimit)}
-	db.cache.OnEvicted(func(key uint64, value *era.Era) {
-		if value == nil {
-			log.Warn("Era1 cache evicted nil value", "epoch", key)
+	// Take care to close era1 files when they are evicted or replaced
+	// in the cache to avoid leaking file descriptors.
+	closeEra := func(epoch uint64, e *era.Era) {
+		if e == nil {
+			log.Warn("Era1 cache contained nil value", "epoch", epoch)
 			return
 		}
-		// Close the era1 file when it is evicted from the cache
-		// to avoid leaks.
-		if err := value.Close(); err != nil {
-			log.Warn("Error closing era1 file", "epoch", key, "err", err)
+		if err := e.Close(); err != nil {
+			log.Warn("Error closing era1 file", "epoch", epoch, "err", err)
 		}
-	})
+
+	}
+	db.cache.OnEvicted(closeEra)
+	db.cache.OnReplaced(closeEra)
 	log.Info("Opened erastore", "datadir", datadir)
 	return db, nil
 }