From fd33e044675fa25b22b6b1ab69ee852632f9350f Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 18:32:40 +0200 Subject: [PATCH] test(search): hold a writer on swapped-in FIFOs; await the swapper PR #161 round 2: ReadRegular bypassing readOpened (non-blocking open, no handle check) stayed green, because the search FIFO race used FIFOs with no writer, which read EOF at once. The swapper now opens each FIFO O_RDWR and keeps every writer open until it exits, as any local process could, so a reader that skips the handle check parks forever; the bypass flip is red. The swapper is stopped and awaited in a t.Cleanup before t.TempDir removal (hyperguild#162). Refs #157 Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/search/brainroot_test.go | 41 +++++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/ingestion/internal/search/brainroot_test.go b/ingestion/internal/search/brainroot_test.go index 77715d4..cfca022 100644 --- a/ingestion/internal/search/brainroot_test.go +++ b/ingestion/internal/search/brainroot_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "strings" + "sync" "sync/atomic" "syscall" "testing" @@ -60,15 +61,44 @@ func TestQueryDoesNotFollowASymlinkedDirectory(t *testing.T) { assert.Zero(t, leaked(rs)) } +// swapInto keeps renaming a regular note, and either a symlink to outside or +// a FIFO, into path. Each FIFO has a writer held open (as any local process +// can): a FIFO with no writer reads EOF at once, which would let a reader +// that skips the handle check pass (PR #161 round 2: with a held writer, a +// bypass of readOpened hung Query after 5 iterations). +// startSwap runs swapInto until the test ends and waits for it before +// t.TempDir's cleanup (hyperguild#162). +func startSwap(t *testing.T, path, outside string, fifo bool) { + t.Helper() + var stop atomic.Bool + var wg sync.WaitGroup + wg.Add(1) + go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop) }() + t.Cleanup(func() { stop.Store(true); wg.Wait() }) +} + func swapInto(path, outside string, fifo bool, stop *atomic.Bool) { tmp := path + ".swap" + // Writers stay open until the swapper exits, so a reader parked on a + // FIFO stays parked (closing one would hand the reader EOF and hide the + // hang). Past the fd limit, new FIFOs simply have no writer. + var writers []*os.File + defer func() { + for _, w := range writers { + _ = w.Close() + } + }() for i := 0; !stop.Load(); i++ { _ = os.Remove(tmp) switch { case i%2 == 0: _ = os.WriteFile(tmp, []byte("# r\n\nzqinside "+"zqracenote"), 0o644) case fifo: - _ = syscall.Mkfifo(tmp, 0o644) + if err := syscall.Mkfifo(tmp, 0o644); err == nil { + if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil { + writers = append(writers, w) + } + } default: _ = os.Symlink(outside, tmp) } @@ -78,9 +108,7 @@ func swapInto(path, outside string, fifo bool, stop *atomic.Bool) { func TestQueryLeaksNothingUnderASymlinkSwapRace(t *testing.T) { brain, facts, outside := searchBrain(t) - var stop atomic.Bool - go swapInto(filepath.Join(facts, "r.md"), outside, false, &stop) - defer stop.Store(true) + startSwap(t, filepath.Join(facts, "r.md"), outside, false) leaks, iters := 0, 0 for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); iters++ { rs, _ := Query(brain, QueryOptions{Query: sentinel157, Limit: 10}) @@ -93,8 +121,7 @@ func TestQueryLeaksNothingUnderASymlinkSwapRace(t *testing.T) { func TestQueryDoesNotHangUnderAFIFOSwapRace(t *testing.T) { brain, facts, outside := searchBrain(t) - var stop atomic.Bool - go swapInto(filepath.Join(facts, "r.md"), outside, true, &stop) + startSwap(t, filepath.Join(facts, "r.md"), outside, true) done := make(chan struct{}) go func() { for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); { @@ -104,9 +131,7 @@ func TestQueryDoesNotHangUnderAFIFOSwapRace(t *testing.T) { }() select { case <-done: - stop.Store(true) case <-time.After(6 * time.Second): - stop.Store(true) if f, err := os.OpenFile(filepath.Join(facts, "r.md"), os.O_WRONLY|syscall.O_NONBLOCK, 0); err == nil { _ = f.Close() }