fix(artwork): make the pool-split worker test race-clean

CI runs the suite under -race, which the local `make test` does not, so
this only showed up there: 230 specs passed and the detector still
failed the run.

The drain-pools spec started Run and never waited for it, so pool
goroutines outlived the spec and raced the config snapshot Ginkgo
restores on cleanup. It now cancels, unparks the blocked lookups and
waits for Run to return.

Two test doubles also had to become concurrency-safe, since the spec is
the first to resolve several artists at once: fakeImageAgent's call
counters, and MockAlbumRepo.GetAll, which records the last query options
on a read path. MockDataStore's lazy accessors get the same treatment —
only MediaFile was guarded before, and two pools now reach them
concurrently. ArtworkQueue takes an unlocked helper for its internal
Artwork call, since repoMu is not reentrant.

core/artwork/agent_images_test.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync"
 
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
@@ -30,6 +31,8 @@ type fakeImageAgent struct {
 	gotAlbumName  string
 	// block, when set, holds every lookup until closed, standing in for a slow/rate-limited agent.
 	block chan struct{}
+	// mu guards the call counters: the worker resolves several items concurrently.
+	mu sync.Mutex
 }
 
 func (f *fakeImageAgent) AgentName() string { return f.name }
@@ -38,8 +41,10 @@ func (f *fakeImageAgent) GetArtistImages(_ context.Context, _, name, _ string) (
 	if f.block != nil {
 		<-f.block
 	}
+	f.mu.Lock()
 	f.artistCalls++
 	f.gotArtistName = name
+	f.mu.Unlock()
 	return f.imgs, f.err
 }
 

core/artwork/worker_test.go

