CODEWITHSUNDEEP
go
Anarz
Anarz

Reputation: 25

Concurrent read/write of a map var snapshot

I encounter a situation that I can not understand. In my code, I use functions have the need to read a map (but not write, only loop through a snapshot of existing datas in this map). There is my code :

type MyStruct struct {
   *sync.RWMutex
   MyMap map[int]MyDatas
}

var MapVar = MyStruct{ &sync.RWMutex{}, make(map[int]MyDatas) }

func MyFunc() {
   MapVar.Lock()
   MapSnapshot := MapVar.MyMap
   MapVar.Unlock()
   for _, a := range MapSnapshot { // Map concurrent write/read occur here
      //Some stuff
   }
}

main() {
   go MyFunc()
}

The function "MyFunc" is run in a go routine, only once, there is no multiple runs of this func. Many other functions are accessing to the same "MapVar" with the same method and it randomly produce a "map concurrent write/read". I hope someone will explain to me why my code is wrong.

Thank you for your time.

edit: To clarify, I am just asking why my range MapSnapshot produce a concurrent map write/read. I cant understand how this map can be concurrently used since I save the real global var (MapVar) in a local var (MapSnapshot) using a sync mutex.

edit: Solved. To copy the content of a map in a new variable without using the same reference (and so to avoid map concurrent read/write), I must loop through it and write each index and content to a new map with a for loop.

Thanks xpare and nilsocket.

Upvotes: 0

Views: 421

Answers (1)

novalagung
novalagung

Reputation: 11532

there is no multiple runs of this func. Many other functions are accessing to the same "MapVar" with the same method and it randomly produce a "map concurrent write/read"

When you pass the value of MapVar.MyMap to MapSnapshot, the Map concurrent write/read will never be occur, because the operation is wrapped with mutex.

But on the loop, the error could happen since practically reading process is happening during loop. So better to wrap the loop with mutex as well.

MapVar.Lock() // lock begin

MapSnapshot := MapVar.MyMap
for _, a := range MapSnapshot {
   // Map concurrent write/read occur here
   // Some stuff
}

MapVar.Unlock() // lock end

UPDATE 1

Here is my response to your argument below:

This for loop takes a lot of time, there is many stuff in this loop, so locking will slow down other routines

As per your statement The function "MyFunc" is run in a go routine, only once, there is no multiple runs of this func, then I think making the MyFunc to be executed as goroutine is not a good choice.

And to increase the performance, better to make the process inside the loop to be executed in a goroutine.

func MyFunc() {
    for _, a := range MapVar.MyMap {
        go func(a MyDatas) {
           // do stuff here
        }(a)
    }
}

main() {
    MyFunc() // remove the go keyword
}

UPDATE 2

If you really want to copy the MapVar.MyMap into another object, passing it to another variable will not solve that (map is different type compared to int, float32 or other primitive type).

Please refer to this thread How to copy a map?

Upvotes: 0

Related Questions