从按值传递的结构值读取字段时发生数据争用

Question

Why does golang race detector complains about the following code:

package main

import (
    "fmt"
    "sync"
)

type Counter struct {
    value int
    mtx     *sync.Mutex
}

func NewCounter() *Counter {
    return &Counter {0, &sync.Mutex{}}
}

func (c *Counter) inc() {
    c.mtx.Lock()
    c.value++
    c.mtx.Unlock()
}

func (c Counter) get() int {
    c.mtx.Lock()
    res := c.value
    c.mtx.Unlock()
    return res
}

func main() {
    var wg sync.WaitGroup
    counter := NewCounter()
    max := 100
    wg.Add(max)

    // consumer
    go func() {
        for i := 0; i < max ; i++ {
            value := counter.get()
            fmt.Printf("counter value = %d
", value)
            wg.Done()
        }
    }()
    // producer
    go func() {
        for i := 0; i < max ; i++ {
            counter.inc()
        }
    }()

    wg.Wait()
}

When I run the code above with -race I'm getting the following warnings:

==================
WARNING: DATA RACE
Read at 0x00c0420042b0 by goroutine 6:
  main.main.func1()
      main.go:39 +0x72

Previous write at 0x00c0420042b0 by goroutine 7:
  main.(*Counter).inc()
      main.go:19 +0x8b
  main.main.func2()
      main.go:47 +0x50

Goroutine 6 (running) created at:
  main.main()
      main.go:43 +0x167

Goroutine 7 (running) created at:
  main.main()
      main.go:49 +0x192
==================

If I change func (c Counter) get() int to func (c *Counter) get() int then everything is working fine. It turns out that the receiver type for get() function should be a pointer. And I'm confused why that is. I'm aware of "-copylocks" but in this case mtx is a pointer, not value. If I change 'mtx' to be value and run program with vet -copylocks I get this warning:

main.go:23: get passes lock by value: main.Counter contains sync.Mutex`

That makes sense.

note: This question is not about how to implement thread safe counter

link to playground code

Answer 1

The race is because of the value receiver for the get() method. In order to call the get() method, a copy of the struct must be passed to the method expression. The method call without the syntactic sugar looks like:

value := Counter.get(*counter)

Copying the struct entails reading the value field, which happens before the method can take the lock, which is why the race is reported on the line of the method call, rather than within the method.

This is why changing the receiver to a pointer receiver will fix the issue. Also, since all receivers need to be pointers, the mtx can be left as a sync.Mutex value so it doesn't need to be initialized.