@@ -631,7 +631,6 @@ var _ = Describe("Worker", func() {
 
 			// Every artist lookup blocks until released, standing in for the rate limiter.
 			block := make(chan struct{})
-			DeferCleanup(func() { close(block) })
 			artists := model.Artists{}
 			for i := range 8 {
 				artists = append(artists, model.Artist{ID: fmt.Sprintf("arx%d", i), Name: "A"})
@@ -650,8 +649,15 @@ var _ = Describe("Worker", func() {
 			})).To(Succeed())
 
 			runCtx, cancel := context.WithCancel(ctx)
-			DeferCleanup(cancel)
-			go func() { _ = w.Run(runCtx) }()
+			done := make(chan struct{})
+			go func() { defer close(done); _ = w.Run(runCtx) }()
+			// Wait for Run to return: a leaked pool goroutine outlives the spec and races the
+			// config snapshot Ginkgo restores on cleanup.
+			DeferCleanup(func() {
+				cancel()
+				close(block) // unpark the blocked lookups so the pools can unwind
+				<-done
+			})
 
 			// The album must land while every artist is still parked in the agent.
 			Eventually(func() bool {

tests/mock_album_repo.go

@@ -2,6 +2,7 @@ package tests
 
 import (
 	"errors"
+	"sync"
 	"time"
 
 	"github.com/navidrome/navidrome/model"
@@ -20,6 +21,7 @@ type MockAlbumRepo struct {
 	All                     model.Albums
 	Err                     bool
 	Options                 model.QueryOptions
+	optionsMu               sync.Mutex
 	SearchQuery             string            // last query passed to Search
 	ReassignAnnotationCalls map[string]string // prevID -> newID
 	CopyAttributesCalls     map[string]string // fromID -> toID
@@ -68,7 +70,10 @@ func (m *MockAlbumRepo) Put(al *model.Album) error {
 
 func (m *MockAlbumRepo) GetAll(qo ...model.QueryOptions) (model.Albums, error) {
 	if len(qo) > 0 {
+		// Recording the last options is a read-path write, and callers resolve concurrently.
+		m.optionsMu.Lock()
 		m.Options = qo[0]
+		m.optionsMu.Unlock()
 	}
 	if m.Err {
 		return nil, errors.New("unexpected error")

tests/mock_data_store.go

@@ -39,6 +39,8 @@ type MockDataStore struct {
 }
 
 func (db *MockDataStore) Library(ctx context.Context) model.LibraryRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedLibrary != nil {
 		return db.MockedLibrary
 	}
@@ -50,6 +52,8 @@ func (db *MockDataStore) Library(ctx context.Context) model.LibraryRepository {
 }
 
 func (db *MockDataStore) Folder(ctx context.Context) model.FolderRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedFolder != nil {
 		return db.MockedFolder
 	}
@@ -61,6 +65,8 @@ func (db *MockDataStore) Folder(ctx context.Context) model.FolderRepository {
 }
 
 func (db *MockDataStore) Tag(ctx context.Context) model.TagRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedTag != nil {
 		return db.MockedTag
 	}
@@ -72,6 +78,8 @@ func (db *MockDataStore) Tag(ctx context.Context) model.TagRepository {
 }
 
 func (db *MockDataStore) Album(ctx context.Context) model.AlbumRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedAlbum != nil {
 		return db.MockedAlbum
 	}
@@ -83,6 +91,8 @@ func (db *MockDataStore) Album(ctx context.Context) model.AlbumRepository {
 }
 
 func (db *MockDataStore) Artist(ctx context.Context) model.ArtistRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedArtist != nil {
 		return db.MockedArtist
 	}
@@ -94,11 +104,11 @@ func (db *MockDataStore) Artist(ctx context.Context) model.ArtistRepository {
 }
 
 func (db *MockDataStore) MediaFile(ctx context.Context) model.MediaFileRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.RealDS != nil && db.MockedMediaFile == nil {
 		return db.RealDS.MediaFile(ctx)
 	}
-	db.repoMu.Lock()
-	defer db.repoMu.Unlock()
 	if db.MockedMediaFile == nil {
 		db.MockedMediaFile = CreateMockMediaFileRepo()
 	}
@@ -106,6 +116,8 @@ func (db *MockDataStore) MediaFile(ctx context.Context) model.MediaFileRepositor
 }
 
 func (db *MockDataStore) Genre(ctx context.Context) model.GenreRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedGenre != nil {
 		return db.MockedGenre
 	}
@@ -117,6 +129,8 @@ func (db *MockDataStore) Genre(ctx context.Context) model.GenreRepository {
 }
 
 func (db *MockDataStore) Playlist(ctx context.Context) model.PlaylistRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedPlaylist != nil {
 		return db.MockedPlaylist
 	}
@@ -128,6 +142,8 @@ func (db *MockDataStore) Playlist(ctx context.Context) model.PlaylistRepository
 }
 
 func (db *MockDataStore) PlayQueue(ctx context.Context) model.PlayQueueRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedPlayQueue != nil {
 		return db.MockedPlayQueue
 	}
@@ -139,6 +155,8 @@ func (db *MockDataStore) PlayQueue(ctx context.Context) model.PlayQueueRepositor
 }
 
 func (db *MockDataStore) UserProps(ctx context.Context) model.UserPropsRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedUserProps != nil {
 		return db.MockedUserProps
 	}
@@ -150,6 +168,8 @@ func (db *MockDataStore) UserProps(ctx context.Context) model.UserPropsRepositor
 }
 
 func (db *MockDataStore) Property(ctx context.Context) model.PropertyRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedProperty != nil {
 		return db.MockedProperty
 	}
@@ -161,6 +181,8 @@ func (db *MockDataStore) Property(ctx context.Context) model.PropertyRepository
 }
 
 func (db *MockDataStore) Share(ctx context.Context) model.ShareRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedShare != nil {
 		return db.MockedShare
 	}
@@ -172,6 +194,8 @@ func (db *MockDataStore) Share(ctx context.Context) model.ShareRepository {
 }
 
 func (db *MockDataStore) User(ctx context.Context) model.UserRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedUser != nil {
 		return db.MockedUser
 	}
@@ -183,6 +207,8 @@ func (db *MockDataStore) User(ctx context.Context) model.UserRepository {
 }
 
 func (db *MockDataStore) Transcoding(ctx context.Context) model.TranscodingRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedTranscoding != nil {
 		return db.MockedTranscoding
 	}
@@ -194,6 +220,8 @@ func (db *MockDataStore) Transcoding(ctx context.Context) model.TranscodingRepos
 }
 
 func (db *MockDataStore) Player(ctx context.Context) model.PlayerRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedPlayer != nil {
 		return db.MockedPlayer
 	}
@@ -205,6 +233,8 @@ func (db *MockDataStore) Player(ctx context.Context) model.PlayerRepository {
 }
 
 func (db *MockDataStore) ScrobbleBuffer(ctx context.Context) model.ScrobbleBufferRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.RealDS != nil && db.MockedScrobbleBuffer == nil {
 		return db.RealDS.ScrobbleBuffer(ctx)
 	}
@@ -217,6 +247,8 @@ func (db *MockDataStore) ScrobbleBuffer(ctx context.Context) model.ScrobbleBuffe
 }
 
 func (db *MockDataStore) Scrobble(ctx context.Context) model.ScrobbleRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedScrobble != nil {
 		return db.MockedScrobble
 	}
@@ -228,6 +260,8 @@ func (db *MockDataStore) Scrobble(ctx context.Context) model.ScrobbleRepository
 }
 
 func (db *MockDataStore) Radio(ctx context.Context) model.RadioRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedRadio != nil {
 		return db.MockedRadio
 	}
@@ -239,6 +273,8 @@ func (db *MockDataStore) Radio(ctx context.Context) model.RadioRepository {
 }
 
 func (db *MockDataStore) Plugin(ctx context.Context) model.PluginRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedPlugin != nil {
 		return db.MockedPlugin
 	}
@@ -250,6 +286,14 @@ func (db *MockDataStore) Plugin(ctx context.Context) model.PluginRepository {
 }
 
 func (db *MockDataStore) Artwork(ctx context.Context) model.ArtworkRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
+	return db.artworkLocked(ctx)
+}
+
+// artworkLocked is the body of Artwork for callers already holding repoMu; repoMu is a plain
+// Mutex, so re-entering through the exported method would deadlock.
+func (db *MockDataStore) artworkLocked(ctx context.Context) model.ArtworkRepository {
 	if db.MockedArtwork != nil {
 		return db.MockedArtwork
 	}
@@ -261,6 +305,8 @@ func (db *MockDataStore) Artwork(ctx context.Context) model.ArtworkRepository {
 }
 
 func (db *MockDataStore) ArtworkQueue(ctx context.Context) model.ArtworkQueueRepository {
+	db.repoMu.Lock()
+	defer db.repoMu.Unlock()
 	if db.MockedArtworkQueue != nil {
 		return db.MockedArtworkQueue
 	}
@@ -268,7 +314,7 @@ func (db *MockDataStore) ArtworkQueue(ctx context.Context) model.ArtworkQueueRep
 		return db.RealDS.ArtworkQueue(ctx)
 	}
 	q := CreateMockArtworkQueueRepo()
-	if aw, ok := db.Artwork(ctx).(*MockArtworkRepo); ok {
+	if aw, ok := db.artworkLocked(ctx).(*MockArtworkRepo); ok {
 		q.ItemArtworkSource = aw
 	}
 	db.MockedArtworkQueue = q