diff --git a/src/sync/map.go b/src/sync/map.go index b26f2dc3bf..e5d6be7941 100644 --- a/src/sync/map.go +++ b/src/sync/map.go @@ -1,6 +1,8 @@ package sync -import "internal/task" +import ( + "internal/task" +) // This file implements just enough of sync.Map to get packages to compile. It // is no more efficient than a map with a lock. @@ -56,17 +58,27 @@ func (m *Map) Store(key, value interface{}) { m.m[key] = value } +// Range calls f for each key and value in the map. If f returns false, the iteration stops. func (m *Map) Range(f func(key, value interface{}) bool) { - m.lock.Lock() - defer m.lock.Unlock() + // Iterate over a key snapshot instead of holding the lock across the callback, + // to prevent deadlock when a Map method is called inside f. + // + // Using a key snapshot in Map.Range is sufficient because Go specifies that: + // - Range only requires that no key is visited more than once, and + // - Range may reflect any mapping from any point during the Range call. - if m.m == nil { - return + m.lock.Lock() + keys := make([]interface{}, 0, len(m.m)) + for k := range m.m { + keys = append(keys, k) } + m.lock.Unlock() - for k, v := range m.m { - if !f(k, v) { - break + for _, k := range keys { + if v, ok := m.Load(k); ok { + if !f(k, v) { + break + } } } } diff --git a/src/sync/map_test.go b/src/sync/map_test.go index a41faa40fd..5ae88789f1 100644 --- a/src/sync/map_test.go +++ b/src/sync/map_test.go @@ -36,3 +36,31 @@ func TestMapSwap(t *testing.T) { t.Errorf("Load after Swap returned %v, %v, want foo, true", v, ok) } } + +func TestMapRangeAndDelete(t *testing.T) { + var sm sync.Map + sm.Store(0, "0") + sm.Store(1, "1") + sm.Store(2, "2") + + sm.Range(func(k, v any) bool { + keyAsInt, ok := k.(int) + if !ok { + return true + } + if keyAsInt%2 == 0 { + sm.Delete(keyAsInt) + } + return true + }) + + if v, ok := sm.Load(0); ok { + t.Errorf("Load(0) after Delete returned %v, %v, want nil, false", v, ok) + } + if v, ok := sm.Load(1); !ok || v.(string) != "1" { + t.Errorf("Load(1) after Delete returned %v, %v, want \"1\", true", v, ok) + } + if v, ok := sm.Load(2); ok { + t.Errorf("Load(2) after Delete returned %v, %v, want nil, false", v, ok) + } +}