-
Notifications
You must be signed in to change notification settings - Fork 341
[plan] Fix sync.Once reset data race in cache-clear functions #18277
Description
Objective
Fix a data race caused by resetting sync.Once values via direct struct assignment (= sync.Once{}), which violates the Go memory model.
Context
From Sergo analysis run §22372431560 (discussion #18227).
Two cache-clear functions reset sync.Once by writing = sync.Once{}, which is a data race per the Go memory model if any goroutine is concurrently inside Do() or reading the once's internal state. The companion result/error variables are also unsynchronized.
Go's sync package documentation states: "A Once must not be copied after first use."
Affected Locations
| File | Line | Code |
|---|---|---|
pkg/cli/repo.go |
~26 | getCurrentRepoSlugOnce = sync.Once{} |
pkg/workflow/repository_features_validation.go |
~88 | getCurrentRepositoryOnce = sync.Once{} |
Approach
Replace the sync.Once reset pattern with a safe alternative:
Option A – Use a sync.Mutex-guarded reset function:
var mu sync.Mutex
var once sync.Once
var cachedResult string
var cachedErr error
func clearCache() {
mu.Lock()
defer mu.Unlock()
once = sync.Once{} // Still racy — don't do this
}Option B (Recommended) – Replace with an atomic pointer or a mutex-protected struct:
type cacheState struct {
result string
err error
done bool
}
var mu sync.Mutex
var state cacheState
func getOrCompute() (string, error) {
mu.Lock()
defer mu.Unlock()
if !state.done {
state.result, state.err = compute()
state.done = true
}
return state.result, state.err
}
func clearCache() {
mu.Lock()
defer mu.Unlock()
state = cacheState{}
}Option C – Use atomic.Pointer with a copy-on-write approach if performance is critical.
Pick the simplest option that eliminates the race while maintaining test utility (i.e., the clear function still works in tests).
Files to Modify
pkg/cli/repo.gopkg/workflow/repository_features_validation.go- Corresponding test files if they exercise the clear functions
Acceptance Criteria
- No direct
sync.Once{}struct assignment after first use - Cache-clear functions are safe to call concurrently with cache-read functions
-
go test -race ./pkg/cli/... ./pkg/workflow/...passes without data race reports - Existing tests continue to pass
Generated by Plan Command for issue #discussion #18227
- expires on Feb 27, 2026, 6:22 AM UTC