Map protected by mutex caused panic: concurrent map writes

[VojtechVitek]

I've got a simple package to protect map from concurrent read/writes via sync.Mutex. But from time to time I get panic concurrent map writes.

Can you guys see any issue with the below code? What am I doing wrong? Or is this a Go bug?

What did you do?

package stringmap

import "sync"

// concurrentStringMap is a concurrent storage for strings (emails, links etc.)
// that automatically removes duplicates. Very helpful for verifying output
// of Newsletter recipients or URL links in Newsletter HTML/Text.
type concurrentStringMap struct {
	sync.Mutex
	strings map[string]struct{}
}

func NewConcurrentStringMap() concurrentStringMap {
	return concurrentStringMap{
		strings: map[string]struct{}{},
	}
}

func (u concurrentStringMap) Add(str string) bool {
	u.Lock()
	defer u.Unlock()
	if _, ok := u.strings[str]; ok {
		return false
	}
	u.strings[str] = struct{}{} // panic happens here
	return true
}

func (u concurrentStringMap) Slice() []string {
	u.Lock()
	defer u.Unlock()
	slice := make([]string, 0, len(u.strings))
	for str, _ := range u.strings {
		slice = append(slice, str)
	}
	return slice
}

What did you expect to see?

No panic?

What did you see instead?

fatal error: concurrent map writes

goroutine 1194 [running]:
runtime.throw(0x1fe22bf, 0x15)
	/usr/local/go/src/runtime/panic.go:596 +0x95 fp=0xc420ca8dc8 sp=0xc420ca8da8
runtime.mapassign(0x1e03b40, 0xc420bbb890, 0xc420ca8ea0, 0x1b)
	/usr/local/go/src/runtime/hashmap.go:499 +0x667 fp=0xc420ca8e68 sp=0xc420ca8dc8
github.com/pressly/api/lib/stringmap.concurrentStringMap.Add(0x0, 0xc420bbb890, 0xc420ec7560, 0x1b, 0x0)
	***/stringmap/stringmap.go:25 +0x127 fp=0xc420ca8ec0 sp=0xc420ca8e68

Does this issue reproduce with the latest release (go1.8.1)?

System details

go version go1.8 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/vojtechvitek/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/hr/5zb8r0yx4sv4_1dc0rlccflm0000gn/T/go-build257984097=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
GOROOT/bin/go version: go version go1.8 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.8 X:framepointer
uname -v: Darwin Kernel Version 16.4.0: Thu Dec 22 22:53:21 PST 2016; root:xnu-3789.41.3~3/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.12.3
BuildVersion:	16D32
lldb --version: lldb-370.0.40
  Swift-3.1

[randall77]

You're copying the mutex. https://golang.org/src/sync/mutex.go, line 25.
You'll need to return a *concurrentStringMap from NewConcurrentStringMap and make all your receivers pointers.

[davecheney]

Declare your methods on *concurrentStringMap. Declaring them on concurrentStringMap will cause the receiver to be copied on each invocation, copying the lock, making it ineffective. Vet should also spot this.

…

On Fri, 21 Apr 2017, 08:31 Vojtech Vitek ***@***.***> wrote: I've got a simple package to protect map from concurrent read/writes via sync.Mutex. But from time to time I get panic concurrent map writes. Can you guys see any issue with the below code? What am I doing wrong? Or is this a Go bug? What did you do? package stringmap import "sync" // concurrentStringMap is a concurrent storage for strings (emails, links etc.)// that automatically removes duplicates. Very helpful for verifying output// of Newsletter recipients or URL links in Newsletter HTML/Text.type concurrentStringMap struct { sync.Mutex strings map[string]struct{} } func NewConcurrentStringMap() concurrentStringMap { return concurrentStringMap{ strings: map[string]struct{}{}, } } func (u concurrentStringMap) Add(str string) bool { u.Lock() defer u.Unlock() if _, ok := u.strings[str]; ok { return false } u.strings[str] = struct{}{} return true } func (u concurrentStringMap) Slice() []string { u.Lock() defer u.Unlock() slice := make([]string, 0, len(u.strings)) for str, _ := range u.strings { slice = append(slice, str) } return slice } What did you expect to see? No panic? What did you see instead? fatal error: concurrent map writes goroutine 1194 [running]: runtime.throw(0x1fe22bf, 0x15) /usr/local/go/src/runtime/panic.go:596 +0x95 fp=0xc420ca8dc8 sp=0xc420ca8da8 runtime.mapassign(0x1e03b40, 0xc420bbb890, 0xc420ca8ea0, 0x1b) /usr/local/go/src/runtime/hashmap.go:499 +0x667 fp=0xc420ca8e68 sp=0xc420ca8dc8github.com/pressly/api/lib/stringmap.concurrentStringMap.Add(0x0, 0xc420bbb890, 0xc420ec7560, 0x1b, 0x0) ***/stringmap/stringmap.go:25 +0x127 fp=0xc420ca8ec0 sp=0xc420ca8e68 Does this issue reproduce with the latest release (go1.8.1)? System details go version go1.8 darwin/amd64 GOARCH="amd64" GOBIN="" GOEXE="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOOS="darwin" GOPATH="/Users/vojtechvitek/go" GORACE="" GOROOT="/usr/local/go" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64" GCCGO="gccgo" CC="clang" GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/hr/5zb8r0yx4sv4_1dc0rlccflm0000gn/T/go-build257984097=/tmp/go-build -gno-record-gcc-switches -fno-common" CXX="clang++" CGO_ENABLED="1" PKG_CONFIG="pkg-config" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" GOROOT/bin/go version: go version go1.8 darwin/amd64 GOROOT/bin/go tool compile -V: compile version go1.8 X:framepointer uname -v: Darwin Kernel Version 16.4.0: Thu Dec 22 22:53:21 PST 2016; root:xnu-3789.41.3~3/RELEASE_X86_64 ProductName: Mac OS X ProductVersion: 10.12.3 BuildVersion: 16D32 lldb --version: lldb-370.0.40 Swift-3.1 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#20060>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcAyQd8myDFshRTyhm7Sydvya0H0x_ks5rx9zcgaJpZM4NDp6-> .

[VojtechVitek]

Oh right! Thanks a lot guys!

[night-codes]

Hi! I have similar problem with mutex on go 1.8.3 (linux64) and all work good at 1.7.4

Simplified code:

type Map struct {
	sync.Mutex
	m map[string]int
}

var mymap = Map{
	m: map[string]int{},
}

/// in gorutinas:
mymap.Lock()
mymap.m["ddd"] = 123
mymap.Unlock()

And I have same panic. I'm copying the mutex too? Why all works perfectly on version 1.7.4?

Comment: A Mutex must not be copied after first use.

Where it was used first time before copying in VojtechVitek's example?

[ianlancetaylor]

@night-codes This issue is closed. For questions about writing Go, please see https://golang.org/wiki/Questions . Please do not follow up on this issue. Thanks.

In the code in this issue the mutex is being copied by the method calls, because they use a value receiver.