False-positive in the Golang Race Detector? [repost] [duplicate]

Question

I had posted this question a few days ago and it got closed as there was an error in the code. Got that fixed up and so re-posting this

package main

import (
    "fmt"
    "time"
    "sync/atomic"
    "math/rand"
)
//This data is normally fetched via HTTP Request
var dummyData1 = []string{"a", "b", "c", "d", "e", "f"}
var activeMap = new(int32)
var map1 = make(map[string]*int32)
var map2 map[string]*int32
var combinedMap = make(map[string]*int32)

func mapKeyUpdater () {
    for _, key := range dummyData1 {
        combinedMap[key] = new(int32)
        map1[key] = new(int32)
    }
    atomic.AddInt32(activeMap, 1)
    time.Sleep(3 * time.Second)
    for {
        if atomic.LoadInt32(activeMap) == 1 {
            map2 = make(map[string]*int32)
            for _, key := range dummyData1 {
                map2[key] = new(int32)
            }
            atomic.AddInt32(activeMap, 1)
            time.Sleep(500 * time.Millisecond) //Added after EDIT. See below
            for key, count := range map1{
                *combinedMap[key] += *count
            }
        } else {
            map1 = make(map[string]*int32)
            for _, key := range dummyData1 {
                map1[key] = new(int32)
            }
            atomic.AddInt32(activeMap, -1)
            time.Sleep(500 * time.Millisecond) //Added after EDIT. See below
            for key, count := range map2 {
                *combinedMap[key] += *count
            }
        }
        time.Sleep(3 * time.Second)
    }
}

func counter () {
    for {
        randomIndex := rand.Intn(5)
        randomKey := dummyData1[randomIndex]
        if atomic.LoadInt32(activeMap) == 1 {
            val := atomic.AddInt32(map1[randomKey], 100)
            fmt.Printf("Added 100 to %v in Map1. Updated value %v\n", randomKey, val)
        } else {
            val := atomic.AddInt32(map2[randomKey], 100)
            fmt.Printf("Added 100 to %v in Map2. Updated value %v\n", randomKey, val)
        }
    }
}

func main () {
    go mapKeyUpdater()
    time.Sleep(500 * time.Millisecond)
    go counter()
    time.Sleep(15 * time.Second)
}

Now when I run this with the command go run -race raceBug.go I get 4 Race's each time. However, it is clear from the output that there is no race, and the maps are working as intended

==================
Added 100 to e in Map2. Updated value 7990900
WARNING: DATA RACE
Write at 0x0000011cdbd0 by goroutine 7:
Added 100 to a in Map2. Updated value 7972000
  main.mapKeyUpdater()
      /raceBug.go:34 +0x14d

Previous read at 0x0000011cdbd0 by goroutine 9:
Added 100 to e in Map2. Updated value 7991000
  [failed to restore the stack]

Goroutine 7 (running) created at:
  main.main()
      /raceBug.go:62 +0x29
Added 100 to e in Map2. Updated value 7991100

Goroutine 9 (running) created at:
  main.main()
      /raceBug.go:64 +0x44
==================
Added 100 to c in Map2. Updated value 7956400
Added 100 to b in Map2. Updated value 7993400
==================
WARNING: DATA RACE
Added 100 to e in Map1. Updated value 100
Read at 0x00c00001acec by goroutine 7:
  main.mapKeyUpdater()
      /raceBug.go:40 +0x2d4

Added 100 to c in Map1. Updated value 100
Previous write at 0x00c00001acec by goroutine 9:
  sync/atomic.AddInt32()
      /usr/local/Cellar/go/1.18/libexec/src/runtime/race_amd64.s:279 +0xb
  sync/atomic.AddInt32()
      <autogenerated>:1 +0x1a
Added 100 to d in Map1. Updated value 100

Goroutine 7 (running) created at:
  main.main()
      /raceBug.go:62 +0x29

Added 100 to b in Map1. Updated value 100
Goroutine 9 (running) created at:
  main.main()
      /raceBug.go:64 +0x44
==================

This article by an engineer at Google says - https://medium.com/@val_deleplace/does-the-race-detector-catch-all-data-races-1afed51d57fb

If you strongly believe that you witnessed a false positive, then report a bug for the race detector. If you have good reasons to believe that the race condition was caused by the standard library or by the runtime (rather than your own code), then report a bug for the standard library or the runtime.

