Skip to content

fix: instance lifecycle — double-spawn guard, group limits, reliable Windows stop - #252

Open
Areson wants to merge 2 commits into
lordmathis:mainfrom
Areson:fix/instance-lifecycle-races
Open

fix: instance lifecycle — double-spawn guard, group limits, reliable Windows stop#252
Areson wants to merge 2 commits into
lordmathis:mainfrom
Areson:fix/instance-lifecycle-races

Conversation

@Areson

@Areson Areson commented Sep 7, 2026

Copy link
Copy Markdown

Problem

Three lifecycle bugs observed in real operation:

  1. Double-spawn / double-binding. If a surviving llama-server was still bound to the instance port (crashed or previously-started instance), starting the instance spawned a second server on the same port — two copies of the model in VRAM at once.
  2. Boot-time VRAM storms. With auto-restart, every persisted-running instance started at once, ignoring group and global running-instance limits — booting multiple large models simultaneously and spilling VRAM into system RAM.
  3. Broken "graceful" stop on Windows. The stop path attempted to deliver Ctrl-C via 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.go

  • Port-in-use guard before spawn: wait up to 15 s (500 ms polls) for the instance port to be released; if still occupied, fail with a netstat -ano | findstr <port> hint instead of double-binding.
  • Stdin pipe to the child. Stop sequence is now: close stdin (EOF is a shutdown request the backends honor) → platform interrupt → grace wait → force-kill.

pkg/instance/process_group_{unix,windows}.go

  • Platform signal helpers; Windows side simplified to its reliable behavior (see below).

pkg/manager/lifecycle.go

  • Idle reaper defers stopping an instance that has in-flight requests (the stop grace of 30 s is far too short for long generations).
  • Auto-restart boot now enforces group and global running-instance limits: starts most-recently-used first and leaves the rest stopped (they start on demand).

pkg/manager/manager.go, pkg/server/handlers.go

  • startMu serializes the evict → start → wait critical section in ensureInstanceRunning. 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 path
Verified against the actual failure modes: direct GenerateConsoleCtrlEvent to the child PID fails with Win32 87; the shared-console group event reaches the caller, not the child; AttachConsole is access-denied; ConPTY 0x03 and stdin-EOF do not fire llama.cpp's handler. setProcAttrs/signalStop are back to no-ops (matching upstream), the golang.org/x/sys direct 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.
  • Double-spawn guard exercised against a surviving process holding the port: start fails with the netstat hint, no second server in VRAM.
  • Group/global boot limits: with MaxRunningInstances / group limits set, extra auto-restart instances stay stopped and start on demand.
  • In-flight guard: idle timeout with active generation logs timed out but has N in-flight request(s); deferring idle stop.
  • Windows stop: stop completes via the force-kill path within the short grace; no 30 s hang.

Areson added 2 commits August 31, 2026 21:31
…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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant