Skip to content

fix: protect RedisStore.mutex map from concurrent access#120

Open
123456wda wants to merge 1 commit into
sony:masterfrom
123456wda:fix/redis-store-map-race
Open

fix: protect RedisStore.mutex map from concurrent access#120
123456wda wants to merge 1 commit into
sony:masterfrom
123456wda:fix/redis-store-map-race

Conversation

@123456wda

Copy link
Copy Markdown

Problem

The mutex field in redis.Store (map[string]*redsync.Mutex) is accessed concurrently by Lock() and Unlock() without any synchronization. Go maps are not safe for concurrent read/write — this causes a data race when multiple DistributedCircuitBreaker instances share the same Store and call Lock/Unlock with different keys concurrently.

Reproduction

store := redis.NewStore(localhost:6379)

dcb1, _ := gobreaker.NewDistributedCircuitBreaker[any](store, settings1)
dcb2, _ := gobreaker.NewDistributedCircuitBreaker[any](store, settings2)

go dcb1.Execute(req1) // store.Lock(gobreaker:mutex:breaker1) → writes map
go dcb2.Execute(req2) // store.Lock(gobreaker:mutex:breaker2) → writes map
// panic: concurrent map writes

The existing TestRedisStore_Concurrent_Locks test covers this scenario but CI runs go test -v without -race, so the race is not detected.

Fix

Add a sync.Mutex to the Store struct to protect all map operations in Lock() and Unlock(). The map mutex is released before calling redsync operations to avoid holding it during potentially blocking network calls.

type Store struct {
    ctx    context.Context
    client redis.UniversalClient
    rs     *redsync.Redsync
+   mu     sync.Mutex
    mutex  map[string]*redsync.Mutex
}

Verification

go test -race ./v2/redis/...

All existing tests pass with the race detector enabled.

The mutex map in Store is accessed concurrently by Lock() and Unlock()
without any synchronization. Since Go maps are not safe for concurrent
writes, this causes a data race when multiple DistributedCircuitBreaker
instances share the same Store and call Lock/Unlock with different keys
concurrently.

Add a sync.Mutex to protect all map operations in Lock() and Unlock().
The map mutex is released before calling redsync operations to avoid
holding it during potentially blocking network calls.

Verified with: go test -race ./v2/redis/...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant