open-feature / open-feature/go-sdk

[BUG] multiprovider: Provider.initialized and shutdownFunc are read and written without synchronization

Open
#557 0 comments 0 reactions 0 assignees View on GitHub
bug multi-provider Needs Triage
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.