Gotchas of storing mutexes in map

Getting Help Code Review
1 
    var mutexes map[string]*sync.Mutex
    var m *sync.Mutex
	if mForID, ok := mutexes[ID]; ok {
		m = mForID
	} else {
		m = &sync.Mutex{}
		mutexes[ID] = m
	}
	m.Lock()
	defer m.Unlock()

    {
    // Code here I want to protect from concurrent access, if the ID is the same.
    // Read some data
    // Mutate it
    // Write the data back
    }

What I am trying to do is prevent data races between go routines that are using the same ID. If the ID is different, there is no problem, but if a go routine is using the same ID as another one, then it must wait to access, modify, and write the data, until the other go routine is finished.

I’ve tested this code and it seems to work as I want it to. If I comment out the defer m.Unlock() line then make two request with the same ID, the second request is permanently locked. Then if I make a third request with a different ID it is allowed to proceed, which is the functionality I am looking for.

Are there any gotchas with the code I have written or a better way to have mutexes that are unique to a specific ID?

I also probably need a way to delete the mutex from the map after a certain period of time.

Thanks for any help you can offer me.

2

I think you’re overthinking it. How about running a separate goroutine for every ID and publish messages to the channel?

1 Like
3

You need a mutex to protect your mutexes map because mutating a map is not “goroutine safe.” What is it you’re trying to do, though? A threadsafe map of mutexes looks like something that might be solvable instead with channels.

1 Like
4

Thank you for your reply!

Yes after reading Sean and your replies I believe a channel is the way to go.

I’m going to do some thinking and try to get a solution with channels working. I seem to have a hard time determining when to use channels vs mutexes.

Thanks again for pointing me in the right direction :grin:

5

Thanks for taking the time to help me out!

Ah I see. So each go routine, regardless of the ID, would have to “wait in line” to access the map.

This is what I’m trying to do:
My server is receiving simultaneous http requests. My server has to do 3 things after it receives one of these requests.

  1. Retrieve a file from bucket storage using the ID contained in the body of the request.
  2. Merge data contained in the body of the request into this file.
  3. Save the file back into bucket storage.

Here’s a terrible drawing of my scenario

2020-06-22 08.57.17
2020-06-22 08.57.173024Ă—4032 3.16 MB

The map of mutexes was my attempt to prevent a data race between two or more requests coming into my server at the same time with the same ID.

Bartłomiej and you have both mentioned this would probably be a better use case of channels so I am going to try implementing a solution using them.

Thanks again for your help!

6

I mean, you can do it both in mutexes and goroutines but I noticed you don’t fully understand how to use them in this scenario.

The very first thing is to create a function that does what you want

func doMyJob(id string) {
    // get file from the S3 bucket
    // modify the data
   // upload the file again to S3 bucket
}

and you can use both mutex and goroutines here, to be honest. Just choose the simplest way for you.

1 Like