open-feature / open-feature/go-sdk
[BUG] multiprovider: Provider.initialized and shutdownFunc are read and written without synchronization
- Dominant language
- Go
- Stars
- 249
- Forks
- 61
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 20
Description
## Observed behavior
`initialized` is a plain `bool` field, while every other piece of shared state on the struct is mutex-guarded (`overallStatusLock`, `providerStatusLock`):
https://github.com/open-feature/go-sdk/blob/6b0824c6c7b9b69e40f26aedcc4406ad4d3a5cec/openfeature/multi/multiprovider.go#L34-L50
It is written in `InitWithContext` and `ShutdownWithContext`, and read in `ShutdownWithContext` and `Track`. Since a provider is shared across concurrent evaluations and tracking calls, this is a genuine race:
```
WARNING: DATA RACE
Write at 0x... by goroutine 12:
multi.(*Provider).ShutdownWithContext() multiprovider.go:606
Previous read at 0x... by goroutine 11:
multi.(*Provider).Track() multiprovider.go:634
```
`shutdownFunc` has the same exposure — assigned in `InitWithContext`, read in `ShutdownWithContext` — with a worse failure mode: `ShutdownWithContext` can observe `initialized == true` while `shutdownFunc` is still nil, and nil-deref when it calls it:
https://github.com/open-feature/go-sdk/blob/6b0824c6c7b9b69e40f26aedcc4406ad4d3a5cec/openfeature/multi/multiprovider.go#L569-L580
## Expected Behavior
`go test -race` clean. Either make `initialized` an `atomic.Bool` (and guard `shutdownFunc` similarly), or fold both under one of the existing locks.
## Steps to reproduce
```go
mp, _ := multi.NewProvider(multi.StrategyFirstMatch,
multi.WithProvider("a", memprovider.NewInMemoryProvider(map[string]memprovider.InMemoryFlag{})))
ctx := context.Background()
mp.InitWithContext(ctx, of.EvaluationContext{})
var wg sync.WaitGroup
wg.Add(2)
go func() {
defer wg.Done()
for i := 0; i < 500; i++ {
mp.Track(ctx, "ev", of.EvaluationContext{}, of.NewTrackingEventDetails(1))
}
}()
go func() {
defer wg.Done()
time.Sleep(2 * time.Millisecond)
mp.ShutdownWithContext(ctx)
}()
wg.Wait()
```
Run with `go test -race`.
Contributor guide
Research direction
Start in openfeature/multi/multiprovider.go, focusing on InitWithContext, ShutdownWithContext, and Track, then run the supplied concurrent reproducer with `go test -race`. Done means concurrent tracking and shutdown no longer report races or permit the described nil shutdownFunc failure.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100