As I am still pretty new at Go, just want to get some confirmation of this.

EDIT: Just to make sure that the combinedMap loop has enough time before it starts, I added a time.Sleep(500 * time.Millisecond). However the Race is still detected, but the output is now different.

New Output

==================
WARNING: DATA RACEAdded 100 to e in Map2. Updated value 9447300

Write at 0x0000011cdbd0 by goroutine 7:
Added 100 to c in Map2. Updated value 9465100
  main.mapKeyUpdater()
      /raceBug2.go:35 +0x14d

Previous read at 0x0000011cdbd0 by goroutine 9:
Added 100 to b in Map2. Updated value 9461300
  [failed to restore the stack]

Goroutine 7 (running) created at:
  main.main()
      /raceBug2.go:64 +0x29
Added 100 to d in Map2. Updated value 9479400

Goroutine 9 (running) created at:
  main.main()
      /raceBug2.go:66 +0x44
Added 100 to c in Map2. Updated value 9465200
==================

Answer 1

A data race happens when two goroutines access the same variable concurrently, and at least one of the accesses is a write.

In your code, the global vars map1 and map2 are accessed by the two goroutines. Using atomic package to read and operate the map item is not enough because the map items value (pointer of int32) are changed when recreating the map in mapKeyUpdater. Use a Mutex to lock these two maps to eliminate race.

package main

import (
    "fmt"
    "math/rand"
    "sync"
    "sync/atomic"
    "time"
)

//This data is normally fetched via HTTP Request
var dummyData1 = []string{"a", "b", "c", "d", "e", "f"}
var activeMap = new(int32)
var combinedMap = make(map[string]*int32)

type myMap struct {
    mutex sync.RWMutex
    value map[string]*int32
}

var (
    map1 = myMap{
        value: make(map[string]*int32),
    }
    map2 = myMap{}
)

func mapKeyUpdater() {
    for _, key := range dummyData1 {
        combinedMap[key] = new(int32)
        map1.mutex.Lock()
        map1.value[key] = new(int32)
        map1.mutex.Unlock()
    }
    atomic.AddInt32(activeMap, 1)
    time.Sleep(3 * time.Second)
    for {
        if atomic.LoadInt32(activeMap) == 1 {
            map2.mutex.Lock()
            map2.value = make(map[string]*int32)
            for _, key := range dummyData1 {
                map2.value[key] = new(int32)
            }
            map2.mutex.Unlock()
            atomic.AddInt32(activeMap, 1)
            time.Sleep(500 * time.Millisecond) //Added after EDIT. See below
            map1.mutex.Lock()
            for key, count := range map1.value {
                *combinedMap[key] += *count
            }
            map1.mutex.Unlock()
        } else {
            map1.mutex.Lock()
            for _, key := range dummyData1 {
                map1.value[key] = new(int32)
            }
            map1.mutex.Unlock()
            atomic.AddInt32(activeMap, -1)
            time.Sleep(500 * time.Millisecond) //Added after EDIT. See below
            map2.mutex.Lock()
            for key, count := range map2.value {
                *combinedMap[key] += *count
            }
            map2.mutex.Unlock()
        }
        time.Sleep(3 * time.Second)
    }
}

func counter() {
    for {
        randomIndex := rand.Intn(5)
        randomKey := dummyData1[randomIndex]
        if atomic.LoadInt32(activeMap) == 1 {
            map1.mutex.Lock()
            val := atomic.AddInt32(map1.value[randomKey], 100)
            map1.mutex.Unlock()
            fmt.Printf("Added 100 to %v in Map1. Updated value %v\n", randomKey, val)
        } else {
            map2.mutex.Lock()
            val := atomic.AddInt32(map2.value[randomKey], 100)
            map2.mutex.Unlock()
            fmt.Printf("Added 100 to %v in Map2. Updated value %v\n", randomKey, val)
        }
    }
}

func main() {
    go mapKeyUpdater()
    time.Sleep(500 * time.Millisecond)
    go counter()
    time.Sleep(15 * time.Second)
}

It should be avoided to using shared variables (like global var) to send value to multiple goroutines. Use channel is the recommended way. Note: using channel to pass pointer is not safe either. Just send value via channel.

Do not communicate by sharing memory; instead, share memory by communicating.