Skip to content

Commit 2e7adc7

Browse files
jkyberneeesclaude
andcommitted
test(memory): fix flaky TestRebuildDoesNotSerializeRecall cleanup race
The test launches the blocked-rebuild goroutine and returned right after close(release), so the rebuild's index-gob writes (saveLocked) could land in t.TempDir AFTER the test function returned, racing the TempDir RemoveAll cleanup — CI failed with 'directory not empty'. (Product code unaffected; purely a test-teardown ordering bug.) Fix: a deferred releaseBackend()+<-g1 unblocks the backend and waits for the rebuild goroutine to finish on every exit path (deferred funcs run before t.Cleanup), so the index is fully written before TempDir is removed. Verified 50x under -race. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4a2b665 commit 2e7adc7

1 file changed

Lines changed: 17 additions & 7 deletions

File tree

internal/memory/repairs_test.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,27 @@ func TestRebuildDoesNotSerializeRecall(t *testing.T) {
201201
}
202202

203203
// G1 triggers the rebuild and blocks inside the embed call.
204-
go func() { store.recallByVector("query", 1) }() //nolint:errcheck // blocked rebuild
204+
g1 := make(chan struct{})
205+
go func() { defer close(g1); store.recallByVector("query", 1) }() //nolint:errcheck // blocked rebuild
206+
207+
// On EVERY exit path (including t.Fatal, which runs deferred funcs), unblock
208+
// the backend and wait for the rebuild goroutine to finish writing the index
209+
// gobs BEFORE t.TempDir cleanup runs — otherwise that write races RemoveAll
210+
// ("directory not empty"). Deferred funcs run before t.Cleanup, so this is
211+
// ordered correctly.
212+
var releaseOnce sync.Once
213+
releaseBackend := func() { releaseOnce.Do(func() { close(release) }) }
214+
defer func() { releaseBackend(); <-g1 }()
215+
205216
select {
206217
case <-entered:
207218
case <-time.After(5 * time.Second):
208219
t.Fatal("rebuild embed never reached the backend")
209220
}
210221

211-
// While G1 is blocked under the (released-held) backend, concurrent recalls
212-
// must NOT hang behind it. If rebuild still held the index write lock during
213-
// the network call, these would block until release and time out.
222+
// While G1 is blocked under the held backend, concurrent recalls must NOT
223+
// hang behind it. If rebuild still held the index write lock during the
224+
// network call, these would block until release and time out.
214225
done := make(chan struct{})
215226
go func() {
216227
var wg sync.WaitGroup
@@ -227,10 +238,9 @@ func TestRebuildDoesNotSerializeRecall(t *testing.T) {
227238
select {
228239
case <-done:
229240
case <-time.After(5 * time.Second):
230-
close(release)
231241
t.Fatal("concurrent recalls were serialized behind the slow rebuild (lock held during network I/O)")
232242
}
233243

234-
// Release the backend; the rebuild completes and a subsequent recall works.
235-
close(release)
244+
// Normal exit: the deferred releaseBackend() lets the rebuild complete and
245+
// the deferred <-g1 waits for it before the test's TempDir is cleaned up.
236246
}

0 commit comments

Comments
 (0)