Race condition even when using sync.Mutex in golang

Question

Complete code is here: https://play.golang.org/p/ggUoxtcv5m go run -race main.go says there is a race condition there which I fail to explain. The program outputs correct final result, though.

The essence:

type SafeCounter struct {
    c int
    sync.Mutex
}

func (c *SafeCounter) Add() {
    c.Lock()
    c.c++
    c.Unlock()
}

var counter *SafeCounter = &SafeCounter{} // global

use *SafeCounter in incrementor:

func incrementor(s string) {
    for i := 0; i < 20; i++ {
        x := counter
        x.Add()
        counter = x
    }
}

The incrementor method is spawned twice in main:

func main() {
    go incrementor()
    go incrementor()
    // some other non-really-related stuff like
    // using waitGroup is ommited here for problem showcase
}

So, as I said, go run -race main.go will always say there is a race cond found.

Also, the final result is always correct (at least I've run this program for a number of times and it always say final counter is 40, which is correct). BUT, the program prints incorrect values in the beginning so you can get something like:

Incrementor1: 0 Counter: 2
Incrementor2: 0 Counter: 3
Incrementor2: 1 Counter: 4
// ang the rest is ok

so, printing out 1 is missing there.

Can somebody explain why there is a race condition there is my code?

Answer 1

You have a number of race conditions, all pointed out specifically by the race detector:

    x := counter      // this reads the counter value without a lock
    fmt.Println(&x.c)
    x.Add()
    counter = x       // this writes the counter value without a lock
    time.Sleep(time.Duration(rand.Intn(3)) * time.Millisecond)
    fmt.Println(s, i, "Counter:", x.c) // this reads the c field without a lock

• race #1 is between the read and the write of the counter value in incrementor

• race #2 is between the concurrent writes to the counter value in incrementor

• race #3 is between the read of the x.c field in fmt.Println, and the increment to x.c in the Add method.

Answer 2

The two lines that read and write the counter pointer are not protected by the mutex and are done concurrently from multiple goroutines.

func incrementor(s string) {
    for i := 0; i < 20; i++ {
        x := counter  // <-- this pointer read
        x.Add()
        counter = x   // <-- races with this pointer write
    }
}