cache: fix race in assert_right_version

This change fixes race condition in assert_right_version(). Racy
situation:
- Two instances have the (empty) cache open: New binary and old binary.
- New binary executes count() inside assert_right_version(), which
  internally starts RO transaction. Returned count is 0.
- Old binary does some writes (RW transaction parallel to RO in the first
  process).
- New binary skips cache clear because cache was empty at the time of check.
- Result: The old binary wrote data with an old format into cache which
  was not cleared and silenty changed version number to a new one.

This is not complete fix because we lack mechanism to detect cache format
change at run-time, but at least it removes one nasty corner case and
cost of this change seems to be minimal.

diff --git a/lib/cache/api.c b/lib/cache/api.c
index d7794eca988d3802cf02771dec1c80a8052eb095..306aa603dd299cebc4a681d75c87106232cf9e3f 100644 (file)
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -77,9 +77,10 @@ static int assert_right_version(struct kr_cache *cache)
                ret = kr_ok();
        } else {
                int oldret = ret;
-               /* Version doesn't match. Recreate cache and write version key. */
+               /* Version doesn't match or we were unable to read it, possibly because DB is empty.
+                * Recreate cache and write version key. */
                ret = cache_op(cache, count);
-               if (ret != 0) { /* Non-empty cache, purge it. */
+               if (ret != 0) { /* Log for non-empty cache to limit noise on fresh start. */
                        kr_log_info("[cache] incompatible cache database detected, purging\n");
                        if (oldret) {
                                kr_log_verbose("[cache] reading version returned: %d\n", oldret);
@@ -91,9 +92,8 @@ static int assert_right_version(struct kr_cache *cache)
                                kr_log_verbose("[cache] version has bad value: %d instead of %d\n",
                                        (int)ver, (int)CACHE_VERSION);
                        }
-                       ret = cache_op(cache, clear);
                }
-               /* Either purged or empty. */
+               ret = cache_op(cache, clear);
        }
        /* Rewrite the entry even if it isn't needed.  Because of cache-size-changing
         * possibility it's good to always perform some write during opening of cache. */