open-feature / open-feature/go-sdk

[BUG] memprovider: concurrent Track calls crash the process with "concurrent map writes"

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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.