From 26c0178ce4118d4401549b2c6beb7223c1ca074c Mon Sep 17 00:00:00 2001
From: kasey <489222+kasey@users.noreply.github.com>
Date: Tue, 4 Jun 2024 17:08:06 -0500
Subject: [PATCH] always close cache warm chan to prevent blocking (#14080)

* always close cache warm chan to prevent blocking

* test that waitForCache does not block

* combine defers to reduce cognitive overhead

* lint

---------

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>
---
 beacon-chain/db/filesystem/pruner.go      | 12 +++++----
 beacon-chain/db/filesystem/pruner_test.go | 31 +++++++++++++++++++++++
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/beacon-chain/db/filesystem/pruner.go b/beacon-chain/db/filesystem/pruner.go
index 9c5be29842eb..5ac4ac50d6fa 100644
--- a/beacon-chain/db/filesystem/pruner.go
+++ b/beacon-chain/db/filesystem/pruner.go
@@ -92,14 +92,16 @@ func windowMin(latest, offset primitives.Slot) primitives.Slot {
 
 func (p *blobPruner) warmCache() error {
 	p.Lock()
-	defer p.Unlock()
+	defer func() {
+		if !p.warmed {
+			p.warmed = true
+			close(p.cacheReady)
+		}
+		p.Unlock()
+	}()
 	if err := p.prune(0); err != nil {
 		return err
 	}
-	if !p.warmed {
-		p.warmed = true
-		close(p.cacheReady)
-	}
 	return nil
 }
 
diff --git a/beacon-chain/db/filesystem/pruner_test.go b/beacon-chain/db/filesystem/pruner_test.go
index ad7dace8f873..3241350dccdc 100644
--- a/beacon-chain/db/filesystem/pruner_test.go
+++ b/beacon-chain/db/filesystem/pruner_test.go
@@ -2,16 +2,19 @@ package filesystem
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"math"
 	"os"
 	"path"
 	"sort"
 	"testing"
+	"time"
 
 	"github.com/prysmaticlabs/prysm/v5/beacon-chain/verification"
 	fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
+	"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
 	"github.com/prysmaticlabs/prysm/v5/testing/require"
 	"github.com/prysmaticlabs/prysm/v5/testing/util"
 	"github.com/spf13/afero"
@@ -34,6 +37,34 @@ func TestTryPruneDir_CachedNotExpired(t *testing.T) {
 	require.Equal(t, 0, pruned)
 }
 
+func TestCacheWarmFail(t *testing.T) {
+	fs := afero.NewMemMapFs()
+	n := blobNamer{root: bytesutil.ToBytes32([]byte("derp")), index: 0}
+	bp := n.path()
+	mkdir := path.Dir(bp)
+	require.NoError(t, fs.MkdirAll(mkdir, directoryPermissions))
+
+	// Create an empty blob index in the fs by touching the file at a seemingly valid path.
+	fi, err := fs.Create(bp)
+	require.NoError(t, err)
+	require.NoError(t, fi.Close())
+
+	// Cache warm should fail due to the unexpected EOF.
+	pr, err := newBlobPruner(fs, 0)
+	require.NoError(t, err)
+	require.ErrorIs(t, pr.warmCache(), errPruningFailures)
+
+	// The cache warm has finished, so calling waitForCache with a super short deadline
+	// should not block or hit the context deadline.
+	ctx := context.Background()
+	ctx, cancel := context.WithDeadline(ctx, time.Now().Add(1*time.Millisecond))
+	defer cancel()
+	c, err := pr.waitForCache(ctx)
+	// We will get an error and a nil value for the cache if we hit the deadline.
+	require.NoError(t, err)
+	require.NotNil(t, c)
+}
+
 func TestTryPruneDir_CachedExpired(t *testing.T) {
 	t.Run("empty directory", func(t *testing.T) {
 		fs := afero.NewMemMapFs()