github-starred/ollama
2
0
Fork 0
mirror of https://github.com/ollama/ollama.git synced 2026-07-16 05:05:37 -05:00
Code Issues 16.9k Packages Projects Releases 122 Wiki Activity

[PR #15425] fix: add mutex to protect StatusWriter.LastErrMsg from data race #15153

New Issue
Open
opened 2026-04-13 01:11:28 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-13 01:11:28 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/ollama/ollama/pull/15425
Author: @sjhddh
Created: 4/8/2026
Status: 🔄 Open

Base: main ← Head: fix/status-writer-race

📝 Commits (1)

  • 8ed4d83 fix: add mutex to protect StatusWriter.LastErrMsg from data race

📊 Changes

3 files changed (+85 additions, -14 deletions)

View changed files

📝 llm/server.go (+14 -14)
📝 llm/status.go (+18 -0)
➕ llm/status_test.go (+53 -0)

📄 Description

Summary

StatusWriter.LastErrMsg in llm/status.go is accessed by multiple goroutines without synchronization, causing a data race detectable with go test -race ./llm/.

Write goroutine: StatusWriter.Write() — called from the stderr-reading goroutine spawned in newLlmServer.

Read goroutines (concurrent):

  • server.go:~301 — startup error path (main goroutine)
  • server.go:~315–320 — reaper go func() that calls cmd.Wait() and also writes LastErrMsg at line 318 to replace the error text
  • server.go:~1279–1280 — getServerStatus() called from the polling loop
  • server.go:~1380–1388 — waitUntilRunning() stall-timeout and process-gone checks
  • server.go:~1698–1699 — generate-loop scanner error handler

Any of these goroutines can run concurrently with the stderr goroutine calling Write(), and the reaper goroutine can also race with every reader.

Changes

  • Add mu sync.Mutex to StatusWriter (zero-value ready, no constructor change needed).
  • Err() string — safe getter; replaces all direct .LastErrMsg reads in server.go.
  • SetErr(string) — safe setter; replaces the one direct write in the reaper goroutine.
  • Write() holds mu while updating LastErrMsg.
  • llm/status_test.go — two tests: a concurrent write/read stress test (passes cleanly under -race) and a SetErr round-trip test.

Test plan

  • go test -race ./llm/ -run TestStatusWriterConcurrency — must pass with no race reports
  • go test -race ./llm/ -run TestStatusWriterSetErr — must pass
  • go build ./... — no compile errors
  • Existing llm tests continue to pass

Notes

LastErrMsg is kept exported to avoid a larger refactor; all external access now goes through Err()/SetErr(). Using sync.Mutex rather than atomic.Value keeps the change minimal and idiomatic for a string field that is infrequently written.

🤖 Generated with Claude Code

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/ollama/ollama/pull/15425 **Author:** [@sjhddh](https://github.com/sjhddh) **Created:** 4/8/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/status-writer-race` --- ### 📝 Commits (1) - [`8ed4d83`](https://github.com/ollama/ollama/commit/8ed4d836e416c4b2f4f284c7aecf5552edb2f424) fix: add mutex to protect StatusWriter.LastErrMsg from data race ### 📊 Changes **3 files changed** (+85 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `llm/server.go` (+14 -14) 📝 `llm/status.go` (+18 -0) ➕ `llm/status_test.go` (+53 -0) </details> ### 📄 Description ## Summary `StatusWriter.LastErrMsg` in `llm/status.go` is accessed by multiple goroutines without synchronization, causing a data race detectable with `go test -race ./llm/`. **Write goroutine:** `StatusWriter.Write()` — called from the stderr-reading goroutine spawned in `newLlmServer`. **Read goroutines (concurrent):** - `server.go:~301` — startup error path (main goroutine) - `server.go:~315–320` — reaper `go func()` that calls `cmd.Wait()` and also *writes* `LastErrMsg` at line 318 to replace the error text - `server.go:~1279–1280` — `getServerStatus()` called from the polling loop - `server.go:~1380–1388` — `waitUntilRunning()` stall-timeout and process-gone checks - `server.go:~1698–1699` — generate-loop scanner error handler Any of these goroutines can run concurrently with the stderr goroutine calling `Write()`, and the reaper goroutine can also race with every reader. ## Changes - Add `mu sync.Mutex` to `StatusWriter` (zero-value ready, no constructor change needed). - `Err() string` — safe getter; replaces all direct `.LastErrMsg` reads in `server.go`. - `SetErr(string)` — safe setter; replaces the one direct write in the reaper goroutine. - `Write()` holds `mu` while updating `LastErrMsg`. - `llm/status_test.go` — two tests: a concurrent write/read stress test (passes cleanly under `-race`) and a `SetErr` round-trip test. ## Test plan - [ ] `go test -race ./llm/ -run TestStatusWriterConcurrency` — must pass with no race reports - [ ] `go test -race ./llm/ -run TestStatusWriterSetErr` — must pass - [ ] `go build ./...` — no compile errors - [ ] Existing `llm` tests continue to pass ## Notes `LastErrMsg` is kept exported to avoid a larger refactor; all external access now goes through `Err()`/`SetErr()`. Using `sync.Mutex` rather than `atomic.Value` keeps the change minimal and idiomatic for a string field that is infrequently written. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-13 01:11:28 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
amd
api
app
bug
build
cli
cloud
compatibility
context-length
create
docker
documentation
embeddings
feature request
feedback wanted
good first issue
gpt-oss
gpu
harmony
help wanted
image
install
intel
js
launch
linux
macos
memory
mlx
model
needs more info
networking
nvidia
ollama.com
performance
pull-request
Mirrored from GitHub Pull Request
 python
question
registry
rendering
thinking
tools
top
vulkan
windows
wsl
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/ollama#15153
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?