How do I use RWMutex when concurrently modifying a map while iterating over it

Question

I want to use a map's keys to request something from an API and then update the corresponding values based on the API's response.

My guess would be the following code.

Or maybe scratch this approach, collect the map-keys in an array before iterating and then use the array entries to make a request and modify the map

wg := &sync.WaitGroup{}
wg.Add(len(someMap))

sem := semaphore.NewWeighted(maxWorkers)
ctx := context.TODO()
mutex := &sync.RWMutex{}

mutex.RLock()
for k, v := range someMap {
    mutex.RUnlock()
    go func(k, v) {
        defer wg.Done()

        sem.Acquire(ctx, 1)
        res, err := API.REQUEST(k)
        sem.Release(1)

        if err != nil {
            return
        }

        v.SomeElement = res.SomeElement
        mutex.Lock()
        someMap[k] = v
        mutex.Unlock()
    }(k, v)
    mutex.RLock()
}
mutex.RUnlock()

wg.Wait()

While IIRC you can successfully lock around a range clause, the problem here is that you may or may not end up iterating over the new keys. From the spec: `If a map entry is created during iteration, that entry may be produced during the iteration or may be skipped`. I would just collect the keys you want to iterate over and make the code easier to understand in general — JimB, Jul 10 '20 at 21:03
Thanks for the answer! Using the extra array would simplify the code a good bit (which is preferable IMO). But I kind of don't like the idea of allocating unnecessary memory. Even though it could even speed up the code, since it'd get rid of all those RLock operations, also the map isn't really 'big'. And no entries will be created, since I only want to update existing entries. — Rico, Jul 10 '20 at 21:15
Well if the map isn't that big, then it's not a big deal to allocate a slice of the keys. Doing a single API call over the network is probably going to allocate orders of magnitude more. Beware of optimizing with no actual data. — JimB, Jul 10 '20 at 21:25
Related / possible duplicate of [Concurrent access to maps with 'range' in Go](https://stackoverflow.com/questions/40442846/concurrent-access-to-maps-with-range-in-go/40456170#40456170) — icza, Jul 10 '20 at 21:40

score 0 · Answer 1 · answered Jul 10 '20 at 21:06

What you're doing should work. However, note that between runlock and rlock all you do is create a goroutine, so you'll have a lot of goroutines waiting for the lock. First iterating the map and then modifying is easier to read and debug. Or, you can change the map and have map[keytype]struct{value *valuetype}, and then you don't need to lock the map, and run your goroutines without a need for mutexes. Each goroutine will modify map[k].value instead of map[k].

How do I use RWMutex when concurrently modifying a map while iterating over it

1 Answers1