mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-12 20:37:12 +02:00
fix(jukebox): bound mpv IPC calls with a timeout to prevent permanent hangs
mpvipc.Connection.Call/Get/Set block on an unbuffered channel with no timeout of their own. A single dropped IPC response (observed on Windows) leaves the waiting goroutine blocked forever, wedging the HTTP handler and every subsequent request on that device permanently (mpv itself keeps running fine). Wrap every mpv IPC call in track.go with a 10s timeout so a lost response fails one command instead of hanging indefinitely. Fixes #5710
This commit is contained in:
parent
4f6afbbe67
commit
6cf115c4e1
1 changed files with 70 additions and 7 deletions
|
|
@ -16,6 +16,14 @@ import (
|
|||
"github.com/navidrome/navidrome/model"
|
||||
)
|
||||
|
||||
// ipcTimeout bounds how long we wait for a response to an mpv IPC command.
|
||||
// mpvipc.Connection.Call/Get/Set block on an unbuffered channel with no
|
||||
// timeout of their own, and a single dropped response (observed on Windows)
|
||||
// can wedge that channel forever, hanging the calling goroutine (and, in turn,
|
||||
// the HTTP request) permanently. Wrapping every call with a timeout turns
|
||||
// that permanent hang into a bounded failure. See #5710.
|
||||
const ipcTimeout = 10 * time.Second
|
||||
|
||||
type MpvTrack struct {
|
||||
MediaFile model.MediaFile
|
||||
PlaybackDone chan bool
|
||||
|
|
@ -25,6 +33,61 @@ type MpvTrack struct {
|
|||
CloseCalled bool
|
||||
}
|
||||
|
||||
// errIPCTimeout is returned when an mpv IPC call does not respond within
|
||||
// ipcTimeout. The underlying goroutine making the call is leaked (it may
|
||||
// still be blocked forever), but the caller is freed to fail fast instead of
|
||||
// hanging.
|
||||
var errIPCTimeout = fmt.Errorf("mpv IPC call timed out after %s", ipcTimeout)
|
||||
|
||||
func (t *MpvTrack) getWithTimeout(property string) (interface{}, error) {
|
||||
type result struct {
|
||||
val interface{}
|
||||
err error
|
||||
}
|
||||
ch := make(chan result, 1)
|
||||
go func() {
|
||||
val, err := t.Conn.Get(property)
|
||||
ch <- result{val, err}
|
||||
}()
|
||||
select {
|
||||
case res := <-ch:
|
||||
return res.val, res.err
|
||||
case <-time.After(ipcTimeout):
|
||||
return nil, errIPCTimeout
|
||||
}
|
||||
}
|
||||
|
||||
func (t *MpvTrack) setWithTimeout(property string, value interface{}) error {
|
||||
ch := make(chan error, 1)
|
||||
go func() {
|
||||
ch <- t.Conn.Set(property, value)
|
||||
}()
|
||||
select {
|
||||
case err := <-ch:
|
||||
return err
|
||||
case <-time.After(ipcTimeout):
|
||||
return errIPCTimeout
|
||||
}
|
||||
}
|
||||
|
||||
func (t *MpvTrack) callWithTimeout(args ...interface{}) (interface{}, error) {
|
||||
type result struct {
|
||||
val interface{}
|
||||
err error
|
||||
}
|
||||
ch := make(chan result, 1)
|
||||
go func() {
|
||||
val, err := t.Conn.Call(args...)
|
||||
ch <- result{val, err}
|
||||
}()
|
||||
select {
|
||||
case res := <-ch:
|
||||
return res.val, res.err
|
||||
case <-time.After(ipcTimeout):
|
||||
return nil, errIPCTimeout
|
||||
}
|
||||
}
|
||||
|
||||
func NewTrack(ctx context.Context, playbackDoneChannel chan bool, deviceName string, mf model.MediaFile) (*MpvTrack, error) {
|
||||
log.Debug("Loading track", "trackPath", mf.Path, "mediaType", mf.ContentType())
|
||||
|
||||
|
|
@ -84,7 +147,7 @@ func (t *MpvTrack) SetVolume(value float32) {
|
|||
log.Debug("Setting volume", "volume", value, "track", t)
|
||||
vol := int(value * 100)
|
||||
|
||||
err := t.Conn.Set("volume", vol)
|
||||
err := t.setWithTimeout("volume", vol)
|
||||
if err != nil {
|
||||
log.Error("Error setting volume", "volume", value, "track", t, err)
|
||||
}
|
||||
|
|
@ -92,7 +155,7 @@ func (t *MpvTrack) SetVolume(value float32) {
|
|||
|
||||
func (t *MpvTrack) Unpause() {
|
||||
log.Debug("Unpausing track", "track", t)
|
||||
err := t.Conn.Set("pause", false)
|
||||
err := t.setWithTimeout("pause", false)
|
||||
if err != nil {
|
||||
log.Error("Error unpausing track", "track", t, err)
|
||||
}
|
||||
|
|
@ -100,7 +163,7 @@ func (t *MpvTrack) Unpause() {
|
|||
|
||||
func (t *MpvTrack) Pause() {
|
||||
log.Debug("Pausing track", "track", t)
|
||||
err := t.Conn.Set("pause", true)
|
||||
err := t.setWithTimeout("pause", true)
|
||||
if err != nil {
|
||||
log.Error("Error pausing track", "track", t, err)
|
||||
}
|
||||
|
|
@ -112,7 +175,7 @@ func (t *MpvTrack) Close() {
|
|||
// trying to shutdown mpv process using socket
|
||||
if t.isSocketFilePresent() {
|
||||
log.Debug("sending shutdown command")
|
||||
_, err := t.Conn.Call("quit")
|
||||
_, err := t.callWithTimeout("quit")
|
||||
if err != nil {
|
||||
log.Warn("Error sending quit command to mpv-ipc socket", err)
|
||||
|
||||
|
|
@ -146,7 +209,7 @@ func (t *MpvTrack) isSocketFilePresent() bool {
|
|||
func (t *MpvTrack) Position() int {
|
||||
retryCount := 0
|
||||
for {
|
||||
position, err := t.Conn.Get("time-pos")
|
||||
position, err := t.getWithTimeout("time-pos")
|
||||
if err != nil && err.Error() == "mpv error: property unavailable" {
|
||||
retryCount += 1
|
||||
log.Debug("Got mpv error, retrying...", "retries", retryCount, err)
|
||||
|
|
@ -179,7 +242,7 @@ func (t *MpvTrack) SetPosition(offset int) error {
|
|||
log.Debug("No position difference, skipping operation", "track", t)
|
||||
return nil
|
||||
}
|
||||
err := t.Conn.Set("time-pos", float64(offset))
|
||||
err := t.setWithTimeout("time-pos", float64(offset))
|
||||
if err != nil {
|
||||
log.Error("Could not set the position in track", "track", t, "offset", offset, err)
|
||||
return err
|
||||
|
|
@ -189,7 +252,7 @@ func (t *MpvTrack) SetPosition(offset int) error {
|
|||
|
||||
func (t *MpvTrack) IsPlaying() bool {
|
||||
log.Debug("Checking if track is playing", "track", t)
|
||||
pausing, err := t.Conn.Get("pause")
|
||||
pausing, err := t.getWithTimeout("pause")
|
||||
if err != nil {
|
||||
log.Error("Problem getting paused status", "track", t, err)
|
||||
return false
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue