Skip to content

[plan] Fix sync.Once reset data race in cache-clear functions #18277

@github-actions

Description

@github-actions

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.go
  • pkg/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

Metadata

Metadata

Labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions