From 0288ebd1a3d827aabe2205470d57b6083c45ea3f Mon Sep 17 00:00:00 2001 From: Nick Fox <6226732+nrfox@users.noreply.github.com> Date: Thu, 6 Mar 2025 11:27:41 -0500 Subject: [PATCH] Fix data race in `store.Items()` (#8214) `store.Items()` was returning the underlying data rather than a copy. --- store/store_test.go | 12 ++++++++++++ store/threadsafe_store.go | 9 +++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/store/store_test.go b/store/store_test.go index ce188beb5e..0619c4888d 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -143,6 +143,18 @@ func TestItemsReturnsWholeMap(t *testing.T) { require.Equal(99, contents["key2"]) } +func TestItemsReturnsCopy(t *testing.T) { + require := require.New(t) + + testStore := store.New[string, int]() + testStore.Replace(map[string]int{"key1": 42, "key2": 99}) + + contents := testStore.Items() + contents["key1"] = 52 + + require.Equal(42, testStore.Items()["key1"]) +} + func TestDeleteKey(t *testing.T) { require := require.New(t) diff --git a/store/threadsafe_store.go b/store/threadsafe_store.go index 1f32cb4a11..b488aa9751 100644 --- a/store/threadsafe_store.go +++ b/store/threadsafe_store.go @@ -1,9 +1,9 @@ package store import ( + "maps" + "slices" "sync" - - "golang.org/x/exp/maps" ) // threadSafeStore implements the Store interface and is safe for concurrent use. @@ -25,10 +25,11 @@ func (s *threadSafeStore[K, V]) Get(key K) (V, bool) { return v, found } +// Items returns a copy of all items in the store. func (s *threadSafeStore[K, V]) Items() map[K]V { s.lock.RLock() defer s.lock.RUnlock() - return s.data + return maps.Clone(s.data) } // Replace replaces the contents of the store with the given map. @@ -63,7 +64,7 @@ func (s *threadSafeStore[K, V]) Remove(key K) { func (s *threadSafeStore[K, V]) Keys() []K { s.lock.RLock() defer s.lock.RUnlock() - return maps.Keys(s.data) + return slices.Collect(maps.Keys(s.data)) } func (s *threadSafeStore[K, V]) Version() uint {