fix: instance lifecycle — double-spawn guard, group limits, reliable Windows stop - #252
Open
Areson wants to merge 2 commits into
Open
fix: instance lifecycle — double-spawn guard, group limits, reliable Windows stop#252Areson wants to merge 2 commits into
Areson wants to merge 2 commits into
Conversation
…its, safe Windows stop Root causes of orphaned llama-server processes, VRAM spillover, and 503 'Loading model' bursts: - Stale monitorProcess goroutines from a superseded start could overwrite status and close a newer monitor's done channel, re-triggering spawns. Guarded with per-start generation tokens (myCmd/myDone). - Group quota (evict-then-start) was non-atomic; CountRunningInGroup was checked separately from the actual start. Serialized quota-check -> evict -> start -> WaitForHealthy under a handler mutex. - Instances were marked Running before the model actually served, so clients hit the 503 load window. - autoStartInstances() did not respect group_limits / max_running_instances ordering; now MRU-sorted and limit-aware. - Idle timeout could stop an instance with in-flight requests; now skips those. - Added a port-in-use guard before spawn (wait instead of double-binding). - Windows stop path: drain in-flight, close stdin, best-effort CTRL_C via GenerateConsoleCtrlEvent, short grace, then TerminateProcess. (Ctrl-C delivery to children is unreliable in a service session; force-kill kept as the reliable final step.) Built with Go 1.27.0 (CGO, mattn/go-sqlite3) + golang.org/x/sys v0.47.0. Smoke-tested: on-demand start OK, stop 30s -> ~5s, port-held race yields exactly one server.
GenerateConsoleCtrlEvent could not be delivered to the child in a service session (verified: direct event to child PID fails with Win32 87; shared-console group event reaches the caller not the child; AttachConsole is access-denied; ConPTY 0x03 and stdin-EOF do not fire the handler). Keep the reliable path: close stdin, short grace, force-kill. - process_group_windows.go: setProcAttrs and signalStop back to no-ops (matching the original), dropping the golang.org/x/sys/windows import. - process.go: stop-path comments now reflect that the Windows stop is the force-kill, not Ctrl-C. - go mod tidy: x/sys is no longer a direct dep (stays indirect via other libs; already in base go.mod).
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.
Problem
Three lifecycle bugs observed in real operation:
GenerateConsoleCtrlEvent, which cannot reach the child in a service session, so the graceful path was misleading and stop behavior was effectively a force-kill with a 30 s grace.Changes
pkg/instance/process.gonetstat -ano | findstr <port>hint instead of double-binding.pkg/instance/process_group_{unix,windows}.gopkg/manager/lifecycle.gopkg/manager/manager.go,pkg/server/handlers.gostartMuserializes the evict → start → wait critical section inensureInstanceRunning. Group quotas are check-then-act, so concurrent requests could otherwise each see room (or evict each other's LRU) and start two models in the same group; the lock also sequences VRAM-heavy model loads on a shared GPU.c13238a— drop the non-functional Windows Ctrl-C pathVerified against the actual failure modes: direct
GenerateConsoleCtrlEventto the child PID fails with Win32 87; the shared-console group event reaches the caller, not the child;AttachConsoleis access-denied; ConPTY0x03and stdin-EOF do not fire llama.cpp's handler.setProcAttrs/signalStopare back to no-ops (matching upstream), thegolang.org/x/sysdirect dependency is dropped, and the stop-path comments now state honestly that on Windows the force-kill is the reliable stop (with a shorter grace there).Verification
go build ./...,go test ./...pass on Windows.MaxRunningInstances/ group limits set, extra auto-restart instances stay stopped and start on demand.timed out but has N in-flight request(s); deferring idle stop.