From 144d048e7a5258545bf7432d41de8c6a30821c60 Mon Sep 17 00:00:00 2001
From: fossisawesome-macbook-with-linux <fossisawesome@github.com>
Date: Sun, 5 Jul 2026 00:30:58 -0400
Subject: [PATCH] fix(jukebox): guard scrobbleCtx with a mutex and fall back
 client ids

scrobbleCtx is written from request-handling goroutines (Start/Skip/
Clear) and read from the async trackSwitcherGoroutine, which is a data
race on the context.Context interface value. Guard it with a RWMutex
and read it through a getScrobbleCtx() accessor.

Also fall back clientId/client to device-scoped values when the
context has none, so an empty clientId can't collide with another
player's session key in the scrobbler's playMap cache.

Addresses review feedback from gemini-code-assist on #5716.
---
 core/playback/device.go | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/core/playback/device.go b/core/playback/device.go
index 110998c92..84982d21a 100644
--- a/core/playback/device.go
+++ b/core/playback/device.go
@@ -37,10 +37,12 @@ type playbackDevice struct {
 	ActiveTrack          Track
 	startTrackSwitcher   sync.Once
 	playTracker          scrobbler.PlayTracker
+	scrobbleMu           sync.RWMutex
 	// scrobbleCtx holds the most recently seen request context (merged onto
 	// serviceCtx so it survives past the originating HTTP request), used to
 	// report playback state from the async trackSwitcherGoroutine, which has
-	// no request of its own.
+	// no request of its own. Written from request-handling goroutines, read
+	// from trackSwitcherGoroutine, so access is guarded by scrobbleMu.
 	scrobbleCtx context.Context
 }
 
@@ -274,10 +276,20 @@ func (pd *playbackDevice) isPlaying() bool {
 // request has completed.
 func (pd *playbackDevice) rememberScrobbleCtx(ctx context.Context) context.Context {
 	merged := request.AddValues(pd.serviceCtx, ctx)
+	pd.scrobbleMu.Lock()
 	pd.scrobbleCtx = merged
+	pd.scrobbleMu.Unlock()
 	return merged
 }
 
+// getScrobbleCtx returns the most recently remembered scrobble context,
+// safe to call concurrently with rememberScrobbleCtx.
+func (pd *playbackDevice) getScrobbleCtx() context.Context {
+	pd.scrobbleMu.RLock()
+	defer pd.scrobbleMu.RUnlock()
+	return pd.scrobbleCtx
+}
+
 // reportActiveTrackStarted reports to the scrobbler that the currently active
 // track started playing. Must be called after switchActiveTrackByIndex.
 func (pd *playbackDevice) reportActiveTrackStarted(ctx context.Context) {
@@ -338,6 +350,15 @@ func (pd *playbackDevice) reportPlayback(ctx context.Context, mf model.MediaFile
 	if !ok {
 		clientId = player.ID
 	}
+	// Fall back to device-scoped identifiers so an empty clientId (e.g. no
+	// jukebox request has run yet) can't collide with another player's
+	// session key in the scrobbler's playMap cache.
+	if clientId == "" {
+		clientId = "jukebox-" + pd.Name
+	}
+	if client == "" {
+		client = "Jukebox"
+	}
 	err := pd.playTracker.ReportPlayback(ctx, scrobbler.ReportPlaybackParams{
 		MediaId:      mf.ID,
 		PositionMs:   positionMs,
@@ -358,7 +379,7 @@ func (pd *playbackDevice) trackSwitcherGoroutine() {
 		case <-pd.PlaybackDone:
 			log.Debug("Track switching detected")
 			if pd.ActiveTrack != nil {
-				pd.reportActiveTrackFinished(pd.scrobbleCtx)
+				pd.reportActiveTrackFinished(pd.getScrobbleCtx())
 				pd.ActiveTrack.Close()
 				pd.ActiveTrack = nil
 			}
@@ -372,7 +393,7 @@ func (pd *playbackDevice) trackSwitcherGoroutine() {
 				}
 				if pd.ActiveTrack != nil {
 					pd.ActiveTrack.Unpause()
-					pd.reportActiveTrackStarted(pd.scrobbleCtx)
+					pd.reportActiveTrackStarted(pd.getScrobbleCtx())
 				}
 			} else {
 				log.Debug("There is no song left in the playlist. Finish.")