From 30425b6d0a5daeb26f36ddbb4c44097241cbd44a Mon Sep 17 00:00:00 2001
From: fossisawesome-macbook-with-linux <fossisawesome@github.com>
Date: Fri, 3 Jul 2026 16:07:05 -0400
Subject: [PATCH] fix(jukebox): close remaining races flagged by review

- pd.mutex is now a sync.RWMutex. getStatus() holds a read lock for
  the whole call, including the blocking mpv Position()/IsPlaying()
  IPC calls, so a concurrent Skip/Clear/Set/track-switch can't close
  ActiveTrack out from under an in-flight status probe. Read lock
  still allows concurrent Status/Get requests to run in parallel.
- Queue.Get() now returns a copy of the backing slice instead of the
  slice itself, so a Get() caller can't observe a torn read if
  Shuffle/Remove/Add/Clear mutate the queue after the lock protecting
  the call is released.

Co-Authored-By: Claude <noreply@anthropic.com>
---
 core/playback/device.go | 31 +++++++++++++++----------------
 core/playback/queue.go  |  7 +++++--
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/core/playback/device.go b/core/playback/device.go
index 5e08b5d48..1b52ce15e 100644
--- a/core/playback/device.go
+++ b/core/playback/device.go
@@ -23,7 +23,7 @@ type Track interface {
 }
 
 type playbackDevice struct {
-	mutex                sync.Mutex
+	mutex                sync.RWMutex
 	serviceCtx           context.Context
 	ParentPlaybackServer PlaybackServer
 	Default              bool
@@ -62,26 +62,25 @@ func (pd *playbackDevice) getStatusLocked() DeviceStatus {
 	}
 }
 
-// getStatus snapshots device state under the lock, then performs the blocking
-// mpv IPC calls (Position/IsPlaying) after releasing it, so it doesn't block
-// other concurrent operations on the device.
+// getStatus takes a read lock for the whole call, including the blocking mpv
+// IPC calls (Position/IsPlaying). This lets concurrent Status/Get requests
+// run in parallel with each other, while still preventing a concurrent
+// mutation (Skip/Clear/Set/track-switch) from closing ActiveTrack out from
+// under the IPC calls.
 func (pd *playbackDevice) getStatus() DeviceStatus {
-	pd.mutex.Lock()
-	track := pd.ActiveTrack
-	index := pd.PlaybackQueue.Index
-	gain := pd.Gain
-	pd.mutex.Unlock()
+	pd.mutex.RLock()
+	defer pd.mutex.RUnlock()
 
 	pos := 0
 	playing := false
-	if track != nil {
-		pos = track.Position()
-		playing = track.IsPlaying()
+	if pd.ActiveTrack != nil {
+		pos = pd.ActiveTrack.Position()
+		playing = pd.ActiveTrack.IsPlaying()
 	}
 	return DeviceStatus{
-		CurrentIndex: index,
+		CurrentIndex: pd.PlaybackQueue.Index,
 		Playing:      playing,
-		Gain:         gain,
+		Gain:         pd.Gain,
 		Position:     pos,
 	}
 }
@@ -108,9 +107,9 @@ func (pd *playbackDevice) String() string {
 
 func (pd *playbackDevice) Get(ctx context.Context) (model.MediaFiles, DeviceStatus, error) {
 	log.Debug(ctx, "Processing Get action", "device", pd)
-	pd.mutex.Lock()
+	pd.mutex.RLock()
 	items := pd.PlaybackQueue.Get()
-	pd.mutex.Unlock()
+	pd.mutex.RUnlock()
 	return items, pd.getStatus(), nil
 }
 
diff --git a/core/playback/queue.go b/core/playback/queue.go
index d15eaad96..6c8a3e59d 100644
--- a/core/playback/queue.go
+++ b/core/playback/queue.go
@@ -42,9 +42,12 @@ func (pd *Queue) Current() *model.MediaFile {
 	return &pd.Items[pd.Index]
 }
 
-// returns the whole queue
+// returns a copy of the whole queue, safe for the caller to use after
+// releasing any lock protecting the Queue.
 func (pd *Queue) Get() model.MediaFiles {
-	return pd.Items
+	items := make(model.MediaFiles, len(pd.Items))
+	copy(items, pd.Items)
+	return items
 }
 
 func (pd *Queue) Size() int {