internal/fmtsort: don't out-of-bounds panic if there's a race condition

Raising an out-of-bounds panic is confusing. There's no indication
that the underlying problem is a race.

The runtime already does a pretty good job of detecting this kind of
race (modification while iterating). We might as well just reorganize
a bit to avoid the out-of-bounds panic.

Fixes #33275

Change-Id: Icdd337ad2eb3c84f999db0850ec1d2ff2c146b6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/191197
Reviewed-by: Martin Möhrmann <moehrmann@google.com>

src/internal/fmtsort/sort.go

@@ -53,12 +53,16 @@ func Sort(mapValue reflect.Value) *SortedMap {
 	if mapValue.Type().Kind() != reflect.Map {
 		return nil
 	}
-	key := make([]reflect.Value, mapValue.Len())
+	// Note: this code is arranged to not panic even in the presence
-	value := make([]reflect.Value, len(key))
+	// of a concurrent map update. The runtime is responsible for
+	// yelling loudly if that happens. See issue 33275.
+	n := mapValue.Len()
+	key := make([]reflect.Value, 0, n)
+	value := make([]reflect.Value, 0, n)
 	iter := mapValue.MapRange()
-	for i := 0; iter.Next(); i++ {
+	for iter.Next() {
-		key[i] = iter.Key()
+		key = append(key, iter.Key())
-		value[i] = iter.Value()
+		value = append(value, iter.Value())
 	}
 	sorted := &SortedMap{
 		Key:   key,

test/fixedbugs/issue33275.go

@@ -0,0 +1,34 @@
+// skip
+
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"fmt"
+	"time"
+)
+
+func main() {
+	// Make a big map.
+	m := map[int]int{}
+	for i := 0; i < 100000; i++ {
+		m[i] = i
+	}
+	c := make(chan string)
+	go func() {
+		// Print the map.
+		s := fmt.Sprintln(m)
+		c <- s
+	}()
+	go func() {
+		time.Sleep(1 * time.Millisecond)
+		// Add an extra item to the map while iterating.
+		m[-1] = -1
+		c <- ""
+	}()
+	<-c
+	<-c
+}

test/fixedbugs/issue33275_run.go

@@ -0,0 +1,25 @@
+// +build !nacl,!js
+// run
+
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Make sure we don't get an index out of bounds error
+// while trying to print a map that is concurrently modified.
+// The runtime might complain (throw) if it detects the modification,
+// so we have to run the test as a subprocess.
+
+package main
+
+import (
+	"os/exec"
+	"strings"
+)
+
+func main() {
+	out, _ := exec.Command("go", "run", "fixedbugs/issue33275.go").CombinedOutput()
+	if strings.Contains(string(out), "index out of range") {
+		panic(`go run issue33275.go reported "index out of range"`)
+	}
+}