impl/memory: Fix memcache stats race.

impl/memory/memcache.go

 // Copyright 2015 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package memory
 
 import (
         "encoding/binary"
         "sync"
         "time"
@@ skipping 60 matching lines @@
 func (m *mcDataItem) toUserItem(key string) *mcItem {
         value := make([]byte, len(m.value))
         copy(value, m.value)
         // Expiration is defined to be 0 when retrieving items from memcache.
         //   https://cloud.google.com/appengine/docs/go/memcache/reference#Item
         // ¯\_(ツ)_/¯
         return &mcItem{key, value, m.flags, 0, m.casID}
 }
 
 type memcacheData struct {
-        lock  sync.RWMutex
+        lock  sync.Mutex
         items map[string]*mcDataItem
         casID uint64
 
         stats mc.Statistics
 }
 
 func (m *memcacheData) mkDataItemLocked(now time.Time, i mc.Item) (ret *mcDataItem) {
         m.casID++
 
         exp := time.Time{}
         if i.Expiration() != 0 {
                 exp = now.Add(i.Expiration()).Truncate(time.Second)
         }
         value := make([]byte, len(i.Value()))
         copy(value, i.Value())
         return &mcDataItem{
                 flags:      i.Flags(),
                 expiration: exp,
                 value:      value,
                 casID:      m.casID,
         }
 }
 
 func (m *memcacheData) setItemLocked(now time.Time, i mc.Item) {
         if cur, ok := m.items[i.Key()]; ok {
                 m.stats.Items--
                 m.stats.Bytes -= uint64(len(cur.value))
         }
         m.stats.Items++

[dnj, 2016/12/06 17:24:06]

All mutations to "m.stats" while holding RLock are races.

         m.stats.Bytes += uint64(len(i.Value()))
         m.items[i.Key()] = m.mkDataItemLocked(now, i)
 }
 
 func (m *memcacheData) delItemLocked(k string) {
         if itm, ok := m.items[k]; ok {
                 m.stats.Items--
                 m.stats.Bytes -= uint64(len(itm.value))
                 delete(m.items, k)
         }
@@ skipping 130 matching lines @@
 }
 
 func (m *memcacheImpl) GetMulti(keys []string, cb mc.RawItemCB) error {
         now := clock.Now(m.ctx)
 
         itms := make([]mc.Item, len(keys))
         errs := make([]error, len(keys))
 
         for i, k := range keys {
                 itms[i], errs[i] = func() (mc.Item, error) {
-                        m.data.lock.RLock()
-                        defer m.data.lock.RUnlock()
+                        m.data.lock.Lock()
+                        defer m.data.lock.Unlock()
                         val, err := m.data.retrieveLocked(now, k)
                         if err != nil {
                                 return nil, err
                         }
                         return val.toUserItem(k), nil
                 }()
         }
 
         for i, itm := range itms {
                 cb(itm, errs[i])
@@ skipping 65 matching lines @@
         }
 
         newval := make([]byte, 8)
         binary.LittleEndian.PutUint64(newval, cur)
         m.data.setItemLocked(now, m.NewItem(key).SetValue(newval))
 
         return cur, nil
 }
 
 func (m *memcacheImpl) Stats() (*mc.Statistics, error) {
-        m.data.lock.RLock()
-        defer m.data.lock.RUnlock()
+        m.data.lock.Lock()
+        defer m.data.lock.Unlock()
 
         ret := m.data.stats
         return &ret, nil
 }