diff --git a/backend/internal/store/kill9_test.go b/backend/internal/store/kill9_test.go index e946dcb9..cb96e022 100644 --- a/backend/internal/store/kill9_test.go +++ b/backend/internal/store/kill9_test.go @@ -2,6 +2,7 @@ package store import ( "bufio" + "errors" "fmt" "io" "math" @@ -44,10 +45,12 @@ import ( const ( killLoopEnv = "TM_STORE_KILL_LOOP_DB" - // killLoopRoundEnv scopes the helper's request hashes to its round. Checkpoints are idempotent by - // request_hash (ON CONFLICT DO NOTHING), so two rounds that generated the same hash would settle - // once between them, leave the count flat, and make the increment check below report a durability - // failure that never happened. The round makes each round's writes unambiguously its own. + // killLoopRoundEnv scopes the helper's request hashes to its round. The rest of a hash is the + // helper's pid and a counter that restarts at zero every round, so two rounds can only collide when + // the OS hands the second helper the pid the first one had. Checkpoints are idempotent by + // request_hash (ON CONFLICT DO NOTHING), so that collision would settle once between the two rounds, + // leave the count flat, and make the increment check below report a durability failure that never + // happened. The round makes each round's writes unambiguously its own. killLoopRoundEnv = "TM_STORE_KILL_LOOP_ROUND" // killLoopReady is printed by the helper exactly once, after its FIRST settle+checkpoint has // committed. It is the whole liveness signal: everything the parent asserts afterwards is about work @@ -111,12 +114,23 @@ type killHelper struct { drained <-chan struct{} } -// killAndReap SIGKILLs the helper and collects it in the order os/exec requires. +// killAndReap SIGKILLs a helper that is expected to be running and collects it in the order os/exec +// requires. A helper that has already exited means the round lost its subject before the kill landed, +// so that is named rather than absorbed. func (h *killHelper) killAndReap(t *testing.T, round int) { t.Helper() if err := h.cmd.Process.Signal(syscall.SIGKILL); err != nil { t.Fatalf("round %d: SIGKILL: %v", round, err) } + h.reap() +} + +// reap leaves nothing of the helper behind. The signal is best-effort because reap also serves the +// paths where the helper may already have exited on its own; what it guarantees is that a live helper +// is stopped BEFORE the reader is waited on. The reader drains until EOF, and a process still running +// never produces one — so waiting for it first would hang the test where it means to fail it. +func (h *killHelper) reap() { + _ = h.cmd.Process.Signal(syscall.SIGKILL) <-h.drained // every read from the pipe has completed… _ = h.cmd.Wait() // …and only now may Wait close it } @@ -156,7 +170,14 @@ func startKillHelper(t *testing.T, round int, dbPath string) *killHelper { } } if !seen { - ready <- fmt.Errorf("helper exited without ever committing a checkpoint (scanner: %v)", sc.Err()) + // A scanner failure and a clean EOF are different facts about the helper — one is a broken + // read, the other a process that ran and said nothing — and the reader keeps them apart so + // the parent's diagnosis names which one happened. + if err := sc.Err(); err != nil { + ready <- fmt.Errorf("the helper's stdout could not be read to the end: %w", err) + } else { + ready <- errors.New("the helper's stdout reached a clean EOF without the commit marker") + } } // An over-long line would have stopped the scanner above; keep draining regardless so a stalled // pipe can never be mistaken for a stalled store. @@ -166,12 +187,11 @@ func startKillHelper(t *testing.T, round int, dbPath string) *killHelper { select { case err := <-ready: if err != nil { - <-drained - _ = cmd.Wait() + h.reap() t.Fatalf("round %d: %v — there was nothing for SIGKILL to land on, so everything asserted afterwards would be vacuous", round, err) } case <-time.After(killLoopReadyWait): - h.killAndReap(t, round) + h.reap() t.Fatalf("round %d: the helper did not commit a first checkpoint within %s — the kill would have landed on a process that had done nothing, and everything asserted afterwards would be vacuous", round, killLoopReadyWait) }