mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
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>
This commit is contained in:
parent
dfd4796e4d
commit
30425b6d0a
@ -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
|
||||
}
|
||||
|
||||
|
||||
@ -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 {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user