0

I'm a bit confused about how to ensure I safe concurrent access to nested maps. Originally my setup was like this, bit I realize I'd need to be able to lock at least one of the maps.

map[string]map[string]Dish

After some thought, the structures I am envisioning look like this:

type Manager struct {
    mu sync.RWMutex
    locations map[string]Restaurant
}
type Restaurant struct {
    mu sync.RWMutex
    menu map[string]Dish
}
type Dish struct {
    name string
    price string
    vegan bool
}

My basic understanding is as follows: If I want to add a new Restaurant to locations, I'd need to lock Manager. If I wanted to add or modify a Dish to menu, I'd need to lock Restaurant, but I'm not sure if I'd need to lock the Manager as well. Similarly, if I wanted to access values from Manager, I'm not sure if I'd need to lock Restaurant as well.

I've tried (unsuccessfully) to force data races while using the -race flag, so I'm not sure if locking Manager anytime Restaurant is mutated, and locking Restaurant everytime I access Manager is necessary, or if my attempts to force a race didn't work.

jimpeter
  • 61
  • 1
  • 4
  • Should not mutex to be pointer to work properly? U better add code to show how to you are locking and to ensure your are not locking different mutexes – Alexander Trakhimenok Feb 26 '20 at 15:54
  • @AlexanderTrakhimenok : https://stackoverflow.com/a/28244472/1286423 You should use a pointer to mutex. – Dolanor Feb 26 '20 at 16:51

1 Answers1

1

First, you don't want to be copying locks, so you need to use pointers:

type Manager struct {
    mu sync.RWMutex
    locations map[string]*Restaurant
}
type Restaurant struct {
    mu sync.RWMutex
    menu map[string]Dish
}

Once you have a *Restaurant instance, you have to lock it to write to menu, and rlock it to read from menu.

To get a restaurant by map lookup, you need to rlock the Manager, and to add things to the Manager, you need to lock it.

So if you want to start from the top and go to the bottom, you have to lock both the manager and then the restaurant. You also have to make sure you lock them in the same order (manager first, restaurant next) whenever you do this to avoid deadlocks.

Burak Serdar
  • 46,455
  • 3
  • 40
  • 59
  • Thank you, great point about the pointer. I was still in the mindset that "a map is a pointer" but Restaurant is a struct not a map. This answers my questions very succinctly, thank you very much! – jimpeter Feb 26 '20 at 15:59
  • @jimpeter you might also look at [`sync.Map`](https://golang.org/pkg/sync/#Map) but be sure you have read the caveats carefully. (Still, it's perfectly OK you implemented it yourself.) – kostix Feb 26 '20 at 16:40