chore: bump go-processmanager to pick up the concurrent-Run fix#11004
Merged
Conversation
Picks up mudler/go-processmanager#7, which closes the check-then-act window in Run's "already started" guard. Concurrent Run calls on one handle could each observe a nil p.proc, each start a process and each launch a monitor goroutine; every monitor does `defer close(p.done)` against a channel created once in New, so the second monitor to see its process exit panicked on close of a closed channel. The same window raced on p.proc itself. LocalAI does not hit this today: the only New/Run pair (pkg/model/process.go:178-191) runs a freshly created handle, and core/services/worker/supervisor.go only ever calls Stop on handles it holds. The bump is defensive, and keeps the dependency from drifting further behind a fix in the process lifecycle we rely on. No exported signature changes upstream; ErrProcessAlreadyRun is additive and keeps the historical "command already started" prefix, so any caller matching on that text is unaffected. Nothing in this repo matches it. Verified: go build ./core/... ./pkg/... clean; go vet clean; go test -race ./pkg/model/ ./core/services/worker/ both ok. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Bash] [Edit]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Picks up mudler/go-processmanager#7.
What it fixes upstream
Run's "already started" guard was a check-then-act onp.procwith no lock. ConcurrentRuncalls on the same handle could each observe a nilp.proc, each start a process, and each launch a monitor goroutine. Every monitor doesdefer close(p.done)against a channel created once inNewand never recreated, so the second monitor to see its process exit panicked with close of closed channel. The same window raced onp.procitself, written unsynchronized while the previous monitor, itsreapChildrenchild andreleasewere reading it.Same class as the PID race picked up in the previous bump (#6), one field over.
Why bump now, given LocalAI does not hit it
This is defensive rather than a fix for an observed failure, and the PR should be judged as such:
New/Runpair ispkg/model/process.go:178-191, which runs a freshly created handlecore/services/worker/supervisor.goonly ever callsStopon handles it holds; it never callsRunStoppattern exists anywhere in the repoSo no current call site can open the window. The value is not drifting further behind fixes in a process lifecycle we depend on, while the delta is still one line and reviewable.
Compatibility
Upstream reports no exported signature changes.
ErrProcessAlreadyRunis additive, and its message keeps the historicalcommand already startedprefix so callers matching on that text keep working. Grepped this repo includingtests/e2e/— nothing matches that text, so even the text-compat path is unused here.Verification
-racespecifically, since the upstream change is a race fix — a plaingo testwould not exercise what this bump is for. Diff isgo.mod+go.sumonly.Note for the maintainer
This stays on a pseudo-version because upstream has no tag past
v0.1.1(which is PR #4). Master was already on a#6pseudo-version, so this is consistent with what is there rather than a new pattern. If you would rather cut av0.1.2on go-processmanager, this PR can be re-pointed at the tag instead.