1

Why does this code cause data race? I have already used atomic add.

package main

import (
    "sync/atomic"
    "time"
)

var a int64

func main() {
    for {
        if a < 100 {
            atomic.AddInt64(&a, 1)
            go run()
        }
    }
}

func run() {
    <-time.After(5 * time.Second)
    atomic.AddInt64(&a, -1)
}

I run command go run --race with this code and get:

==================
WARNING: DATA RACE
Write at 0x000001150f30 by goroutine 8:
  sync/atomic.AddInt64()
      /usr/local/Cellar/go/1.11.2/libexec/src/runtime/race_amd64.s:276 +0xb
  main.run()
      /Users/flask/test.go:22 +0x6d

Previous read at 0x000001150f30 by main goroutine:
  main.main()
      /Users/flask/test.go:12 +0x3a

Goroutine 8 (running) created at:
  main.main()
      /Users/flask/test.go:15 +0x75
==================

Could you help me explain this? And how to fix this warning? Thanks!

Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
fatDuo
  • 23
  • 2

1 Answers1

6

You didn't use the atomic package at all places where you accessed the variable. All access must be synchronized to variables that are accessed from multiple goroutines concurrently, including reads:

for {
    if value := atomic.LoadInt64(&a); value < 100 {
        atomic.AddInt64(&a, 1)
        go run()
    }
}

With that change, the race condition goes away.

If you just want to inspect the value, you don't even need to store it in a variable, so you may simply do:

for {
    if atomic.LoadInt64(&a) < 100 {
        atomic.AddInt64(&a, 1)
        go run()
    }
}
icza
  • 389,944
  • 63
  • 907
  • 827
  • `a` can still be concurrently modified between `LoadInt64` and `AddInt64`, yes? (Even if the Go race detector doesn't explicitly flag this as a data race.) You might need a `CompareAndSwapInt64` to be safe. – David Maze Jan 08 '19 at 11:31
  • @DavidMaze Yes, you're right. I was only to show that the data race was because of the unsynchronized read of the variable `a`. Although in the example `a` will never grow beyond `100`, because the only other modification is a decrementing change. If more control is required, I'd use a `sync.Mutex` instead. – icza Jan 08 '19 at 11:43
  • it is a shame I didn't know that. Anyway, you saved my day, thanks! – Sergii Getman Nov 04 '22 at 14:18