Код-ревью Senior sync и атомики обновлено
Код-ревью Go: кеш на RWMutex
Вопрос
Кеш с ленивой загрузкой значений. Найдите проблемы с блокировками и гонками.
type Cache struct {
mu sync.RWMutex
items map[string]string
hits int
}
func (c Cache) Get(key string) (string, bool) {
c.mu.RLock()
v, ok := c.items[key]
if ok {
c.hits++
}
c.mu.RUnlock()
return v, ok
}
func (c *Cache) Load(key string, fetch func(string) (string, error)) (string, error) {
c.mu.Lock()
defer c.mu.Unlock()
if v, ok := c.Get(key); ok {
return v, nil
}
v, err := fetch(key)
if err != nil {
return "", err
}
c.items[key] = v
return v, nil
}
Коротко
Исправленный вариант:
Что должен найти ревьюер
- Ошибкастрока 7Ресивер по значению копирует структуру вместе с мьютексом: блокировка берётся на копии и ничего не защищает, а
hitsувеличивается у копии. go vet (copylocks) предупреждает об этом. - Ошибкастрока 11Запись под RLock: несколько читателей одновременно меняют hits, это гонка данных. Счётчик должен быть atomic.Int64 или меняться под Lock.
- Ошибкастрока 20Get вызывается при удерживаемом Lock. С ресивером-указателем RLock внутри заблокировался бы навсегда (мьютекс не реентерабельный), а здесь копируется уже заблокированный мьютекс, и RLock на копии тоже повиснет: дедлок.
- Производительностьстрока 23Медленная загрузка (сеть, база) выполняется под эксклюзивной блокировкой: все читатели всех ключей ждут. Загружать нужно без блокировки, а одновременные промахи по одному ключу объединять через singleflight.
- Ошибкастрока 3Нулевое значение Cache непригодно: запись в nil-мапу вызовет panic. Нужен конструктор или ленивая инициализация под Lock.
Разбор
Исправленный вариант:
type Cache struct {
mu sync.RWMutex
items map[string]string
hits atomic.Int64
group singleflight.Group
}
func (c *Cache) Get(key string) (string, bool) {
c.mu.RLock()
v, ok := c.items[key]
c.mu.RUnlock()
if ok {
c.hits.Add(1)
}
return v, ok
}
func (c *Cache) Load(key string, fetch func(string) (string, error)) (string, error) {
if v, ok := c.Get(key); ok {
return v, nil
}
v, err, _ := c.group.Do(key, func() (any, error) { return fetch(key) })
if err != nil {
return "", err
}
c.mu.Lock()
if c.items == nil {
c.items = map[string]string{}
}
c.items[key] = v.(string)
c.mu.Unlock()
return v.(string), nil
}
Ответьте сами, а не прочитайте
В тренажёре вопрос попадёт в повторения и вернётся, когда начнёт забываться. Плюс билеты с зачётом: 20 вопросов, не больше 2 ошибок.