diff --git a/common/lru/basiclru.go b/common/lru/basiclru.go index 2fa26bdba3..6b123d9c61 100644 --- a/common/lru/basiclru.go +++ b/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)) diff --git a/common/lru/lru.go b/common/lru/lru.go index 8a3da5de79..f037b24311 100644 --- a/common/lru/lru.go +++ b/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) +} diff --git a/core/rawdb/eradb/eradb.go b/core/rawdb/eradb/eradb.go index 655712da99..ae19016cc6 100644 --- a/core/rawdb/eradb/eradb.go +++ b/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 }