sync: clarify the validity to call Map methods inside Range

This change clarifies that calling all Map methods inside the callback
of Range is allowed. For further assurance, a nested range call test
is also added.

Fixes #46399

Change-Id: I0a766a5c1470e6b573ec35df1ccd62b2e46f1561
Reviewed-on: https://go-review.googlesource.com/c/go/+/337389
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Ian Lance Taylor <iant@golang.org>

diff --git a/src/sync/map.go b/src/sync/map.go
index dfb62dd3e801cc8717ab7791b0a0e31f4ec43a1c..7a6c82e5c395191b97a3b27ccc20092b0b137059 100644 (file)
--- a/src/sync/map.go
+++ b/src/sync/map.go
@@ -311,8 +311,9 @@ func (e *entry) delete() (value interface{}, ok bool) {
 //
 // Range does not necessarily correspond to any consistent snapshot of the Map's
 // contents: no key will be visited more than once, but if the value for any key
-// is stored or deleted concurrently, Range may reflect any mapping for that key
-// from any point during the Range call.
+// is stored or deleted concurrently (including by f), Range may reflect any
+// mapping for that key from any point during the Range call. Range does not
+// block other methods on the receiver; even f itself may call any method on m.
 //
 // Range may be O(N) with the number of elements in the map even if f returns
 // false after a constant number of calls.

diff --git a/src/sync/map_test.go b/src/sync/map_test.go
index 7f163caa5c95d111845844e41fa401065b4a9766..c4a8f8b99aaa95d670756187a30078522086a1cc 100644 (file)
--- a/src/sync/map_test.go
+++ b/src/sync/map_test.go
@@ -195,3 +195,53 @@ func TestIssue40999(t *testing.T) {
                runtime.GC()
        }
 }
+
+func TestMapRangeNestedCall(t *testing.T) { // Issue 46399
+       var m sync.Map
+       for i, v := range [3]string{"hello", "world", "Go"} {
+               m.Store(i, v)
+       }
+       m.Range(func(key, value interface{}) bool {
+               m.Range(func(key, value interface{}) bool {
+                       // We should be able to load the key offered in the Range callback,
+                       // because there are no concurrent Delete involved in this tested map.
+                       if v, ok := m.Load(key); !ok || !reflect.DeepEqual(v, value) {
+                               t.Fatalf("Nested Range loads unexpected value, got %+v want %+v", v, value)
+                       }
+
+                       // We didn't keep 42 and a value into the map before, if somehow we loaded
+                       // a value from such a key, meaning there must be an internal bug regarding
+                       // nested range in the Map.
+                       if _, loaded := m.LoadOrStore(42, "dummy"); loaded {
+                               t.Fatalf("Nested Range loads unexpected value, want store a new value")
+                       }
+
+                       // Try to Store then LoadAndDelete the corresponding value with the key
+                       // 42 to the Map. In this case, the key 42 and associated value should be
+                       // removed from the Map. Therefore any future range won't observe key 42
+                       // as we checked in above.
+                       val := "sync.Map"
+                       m.Store(42, val)
+                       if v, loaded := m.LoadAndDelete(42); !loaded || !reflect.DeepEqual(v, val) {
+                               t.Fatalf("Nested Range loads unexpected value, got %v, want %v", v, val)
+                       }
+                       return true
+               })
+
+               // Remove key from Map on-the-fly.
+               m.Delete(key)
+               return true
+       })
+
+       // After a Range of Delete, all keys should be removed and any
+       // further Range won't invoke the callback. Hence length remains 0.
+       length := 0
+       m.Range(func(key, value interface{}) bool {
+               length++
+               return true
+       })
+
+       if length != 0 {
+               t.Fatalf("Unexpected sync.Map size, got %v want %v", length, 0)
+       }
+}