go: sync: concurrent map iteration and map write on Map error

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go version go1.10 darwin/amd64 ZShens-MacBook-Pro:network2 zshen$ go env GOARCH=“amd64” GOBIN=“” GOCACHE=“/Users/zshen/Library/Caches/go-build” GOEXE=“” GOHOSTARCH=“amd64” GOHOSTOS=“darwin” GOOS=“darwin” GOPATH=“/Users/zshen/Workspace/go” GORACE=“” GOROOT=“/usr/local/Cellar/go/1.10/libexec” GOTMPDIR=“” GOTOOLDIR=“/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64” GCCGO=“gccgo” CC=“clang” CXX=“clang++” CGO_ENABLED=“1” CGO_CFLAGS=“-g -O2” CGO_CPPFLAGS=“” CGO_CXXFLAGS=“-g -O2” CGO_FFLAGS=“-g -O2” CGO_LDFLAGS=“-g -O2” PKG_CONFIG=“pkg-config” GOGCCFLAGS=“-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/k4/tn3jnym94dq51ghd4krhvgfr0000gn/T/go-build339950240=/tmp/go-build -gno-record-gcc-switches -fno-common”

What did you do?

Modify and read sync.Map concurrently

What did you expect to see?

Work fine

What did you see instead?

fatal error: concurrent map iteration and map write

goroutine 223 [running]: runtime.throw(0x153d054, 0x26) /usr/local/Cellar/go/1.10/libexec/src/runtime/panic.go:619 +0x81 fp=0xc420643750 sp=0xc420643730 pc=0x102cf71 runtime.mapiternext(0xc420643870) /usr/local/Cellar/go/1.10/libexec/src/runtime/hashmap.go:747 +0x55c fp=0xc4206437e0 sp=0xc420643750 pc=0x100aeec runtime.mapiterinit(0x1489e60, 0xc4202dee70, 0xc420643870) /usr/local/Cellar/go/1.10/libexec/src/runtime/hashmap.go:737 +0x1f1 fp=0xc420643808 sp=0xc4206437e0 pc=0x100a8a1 sync.(*Map).Range(0xc420371590, 0xc420643908) /usr/local/Cellar/go/1.10/libexec/src/sync/map.go:332 +0xce fp=0xc4206438e0 sp=0xc420643808 pc=0x10626ee

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 28 (20 by maintainers)

Most upvoted comments

  1. This is not a regression introduced in Go 1.10, because the stack trace posted by @barryqiu is produced by Go 1.9.

  2. I read the code of sync.Map and I think this can’t be a bug in it.

@barryqiu @zjshen14 Are you sure your code doesn’t copy the sync.Map after first use? I can reproduce the identical stack trace if I copy sync.Map. The code is

package main

import (
	"math/rand"
	"sync"
)

func main() {
	var m sync.Map

	for i := 0; i < 64; i++ {
		key := rand.Intn(128)
		m.Store(key, key)
	}
	n := m
	go func() {
		for {
			key := rand.Intn(128)
			m.Store(key, key)
		}
	}()
	for {
		n.Range(func(key, value interface{}) bool {
			return key == value
		})
	}
}

The document says “A Map must not be copied after first use” https://golang.org/pkg/sync/#Map Actually, everything in sync and sync/atomic must not be copied after first use.

After copying the sync.Map, two sync.Maps are holding the same map. Two goroutines can access this map without protection of one mutex.

It’s nice that we now run go vet automatically, but I really, really wish it was more obvious in the release notes that not all checks are enabled by default

https://golang.org/doc/go1.10#introduction

The mention here is not enough: apparently many people don’t read the entire document, as I’ve discovered myself by finding this issue resident in living codebases.

Would some sort of copy detection help here?

The vet copylocks check already catches it (https://golang.org/cl/59690). I do not know why the copylocks check was not enabled by default as part of #18085.

It’s commented out in a TODO(@rsc) here:

https://github.com/golang/go/blob/91f74069ef442f8d963f43cc898af8af3e8b8d0e/src/cmd/go/internal/test/test.go#L515-L526

Would some sort of copy detection help here? Have the sync.Map store the address on first use and clearly panic if it changes?

This looks like memory corruption. Have you tried running your program under the race detector? See https://blog.golang.org/race-detector .