open-feature / open-feature/go-sdk
[BUG] memprovider: concurrent Track calls crash the process with "concurrent map writes"
- Dominant language
- Go
- Stars
- 249
- Forks
- 61
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 20
Description
## Observed behavior
`InMemoryProvider.Track` appends to a map field on a **value receiver**, with no synchronization:
https://github.com/open-feature/go-sdk/blob/6b0824c6c7b9b69e40f26aedcc4406ad4d3a5cec/openfeature/memprovider/in_memory_provider.go#L136-L142
Concurrent `Track` calls — the normal case, since a provider is shared across goroutines — trip the runtime's map-corruption detector. This is a `fatal error`, **not** a recoverable panic: no `recover()` can save the process.
```
WARNING: DATA RACE
Write at 0x... by goroutine 10:
memprovider.InMemoryProvider.Track() in_memory_provider.go:137
Previous write by goroutine 15: (same line)
...
fatal error: concurrent map writes
```
A second, smaller problem: `trackingEvents` has **no accessor**. It appears only at its declaration, its initialization in `NewInMemoryProvider`, and that one write. Nothing can read the recorded events back, so `Track` is write-only state that no test can assert on — which is likely why this went unnoticed. The existing `TestInMemoryProvider_Track` calls `Track` and asserts nothing.
## Expected Behavior
`trackingEvents` guarded by a mutex (which requires a pointer receiver), plus a getter so the recorded events are usable for the testing purpose this provider exists to serve.
Worth coordinating with #530 / #539: a spec-compliant `UpdateFlags` needs a pointer receiver and a mutex over `flags` anyway, and the same change would naturally cover `trackingEvents`.
`flags` has a related aliasing hazard today — `NewInMemoryProvider` stores the caller's map by reference with no copy, so a caller mutating their own map races in-flight evaluations:
https://github.com/open-feature/go-sdk/blob/6b0824c6c7b9b69e40f26aedcc4406ad4d3a5cec/openfeature/memprovider/in_memory_provider.go#L21-L26
The other SDKs all copy defensively — Java `new ConcurrentHashMap<>(flags)`, JS `{ ...flagConfiguration }`, Python `flags.copy()` — as does this repo's own `NewEvaluationContext`.
## Steps to reproduce
```go
p := memprovider.NewInMemoryProvider(map[string]memprovider.InMemoryFlag{})
var wg sync.WaitGroup
for i := 0; i < 8; i++ {
wg.Add(1)
go func(n int) {
defer wg.Done()
for j := 0; j < 200; j++ {
p.Track(context.Background(), fmt.Sprintf("ev-%d-%d", n, j),
of.EvaluationContext{}, of.NewTrackingEventDetails(1))
}
}(i)
}
wg.Wait()
```
`go test -race` reports the data race; without `-race` it still reaches `fatal error: concurrent map writes` and takes the process down.
Contributor guide
Research direction
Start with openfeature/memprovider/in_memory_provider.go, especially InMemoryProvider.Track and NewInMemoryProvider, then read TestInMemoryProvider_Track. Run the supplied concurrent reproduction with go test -race; done means concurrent tracking no longer reports a race or crashes, and the recorded events can be retrieved and asserted in tests. Coordinate the flags-related scope with #530 and #539.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100