fix(playback): submit a durability write, do not perform it

Every write goes through one connection — MaxOpenConns(1), because
SQLite has one writer — and a background pass can hold it for a long
time. The player and the queue wrote inline from paths that hold their
own mutexes, so a contended writer did not merely slow persistence
down: SetQueue blocked in LoadFile's saveState and then in
persistState, while holding q.mu and p.mu.

That is the exact shape of the report: the track changed and the
transport sat at paused, nothing appeared in the queue, and the play
button did nothing because Queue.Play waited on the same held q.mu.
Diagnosed by profiling the running app — 91% of its CPU was
BackfillLibraryDiscographies → upsertBatch, with four of its six
workers parked in sql.(*DB).conn.

Jobs now run in submission order on one goroutine per component, each
carrying its own snapshot. A job must not touch the component's fields
— it holds no lock and the state has moved on — which is why
persistTracks clones. SaveState still flushes and waits, because that
is the one caller for which the row has to exist on return.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UDCbcCZQepnpSQYJ6SxxZm
This commit is contained in:
2026-08-14 13:33:29 -04:00
co-authored by Claude Opus 5
parent dc890d1fcc
commit 878cf4b561
6 changed files with 370 additions and 52 deletions
+61
View File
@@ -0,0 +1,61 @@
package player
// The player saves its state — volume, mute, the loaded track and its
// position — from inside every path that changes any of them, and every
// one of those paths holds p.mu. The write goes through SQLite's
// single writer connection (database.DB sets MaxOpenConns(1)), which a
// background pass can hold for seconds at a time.
//
// Inline, that made LoadFile block on an unrelated backfill while
// holding p.mu, so the queue's own SetQueue — which calls it under q.mu
// — froze the transport and the queue panel with it: the track changed,
// the state stayed paused, and Play() had not been reached yet.
//
// So the write is submitted and runs in submission order on one
// goroutine, off the lock. A job must not touch p; it gets a snapshot.
// SaveState (shutdown) is the one caller that waits.
// persistQueueDepth is how many pending writes are buffered before a
// submission runs inline. Player state is written a few times per
// track, so the buffer is for a stalled writer, not for throughput.
const persistQueueDepth = 64
// writer returns the persistence goroutine's channel, starting it on
// first use.
func (p *Player) writer() chan func() {
p.persistOnce.Do(func() {
p.persistCh = make(chan func(), persistQueueDepth)
go func() {
for job := range p.persistCh {
job()
}
}()
})
return p.persistCh
}
// submitWrite queues a database write to run off the caller's lock.
// The caller may hold p.mu; the job may not touch p's state.
func (p *Player) submitWrite(job func()) {
select {
case p.writer() <- job:
default:
// The buffer is full, so the writer connection has been held
// for a long time. Running inline is the old behaviour and
// blocks the caller, but losing the state is worse: it is what
// the app reopens to.
p.logger.Warn("Player write buffer full, persisting inline")
job()
}
}
// flushWrites blocks until every write submitted so far has run.
func (p *Player) flushWrites() {
done := make(chan struct{})
p.writer() <- func() { close(done) }
<-done
}
+45 -27
View File
@@ -70,6 +70,12 @@ type Player struct {
// consumer can tell "the same second, again" from "a fresh
// reading" and reset its interpolation on both.
positionSeq uint64
// persistCh carries state writes to the single goroutine that runs
// them, so no path holds mu while waiting on SQLite's writer
// connection. See persistwriter.go.
persistOnce sync.Once
persistCh chan func()
}
// PositionInfo is the payload of the PlaybackPositionChanged event:
@@ -1180,17 +1186,25 @@ func (p *Player) buildMediaMetadata(
// State persistence
// ---------------------------------------------------------------
// SaveState persists the current player state to the database.
// This is called during shutdown to capture the final state.
// SaveState persists the current player state to the database and
// waits for the write. This is called during shutdown to capture the
// final state, which is the one case that cannot be deferred.
func (p *Player) SaveState() {
p.mu.Lock()
defer p.mu.Unlock()
p.saveState()
p.mu.Unlock()
p.flushWrites()
}
// saveState is the internal helper that writes the current player
// state to the database. Must be called with p.mu held.
// saveState snapshots the current player state and hands it to the
// persistence goroutine. Must be called with p.mu held.
//
// It does not write here: every caller holds p.mu (LoadFile, Play,
// Pause, Seek, the volume paths), the write goes through SQLite's
// single writer connection, and a background pass can hold that for
// seconds — which is how loading a track came to block the whole
// player behind a durability write. See persistwriter.go.
func (p *Player) saveState() {
if p.db == nil {
p.logger.Warn(
@@ -1215,29 +1229,29 @@ func (p *Player) saveState() {
positionSeconds := int64(p.displayPositionSecsLocked())
err := p.db.Queries.UpdatePlayerState(
p.db.Ctx,
sqlcgen.UpdatePlayerStateParams{
Volume: volume,
Muted: muted,
LastTrackPath: trackPath,
LastPositionSeconds: positionSeconds,
},
)
if err != nil {
p.logger.Error(
"Failed to save player state", "err", err,
)
return
params := sqlcgen.UpdatePlayerStateParams{
Volume: volume,
Muted: muted,
LastTrackPath: trackPath,
LastPositionSeconds: positionSeconds,
}
p.logger.Info("Player state saved",
"volume", volume,
"muted", muted,
"trackPath", trackPath,
"positionSeconds", positionSeconds,
)
p.submitWrite(func() {
if err := p.db.Queries.UpdatePlayerState(p.db.Ctx, params); err != nil {
p.logger.Error(
"Failed to save player state", "err", err,
)
return
}
p.logger.Info("Player state saved",
"volume", volume,
"muted", muted,
"trackPath", trackPath,
"positionSeconds", positionSeconds,
)
})
}
// ---------------------------------------------------------------
@@ -1253,6 +1267,10 @@ func (p *Player) RestoreState() {
}
func (p *Player) restoreStateLocked() {
// Reads back what saveState wrote, so it waits for anything still
// in flight rather than restoring the state before last.
p.flushWrites()
defer profiling.TimeOp(p.logger, "player.RestoreState")()
if p.db == nil {