diff --git a/cmd/mavgpud/runner.go b/cmd/mavgpud/runner.go index 39ce341..83a68b5 100644 --- a/cmd/mavgpud/runner.go +++ b/cmd/mavgpud/runner.go @@ -24,7 +24,12 @@ type runner struct { mu sync.Mutex cmd *exec.Cmd ready bool - http *http.Client + // yielding — stop() has sent the signal and the exit that follows is ours. + // llama-server aborts on SIGTERM (its static teardown throws, upstream + // ggml-org/llama.cpp), so a routine yield and a real crash produce the same + // "signal: aborted" and used to log identically (Vikunja #491). + yielding bool + http *http.Client } func newRunner(bin string, args []string, readyURL string) *runner { @@ -70,13 +75,18 @@ func (r *runner) start() error { if err := cmd.Start(); err != nil { return err } - r.cmd, r.ready = cmd, false + r.cmd, r.ready, r.yielding = cmd, false, false log.Printf("mavgpud: started llama-server pid=%d", cmd.Process.Pid) go func() { err := cmd.Wait() r.mu.Lock() - r.cmd, r.ready = nil, false + yielded := r.yielding + r.cmd, r.ready, r.yielding = nil, false, false r.mu.Unlock() + if yielded { + log.Printf("mavgpud: llama-server stopped, card yielded (%v)", err) + return + } log.Printf("mavgpud: llama-server exited: %v", err) }() return nil @@ -90,6 +100,10 @@ func (r *runner) stop(grace time.Duration) { r.mu.Lock() cmd := r.cmd r.ready = false + if cmd != nil && cmd.Process != nil { + // The exit that follows is ours, not a crash. + r.yielding = true + } r.mu.Unlock() if cmd == nil || cmd.Process == nil { return diff --git a/cmd/mavgpud/runner_test.go b/cmd/mavgpud/runner_test.go new file mode 100644 index 0000000..e9cae0d --- /dev/null +++ b/cmd/mavgpud/runner_test.go @@ -0,0 +1,59 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// fakeServer writes an executable standing in for llama-server: it ignores +// SIGTERM the way the real one effectively does — by dying messily rather than +// cleanly — and reports a non-zero status. +func fakeServer(t *testing.T, body string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "fake-llama-server") + if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body+"\n"), 0o755); err != nil { + t.Fatal(err) + } + return path +} + +// A deliberate stop is a yield, and the log has to say so. +// +// llama-server aborts inside its own static teardown on SIGTERM, so the exit +// status of a routine yield is identical to that of a real crash. Reading the +// mavgpud log, the two were indistinguishable (Vikunja #491). +func TestStopMarksTheExitAsAYield(t *testing.T) { + r := newRunner(fakeServer(t, "while : ; do sleep 1 ; done"), nil, "") + if err := r.start(); err != nil { + t.Fatalf("start: %v", err) + } + r.mu.Lock() + if r.yielding { + t.Error("a freshly started server is already marked as yielding") + } + r.mu.Unlock() + + r.stop(2 * time.Second) + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if !r.running() { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("the child outlived stop") +} + +// Stopping when nothing is running must not arm the flag for the next child. +// The next exit after that would be a real crash logged as a yield. +func TestStopWithNoChildDoesNotArmTheFlag(t *testing.T) { + r := newRunner("/nonexistent", nil, "") + r.stop(10 * time.Millisecond) + r.mu.Lock() + defer r.mu.Unlock() + if r.yielding { + t.Error("stop armed the yield flag with no child running") + } +} diff --git a/deploy/mavgpud.service b/deploy/mavgpud.service index b200baa..dc0d6f2 100644 --- a/deploy/mavgpud.service +++ b/deploy/mavgpud.service @@ -19,6 +19,10 @@ RestartSec=5 # llama-server on SIGTERM, so give it longer than stop_grace to do that. KillSignal=SIGTERM TimeoutStopSec=60 +# llama-server aborts inside its own static teardown on SIGTERM, so every +# routine yield used to write a multi-gigabyte core into systemd-coredump +# (Vikunja #491). Yielding is meant to happen several times a day. +LimitCORE=0 [Install] WantedBy=default.target