test(dashboard): quiesce background algo-sweep before TempDir cleanup (Windows CI)#5896
Merged
Merged
Conversation
…rdown (fix Windows RemoveAll) TestHandleRepoRelationships_NoRef (and its ~19 buildRefEndpointFixture siblings, plus the handlers_v2_diff setupDiffTest fixture) can red the v0.1.9 3-OS acceptance run on windows-latest with a "TempDir RemoveAll cleanup: unlinkat <...>\refs\_unknown: The directory is not empty" error. Root cause is NOT the mmap-default-on / generation-fb change: the fixture writes graph.json only, so CurrentGraphPath resolves the (absent) flat graph.fb, fbreader.Open fails, and DashRepo.Reader stays nil -- no mapping is held. The real hazard is the deferred Pass-4 sweep. A cache load of a non-empty graph (loadGroupForRef -> applyAlgorithmsOnLoad) schedules schedulePendingAlgo, which runs on a BACKGROUND goroutine and writes graph-algo.json (os.CreateTemp + rename) into the repo's state dir AFTER the triggering request already returned. When that state dir is a t.TempDir, the write races Go's automatic RemoveAll teardown; on Windows a file created/opened concurrently with a directory removal fails RemoveAll. On Unix unlink-while-open is legal, so it only ever reddens Windows -- same class as PR #5893. This is a test-teardown gap, not a product handle leak: in production the state dir is the durable daemon root, never removed out from under the sweep, so writing graph-algo.json there is correct. Fix: add quiesceGraphCache(t, srv) -- sets backgroundAlgoGate so every scheduled sweep parks BEFORE touching the filesystem (no graph-algo.json, nor its temp handle, is ever created under the temp dir), and registers a cleanup that restores the prior gate and calls graphs.InvalidateAll() (the production reader-eviction primitive, closing any mmap fbreader.Reader as defence for the mmap-default-on path). Wired into the two confirmed cache-load-over-temp-dir fixtures: buildRefEndpointFixture and setupDiffTest. Audited the whole internal/dashboard test package: no other fixture holds a reader or spawns the sweep over a temp dir (others seed the cache directly, stat-only, close their readers, or use empty/invalid graphs). The background-sweep goroutine is dashboard-only. Gates: gofmt -l clean, go build, go vet, go test ./internal/dashboard/ (942 pass) and full go test ./... (11547 pass, 128 packages) all green on macOS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017quGgaqK7NRoxGT6BTqV2o
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the sole windows-latest failure in the v0.1.9 3-OS acceptance CI:
TestHandleRepoRelationships_NoRef→TempDir RemoveAll cleanup: ...refs\_unknown: The directory is not empty. Refs #5850.Root cause (empirically confirmed — NOT mmap): the ref-endpoint fixture is
graph.json-only, so no memory mapping is ever held. The failure is a deferred background algo-sweep goroutine (schedulePendingAlgo) writinggraph-algo.jsoninto the state dir after the triggering HTTP request returns, racing Go's automaticRemoveAllof thet.TempDir. On Windows a file created concurrently with directory removal makesRemoveAllfail; on Unix unlink-while-open is legal, so it only reddens Windows. Timing-flake shared by ~20 fixture siblings; this one lost the race that run. Production state dirs are durable (never swept), so this is a test gap, not a product leak.Fix: new
quiesceGraphCache(t, srv)helper parks the sweep via the existingbackgroundAlgoGatehook (nograph-algo.jsonwritten under the temp dir) and, int.Cleanup(LIFO before RemoveAll), restores the gate + callssrv.graphs.InvalidateAll()(production reader-eviction, defense for the mmap-default-on path). Wired intobuildRefEndpointFixture(the failing test + ~19 siblings) andsetupDiffTest(second instance). Whole-internal/dashboardaudit found no other fixture holds a reader or spawns the sweep over a temp dir.Test-only; full suite 11547 pass. Real proof is the Windows CI re-run (can't red→green on macOS — Unix allows unlink-while-open).
🤖 Generated with Claude Code
https://claude.ai/code/session_017quGgaqK7NRoxGT6BTqV2o