From f986f462459d567b65d2d1af91829d4b34b3c8c7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Petr=20=C5=A0pa=C4=8Dek?= Date: Mon, 7 Sep 2020 16:08:05 +0200 Subject: [PATCH] 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. --- lib/cache/api.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cache/api.c b/lib/cache/api.c index d7794eca9..306aa603d 100644 --- 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. */ -- 2.47.2