From b5d373f340f4b4f946446d3dab94c123365e0121 Mon Sep 17 00:00:00 2001 From: Daniel Hiltgen Date: Tue, 8 Sep 2026 13:13:59 -0700 Subject: [PATCH] fix data races in progress and sched (#18319) progress: join the render loop in stop and do the final writes after the goroutine exits, so Stop/StopAndClear cannot race an in-flight render on the shared bufio.Writer. sched: read the unload-mutable fields in runnerRef.LogValue only under a successful refMu.TryLock and omit them when contended, since slog resolves it on goroutines that may already hold refMu. --- progress/progress.go | 11 +++++++++-- server/sched.go | 28 +++++++++++++++++----------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/progress/progress.go b/progress/progress.go index fa3910100..b14f7896d 100644 --- a/progress/progress.go +++ b/progress/progress.go @@ -32,10 +32,13 @@ type Progress struct { stopOnce sync.Once // done is closed to tell the render loop to exit. done chan struct{} + // loopDone is closed by the render loop when it exits; stop waits on it so + // no in-flight render can race the writes in Stop and StopAndClear. + loopDone chan struct{} } func NewProgress(w io.Writer) *Progress { - p := &Progress{w: bufio.NewWriter(w), done: make(chan struct{})} + p := &Progress{w: bufio.NewWriter(w), done: make(chan struct{}), loopDone: make(chan struct{})} go p.start() return p } @@ -49,6 +52,9 @@ func (p *Progress) stop() (bool, int) { stopped = true }) + // Wait for the render loop to exit so no render can race the writes below. + <-p.loopDone + p.mu.Lock() defer p.mu.Unlock() @@ -74,12 +80,12 @@ func (p *Progress) Stop() bool { } func (p *Progress) StopAndClear() bool { + stopped, pos := p.stop() defer p.w.Flush() fmt.Fprint(p.w, "\033[?25l") defer fmt.Fprint(p.w, "\033[?25h") - stopped, pos := p.stop() if stopped { // clear all progress lines for i := range pos { @@ -144,6 +150,7 @@ func (p *Progress) renderLocked() { func (p *Progress) start() { ticker := time.NewTicker(100 * time.Millisecond) defer ticker.Stop() + defer close(p.loopDone) for { select { diff --git a/server/sched.go b/server/sched.go index ade914353..c1a95c339 100644 --- a/server/sched.go +++ b/server/sched.go @@ -1498,6 +1498,9 @@ func (s *Scheduler) waitForVRAMRecovery(runner *runnerRef, runners []ml.Filtered return finished } +// LogValue may run from goroutines that already hold refMu (see the scheduler +// debug logs), so the unload-mutable fields are read only under TryLock and +// omitted when the lock is contended. func (runner *runnerRef) LogValue() slog.Value { if runner == nil { return slog.StringValue("nil") @@ -1507,24 +1510,27 @@ func (runner *runnerRef) LogValue() slog.Value { modelID = runner.modelKey } attrs := []slog.Attr{} - if runner.model != nil { - attrs = append(attrs, slog.String("name", runner.model.Name)) - } - if len(runner.gpus) > 0 { - attrs = append(attrs, - slog.Any("inference", runner.gpus), - ) + if runner.refMu.TryLock() { + if runner.model != nil { + attrs = append(attrs, slog.String("name", runner.model.Name)) + } + if len(runner.gpus) > 0 { + attrs = append(attrs, + slog.Any("inference", slices.Clone(runner.gpus)), + ) + } + attrs = append(attrs, slog.Int("pid", runner.pid)) + if runner.Options != nil { + attrs = append(attrs, slog.Int("num_ctx", runner.Options.NumCtx)) + } + runner.refMu.Unlock() } attrs = append(attrs, slog.String("size", format.HumanBytes2(runner.totalSize)), slog.String("vram", format.HumanBytes2(runner.vramSize)), slog.Int("parallel", runner.numParallel), - slog.Int("pid", runner.pid), slog.String("model", modelID), ) - if runner.Options != nil { - attrs = append(attrs, slog.Int("num_ctx", runner.Options.NumCtx)) - } return slog.GroupValue(attrs...) }