impl/memory: Fix memcache stats race.

impl/memory: Fix memcache stats race.

Fix a race in memcache stats that was caused by modification of common
stats varibles while holding a possibly-shared RLock. This fix switches
locking from RWMutex to Mutex, since potentially every access contains
mutations to the shared state.

BUG=None
TEST=local
  - Re-ran external test with fix, race no longer triggered.

R=iannucci@chromium.org

Committed: https://github.com/luci/gae/commit/c12ab6ff5cb4e6d9f78c89a172f8cc08bcdf56d5

[dnj, 2016-12-06 06:22:45]

PTAL. Stupid problem, stupid fix.

I actually implemented a pretty nice sync/atomic solution and kept the RWLock,
but abandoned it when I realized how many 64-bit-aligned values I'd have to
maintain in Statistics in order to satisfy the sync/atomic 32-bit happiness
caveat.

We could split stats lock out separately, but that seems like no better than
this approach, since all paths will hit the same lock anyway.

[iannucci, 2016-12-06 06:25:33]

Hm... I'm not sure I understand WHY there was a race, or why this fixes it, but
lgtm.

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

https://codereview.chromium.org/2554793002/diff/1/impl/memory/memcache.go
File impl/memory/memcache.go (right):

https://codereview.chromium.org/2554793002/diff/1/impl/memory/memcache.go#new...
impl/memory/memcache.go:110: m.stats.Items++
All mutations to "m.stats" while holding RLock are races.

[dnj, 2016-12-06 17:24:43]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2016-12-06 17:24:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 17:29:01]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481045083822860, "parent_rev":
"794153648435d6a4d6c77320da2ac10948e8800c", "commit_rev":
"c12ab6ff5cb4e6d9f78c89a172f8cc08bcdf56d5"}

[commit-bot: I haz the power, 2016-12-06 17:29:04]

Description was changed from

==========
impl/memory: Fix memcache stats race.

Fix a race in memcache stats that was caused by modification of common
stats varibles while holding a possibly-shared RLock. This fix switches
locking from RWMutex to Mutex, since potentially every access contains
mutations to the shared state.

BUG=None
TEST=local
  - Re-ran external test with fix, race no longer triggered.

R=iannucci@chromium.org
==========

to

==========
impl/memory: Fix memcache stats race.

Fix a race in memcache stats that was caused by modification of common
stats varibles while holding a possibly-shared RLock. This fix switches
locking from RWMutex to Mutex, since potentially every access contains
mutations to the shared state.

BUG=None
TEST=local
  - Re-ran external test with fix, race no longer triggered.

R=iannucci@chromium.org

Committed:
https://github.com/luci/gae/commit/c12ab6ff5cb4e6d9f78c89a172f8cc08bcdf56d5
==========

[commit-bot: I haz the power, 2016-12-06 17:29:05]

Committed patchset #1 (id:1) as
https://github.com/luci/gae/commit/c12ab6ff5cb4e6d9f78c89a172f8cc08bcdf56d5