Is this a right way to update a global golang map with concurrent read?

Question

this is a fatal error: concurrent map read and map write demo code:

package main

var Map = make(map[int]int)

func main() {

    for i := 0; i < 100000; i++ {
        go updateMap()
        go readMap(i)
    }

}

func readMap(key int) int {
    return Map[key]
}

func updateMap() {
    for j := 0; j < 1000; j++ {
        Map[j] = j
    }
}

I don't use the lock or sync.Map, just use tmp var to replace global map, it will not panic, codes like this:

package main

var Map = make(map[int]int)

func main() {

    for i := 0; i < 100000; i++ {
        go updateMap()
        go readMap(i)
    }

}

func readMap(key int) int {
    return Map[key]
}

func updateMap() {
    tmp := map[int]int{}
    for j := 0; j < 1000; j++ {
        tmp[j] = j
    }
    Map = tmp
}

Is there any problem with this way to update the global map?

Depends on what you want. The second code won't panic but you don't have any guarantees with the value you will read either. Highly recommend `go run -race` if eliminating race conditions is a concern. — Kelsnare, Apr 14 '22 at 04:43
To be sure you should use a waitgroup and wait for all goroutines to end properly before finish your program. — Tiago Peczenyj, Apr 14 '22 at 07:41

score 1 · Answer 1 · answered Apr 14 '22 at 04:14

The second way of updating the map will not panic, because the map being updated is a temporary map, but there is still a race condition. After the updateMap function, there is no guarantee when the other goroutine will see the effects of updateMap, if ever.

The correct way is to use a RWMutex for both cases. The readMap should use RLock/RUnlock, and updateMap should use Lock/Unlock. For the second case, if the updateMap the only goroutine that ever updates the map, you can use Rlock/RUnlock during the copy phase, and Lock/Unlock when you assign the map.

Is this a right way to update a global golang map with concurrent read?

1 Answers1