Fix data race in Linux compress maintainer tests#2
Merged
Conversation
TestCompressMaintainOwner and TestCompressMaintainMode waited for the background compression goroutine with a fixed sleep before reading state it writes (the fakeFS owner map and the compressed file). The sleep established no happens-before relationship, so the race detector reported a data race on Linux, where these tests run. Replace the sleep with l.Close(), which now flushes the pending compression and joins the mill goroutine, synchronizing its writes with the subsequent assertions. This also makes the tests deterministic rather than relying on a timing window. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com> Co-authored-by: Cursor <cursoragent@cursor.com>
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
1 participant
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.
Summary
CI failed on
v2.0with a data race inTestCompressMaintainOwner(and the same latent issue exists inTestCompressMaintainMode). These tests are Linux-only (//go:build linux), so the race only surfaces on the Ubuntu CI runner and not on a macOS development machine.Both tests trigger a rotation, then waited for the background compression goroutine with a fixed
<-time.After(10ms)sleep before reading state that the goroutine writes — thefakeFSowner map and the compressed output file. The sleep establishes no happens-before relationship with the goroutine, so the-racedetector correctly flags concurrent access to thefakeFSmap.The fix replaces the sleep with
l.Close(). SinceClosenow flushes the pending compression and joins the mill goroutine (via aWaitGroup), it both guarantees the compression has run and synchronizes the goroutine's writes with the assertions that follow. This removes the race and makes the tests deterministic instead of relying on a timing window.Verified by running the full test suite with
-raceongolang:1.26(Linux), andgolangci-lintwithGOOS=linux; both pass.Made with Cursor