Iterating a RWMutex-protected map and making changes in the loop

Getting Help
1

I have a map that is protected by sync.RWMutex. Every once in a while I iterate it and check if any of the values need to be updated. If so, I update the value. The problem is that getting the new value can take a long time (several seconds). Is this safe to do?

mapMutex.RLock()
for key, val := range myMap {
    if val.something {
        mapMutex.RUnlock()
        newVal := longOperation()
        mapMutex.Lock()
        myMap[key] = newVal
        mapMutex.Unlock()
        mapMutex.RLock()
    }
}
mapMutex.RUnlock()

Or is there a better way to do this without creating lock contention? The map is read-heavy, but I can’t be blocking readers for several seconds either.

(I should clarify that only this goroutine is changing these entries in the map; other goroutines may be adding to the map or changing OTHER entries in the map, but not the ones this does.)

2

This is not safe. After the read lock inside if (mapMutex.RUnlock) is released the map can be modified by another thread. After that you cannot carry on iterating over the map.

To reduce time the write lock is held you can do something like:

mapMutex.RLock()
for key, val := range myMap {
    if val.something {
        updatesMap[key] = longOperation()
    }
}
mapMutex.RULock()

mapMutex.Lock()
for key, val := range myMap {
    if newVal, ok := updatesMap[key]; ok {
        myMap[key] = newVal
    }
}
mapMutex.Unlock()

Alternatevely you can hold myMap in atomic.Value and re-create the whole map on write.

7 Likes
3

I agree with Konstantin. However, it would also be a good idea to write a concurrent test and run it with -race.

4

Good idea – I basically came up with the same solution (the updates map) independently, which you now have verified! Thanks.

Yeah… I’ll work on that next. Cheers!

5

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.