luci / gae / 145

82%

push

travis-ci

Fix memory corruption bug in impl/memory

When you `SetCollection` in gkvlite, it invalidates the old
collection, but keeps the Items of the previous collection. The way
that impl/memory was using SetCollection was more along the lines of
GetOrCreate, which could cause corruption or other issues if two
codepaths did SetCollection simultaneously.

This fixes the corruption bug by removing SetCollection from the
memStore interface and replaces it with a GetOrCreateCollection,
which checks for the collection, and only calls SetCollection if it
doesn't exist. As a bonus, this seems to give a slight performance
speedup over the previous broken code.

Additionally, this forces all uses of VisitItemsAscend to use
a Snapshot, to avoid accidental modification of a Collection while
its being iterated over, which can lead to the situation discussed
above, or other nasty errors.

Finally, this adds an optional 'gkvlite tracer' functionality
which came in (extremely) handy for finding this bug. When given
a folder, it will emit go-code-like invocations of the functions run
on every collection, and includes enough information to reveal which
functions are being called on the root collection v. snapshots, and
if there are any overlapping VisitItemsAscend invocations, etc.

R=dnj@chromium.org
BUG=550684

Review URL: https://codereview.chromium.org/1911263002

211 of 211 new or added lines in 8 files covered. (100.0%)

6498 of 7957 relevant lines covered (81.66%)

1287.4 hits per line