Go: 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 [e-mail] 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
https://github.com/golang/go/issues/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.