How can one implement a thread-safe wrapper to maps in Go by locking?

Question

I'm trying to wrap a general map (with interface{} as both key and value) as in-memory key-value store that I named MemStore. But it is not thread-safe, despite my use of a sync.RWMutex to lock access to the underlying map. I did verify that it works fine when used from a single goroutine. However, just two concurrent goroutines accessing it results in panic: runtime error: invalid memory address or nil pointer dereference.

What is causing this problem, and what is the proper way to achieve thread-safety in Go? Whilst in this example, channels to a single goroutine interacting with the map would work, I am specifically looking for a solution that works with explicit locking. File keyval.go:

package keyval

import "sync"

type MemStore struct {
    data map[interface{}]interface{}
    mutex sync.RWMutex
}

func NewMemStore() (MemStore) {
    m := MemStore{
        data: make(map[interface{}]interface{}),
        // mutex does not need initializing
    }
    return m
}

func (m MemStore) Set(key interface{}, value interface{}) (err error) {
    m.mutex.Lock()
    defer m.mutex.Unlock()

    if value != nil {
        m.data[key] = value
    } else {
        delete(m.data, key);
    }

    return nil
}

File keyval_test.go:

package keyval

import "testing"

func setN(store Store, N int, done chan<- struct{}) {
    for i := 0; i < N; i++ {
        store.Set(i, -i)
    }
    done <- struct{}{}
}

func BenchmarkMemStore(b *testing.B) {
    store := NewMemStore()
    done := make(chan struct{})
    b.ResetTimer()
    go setN(store, b.N, done)
    go setN(store, b.N, done)
    <-done
    <-done
}

Output of go test -bench .:

testing: warning: no tests to run
PASS
BenchmarkMemStore       panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x1 pc=0x80502eb]

goroutine 8 [running]:
runtime.panic(0x810f180, 0x821fc88)
        /usr/lib/go/src/pkg/runtime/panic.c:266 +0xac
github.com/pyramids/keyval.MemStore.Set(0x1852e4a0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/steinb/go/src/github.com/pyramids/keyval/keyval.go:200 +0xb6
github.com/pyramids/keyval.(*MemStore).Set(0x1852efe0, 0x80f38c0, 0x19ff, 0x80f38c0, 0xffffe601, ...)
        /home/steinb/go/src/github.com/pyramids/keyval/keyval.go:1 +0xa1
github.com/pyramids/keyval.setN(0xb77b89d8, 0x1852efe0, 0x2710, 0x1851c090)
        /home/steinb/go/src/github.com/pyramids/keyval/memstore_test.go:69 +0x5b
created by github.com/pyramids/keyval.BenchmarkMemStore
        /home/steinb/go/src/github.com/pyramids/keyval/memstore_test.go:89 +0x116

goroutine 1 [chan receive]:
testing.(*B).run(0x1854a000, 0x3, 0xb76cdeb4, 0x1, 0x1, ...)
        /usr/lib/go/src/pkg/testing/benchmark.go:171 +0x4b
testing.RunBenchmarks(0x814d840, 0x821c828, 0x1, 0x1)
        /usr/lib/go/src/pkg/testing/benchmark.go:303 +0x464
testing.Main(0x814d840, 0x8222220, 0x0, 0x0, 0x821c828, ...)
        /usr/lib/go/src/pkg/testing/testing.go:411 +0x151
main.main()
        github.com/pyramids/keyval/_test/_testmain.go:47 +0x83

goroutine 3 [chan receive]:
github.com/pyramids/keyval.BenchmarkMemStore(0x1854a000)
        /home/steinb/go/src/github.com/pyramids/keyval/memstore_test.go:91 +0x188
testing.(*B).runN(0x1854a000, 0x2710)
        /usr/lib/go/src/pkg/testing/benchmark.go:119 +0x7a
testing.(*B).launch(0x1854a000)
        /usr/lib/go/src/pkg/testing/benchmark.go:207 +0x12c
created by testing.(*B).run
        /usr/lib/go/src/pkg/testing/benchmark.go:170 +0x32

goroutine 9 [runnable]:
sync.(*RWMutex).Lock(0x18582a64)
        /usr/lib/go/src/pkg/sync/rwmutex.go:72
github.com/pyramids/keyval.MemStore.Set(0x1852e4a0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/steinb/go/src/github.com/pyramids/keyval/keyval.go:179 +0x5d
github.com/pyramids/keyval.(*MemStore).Set(0x1852efc0, 0x80f38c0, 0x2302, 0x80f38c0, 0xffffdcfe, ...)
        /home/steinb/go/src/github.com/pyramids/keyval/keyval.go:1 +0xa1
github.com/pyramids/keyval.setN(0xb77b89d8, 0x1852efc0, 0x2710, 0x1851c090)
        /home/steinb/go/src/github.com/pyramids/keyval/memstore_test.go:69 +0x5b
created by github.com/pyramids/keyval.BenchmarkMemStore
        /home/steinb/go/src/github.com/pyramids/keyval/memstore_test.go:90 +0x172
exit status 2
FAIL    github.com/pyramids/keyval      0.056s

Answer 1

Effective Go

Pointers vs. Values

The rule about pointers vs. values for receivers is that value methods can be invoked on pointers and values, but pointer methods can only be invoked on pointers. This is because pointer methods can modify the receiver; invoking them on a copy of the value would cause those modifications to be discarded.

To visibly modify the MemStore struct variable mutex field, use a pointer receiver. You are modifying a copy, which is invisible to other go routines. For example,

File keyval.go:

package keyval

import "sync"

type MemStore struct {
    data  map[interface{}]interface{}
    mutex sync.RWMutex
}

func NewMemStore() *MemStore {
    m := &MemStore{
        data: make(map[interface{}]interface{}),
        // mutex does not need initializing
    }
    return m
}

func (m *MemStore) Set(key interface{}, value interface{}) (err error) {
    m.mutex.Lock()
    defer m.mutex.Unlock()

    if value != nil {
        m.data[key] = value
    } else {
        delete(m.data, key)
    }

    return nil
}

File keyval_test.go:

package keyval

import "testing"

func setN(store *MemStore, N int, done chan<- struct{}) {
    for i := 0; i < N; i++ {
        store.Set(i, -i)
    }
    done <- struct{}{}
}

func BenchmarkMemStore(b *testing.B) {
    store := NewMemStore()
    done := make(chan struct{})
    b.ResetTimer()
    go setN(store, b.N, done)
    go setN(store, b.N, done)
    <-done
    <-done
}

Benchmark:

$ go test -v -bench=.
testing: warning: no tests to run
PASS
BenchmarkMemStore    1000000          1244 ns/op
ok      so/test 1.275s
$