mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
fix(daemon): exit on SIGTERM instead of deadlocking on its own wait group (#625)
Start registers startCluster as a member of shutdownWg and then waits on that group. startCluster's tail waited on the same group, so it could not return until the counter it was waiting on reached zero, and the counter could not reach zero until it returned. The SIGTERM handler completed and logged "Shutdown complete" in ~20ms, but the process never exited: it sat until systemd's TimeoutStopSec=15 and was SIGKILLed, on every stop. Because the member stop chain is serialized, that 15s also gated viperblock, predastore and nats, which each stop in milliseconds once reached. The duplicate setupShutdown in the same tail registered a second signal.Notify and a second handler goroutine, which is where the duplicated shutdown log lines and NATS unsubscribe pass came from. Drop both. Start already installs the handler and owns the single wait. The invariant test pins each helper to one call site, since a second call site is the defect rather than a symptom of it.
This commit is contained in:
@@ -1853,9 +1853,10 @@ func (d *Daemon) startCluster() error {
|
||||
d.ready.Store(true)
|
||||
slog.Info("Daemon fully initialized", "node", d.node, "startupTime", time.Since(d.startTime).Round(time.Second))
|
||||
|
||||
// Return once bootstrap is done. Start already installed the signal handler
|
||||
// and owns the single awaitShutdown; waiting here would block on the wait
|
||||
// group this goroutine is itself a member of, which never resolves.
|
||||
d.setupReload()
|
||||
d.setupShutdown()
|
||||
d.awaitShutdown()
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
package daemon
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// awaitShutdown blocks on shutdownWg, and every goroutine registered with
|
||||
// shutdownWg.Go is a counted member of it. Calling it from inside such a
|
||||
// goroutine deadlocks: the member cannot decrement until it returns, and it
|
||||
// cannot return until the counter it is waiting on reaches zero.
|
||||
//
|
||||
// startCluster runs as a shutdownWg member and once called awaitShutdown at its
|
||||
// tail, so the daemon never exited on SIGTERM and was SIGKILLed at
|
||||
// TimeoutStopSec on every stop. Pinning both helpers to a single call site each
|
||||
// keeps that shape from returning — a second call site is the defect.
|
||||
func TestShutdownHelpersHaveASingleCallSite(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
helper string
|
||||
caller string
|
||||
why string
|
||||
}{
|
||||
{
|
||||
helper: "awaitShutdown",
|
||||
caller: "Start",
|
||||
why: "blocks on shutdownWg; a second caller is almost certainly a member of that group and will deadlock",
|
||||
},
|
||||
{
|
||||
helper: "setupShutdown",
|
||||
caller: "Start",
|
||||
why: "each call registers its own signal.Notify and shutdownWg goroutine, so a second caller duplicates the handler",
|
||||
},
|
||||
} {
|
||||
t.Run(tc.helper, func(t *testing.T) {
|
||||
sites := callSites(t, tc.helper)
|
||||
if len(sites) == 1 && sites[0].caller == tc.caller {
|
||||
return
|
||||
}
|
||||
|
||||
var b strings.Builder
|
||||
fmt.Fprintf(&b, "%s must be called exactly once, from %s — %s\n",
|
||||
tc.helper, tc.caller, tc.why)
|
||||
for _, s := range sites {
|
||||
fmt.Fprintf(&b, " %s:%d: called from %s\n", s.file, s.line, s.caller)
|
||||
}
|
||||
t.Fatal(b.String())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type callSite struct {
|
||||
file string
|
||||
line int
|
||||
caller string
|
||||
}
|
||||
|
||||
// callSites reports every call to a method on the daemon receiver, with the
|
||||
// enclosing function of each.
|
||||
func callSites(t *testing.T, method string) []callSite {
|
||||
t.Helper()
|
||||
|
||||
entries, err := os.ReadDir(".")
|
||||
if err != nil {
|
||||
t.Fatalf("read package dir: %v", err)
|
||||
}
|
||||
|
||||
var found []callSite
|
||||
for _, entry := range entries {
|
||||
name := entry.Name()
|
||||
if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
|
||||
continue
|
||||
}
|
||||
|
||||
fset := token.NewFileSet()
|
||||
file, perr := parser.ParseFile(fset, name, nil, parser.SkipObjectResolution)
|
||||
if perr != nil {
|
||||
t.Fatalf("parse %s: %v", name, perr)
|
||||
}
|
||||
|
||||
// Track the enclosing FuncDecl so a hit can name its caller rather than
|
||||
// just its line.
|
||||
ast.Inspect(file, func(n ast.Node) bool {
|
||||
fn, ok := n.(*ast.FuncDecl)
|
||||
if !ok || fn.Name == nil || fn.Body == nil {
|
||||
return true
|
||||
}
|
||||
ast.Inspect(fn.Body, func(inner ast.Node) bool {
|
||||
sel, ok := inner.(*ast.SelectorExpr)
|
||||
if !ok || sel.Sel.Name != method {
|
||||
return true
|
||||
}
|
||||
pos := fset.Position(sel.Pos())
|
||||
found = append(found, callSite{filepath.Base(pos.Filename), pos.Line, fn.Name.Name})
|
||||
return true
|
||||
})
|
||||
return true
|
||||
})
|
||||
}
|
||||
|
||||
sort.Slice(found, func(i, j int) bool {
|
||||
if found[i].file != found[j].file {
|
||||
return found[i].file < found[j].file
|
||||
}
|
||||
return found[i].line < found[j].line
|
||||
})
|
||||
return found
|
||||
}
|
||||
Reference in New Issue
Block a user