From: Vladimír Čunát Date: Fri, 3 Aug 2018 13:07:30 +0000 (+0200) Subject: lua cache bindings: error out if cache isn't open yet X-Git-Tag: v3.0.0~18^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1a16801bd6209a08173a8772dc009f26176effc;p=thirdparty%2Fknot-resolver.git lua cache bindings: error out if cache isn't open yet The catch is that during configuration file processing, no cache is open (yet), as kresd can't know if the config does open it in some later part (with non-default path or size). Now we just throw an error. Exceptions: - cache.open() and cache.backends(), of course :-) - cache.ns_tout() - not required, it's not really inside cache - cache.close() - it sounds reasonable to allow "closing a closed cache" This immediately caught a typo in cache metatable. --- diff --git a/NEWS b/NEWS index 751bd72ae..2bc662684 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,9 @@ +Incompatible changes +-------------------- +- cache: fail lua cache operations if it isn't open (!639) + Note that by default cache is open *after* reading the configuration, + and older versions were silently doing nothing. + Knot Resolver 2.4.1 (2018-08-02) ================================ diff --git a/daemon/bindings.c b/daemon/bindings.c index 18035e2b1..ee5ba62a2 100644 --- a/daemon/bindings.c +++ b/daemon/bindings.c @@ -848,6 +848,21 @@ int lib_net(lua_State *L) return 1; } + + +/** @internal return cache, or throw lua error if not open */ +struct kr_cache * cache_assert_open(lua_State *L) +{ + struct engine *engine = engine_luaget(L); + struct kr_cache *cache = &engine->resolver.cache; + assert(cache); + if (!cache || !kr_cache_is_open(cache)) { + format_error(L, "no cache is open yet, use cache.open() or cache.size, etc."); + lua_error(L); + } + return cache; +} + /** Return available cached backends. */ static int cache_backends(lua_State *L) { @@ -865,12 +880,10 @@ static int cache_backends(lua_State *L) /** Return number of cached records. */ static int cache_count(lua_State *L) { - struct engine *engine = engine_luaget(L); - const struct kr_cdb_api *api = engine->resolver.cache.api; + struct kr_cache *cache = cache_assert_open(L); - struct kr_cache *cache = &engine->resolver.cache; - int count = api->count(cache->db); - if (kr_cache_is_open(cache) && count >= 0) { + int count = cache->api->count(cache->db); + if (count >= 0) { /* First key is a version counter, omit it if nonempty. */ lua_pushinteger(L, count ? count - 1 : 0); return 1; @@ -881,8 +894,7 @@ static int cache_count(lua_State *L) /** Return time of last checkpoint, or re-set it if passed `true`. */ static int cache_checkpoint(lua_State *L) { - struct engine *engine = engine_luaget(L); - struct kr_cache *cache = &engine->resolver.cache; + struct kr_cache *cache = cache_assert_open(L); if (lua_gettop(L) == 0) { /* Return the current value. */ lua_newtable(L); @@ -908,8 +920,7 @@ static int cache_checkpoint(lua_State *L) /** Return cache statistics. */ static int cache_stats(lua_State *L) { - struct engine *engine = engine_luaget(L); - struct kr_cache *cache = &engine->resolver.cache; + struct kr_cache *cache = cache_assert_open(L); lua_newtable(L); lua_pushnumber(L, cache->stats.hit); lua_setfield(L, -2, "hit"); @@ -943,8 +954,7 @@ static const struct kr_cdb_api *cache_select(struct engine *engine, const char * static int cache_max_ttl(lua_State *L) { - struct engine *engine = engine_luaget(L); - struct kr_cache *cache = &engine->resolver.cache; + struct kr_cache *cache = cache_assert_open(L); int n = lua_gettop(L); if (n > 0) { @@ -967,8 +977,7 @@ static int cache_max_ttl(lua_State *L) static int cache_min_ttl(lua_State *L) { - struct engine *engine = engine_luaget(L); - struct kr_cache *cache = &engine->resolver.cache; + struct kr_cache *cache = cache_assert_open(L); int n = lua_gettop(L); if (n > 0) { @@ -1125,12 +1134,7 @@ static int cache_remove_prefix(struct kr_cache *cache, const char *args) /** Prune expired/invalid records. */ static int cache_prune(lua_State *L) { - struct engine *engine = engine_luaget(L); - struct kr_cache *cache = &engine->resolver.cache; - if (!kr_cache_is_open(cache)) { - return 0; - } - + struct kr_cache *cache = cache_assert_open(L); /* Check parameters */ int prune_max = UINT16_MAX; int n = lua_gettop(L); @@ -1155,11 +1159,7 @@ static int cache_prune(lua_State *L) /** Clear all records. */ static int cache_clear(lua_State *L) { - struct engine *engine = engine_luaget(L); - struct kr_cache *cache = &engine->resolver.cache; - if (!kr_cache_is_open(cache)) { - return 0; - } + struct kr_cache *cache = cache_assert_open(L); /* Check parameters */ const char *args = NULL; @@ -1187,6 +1187,7 @@ static int cache_clear(lua_State *L) } /* Clear reputation tables */ + struct engine *engine = engine_luaget(L); lru_reset(engine->resolver.cache_rtt); lru_reset(engine->resolver.cache_rep); lru_reset(engine->resolver.cache_cookie); @@ -1235,11 +1236,7 @@ static void cache_dump_key(lua_State *L, knot_db_val_t *key) /** Query cached records. */ static int cache_get(lua_State *L) { - struct engine *engine = engine_luaget(L); - struct kr_cache *cache = &engine->resolver.cache; - if (!kr_cache_is_open(cache)) { - return 0; - } + //struct kr_cache *cache = cache_assert_open(L); // to be fixed soon /* Check parameters */ int n = lua_gettop(L); @@ -1325,16 +1322,7 @@ static int cache_zone_import(lua_State *L) goto finish; } - struct engine *engine = engine_luaget(L); - if (!engine) { - strncpy(msg, "internal error, empty engine pointer", sizeof(msg)); - goto finish; - } - struct kr_cache *cache = &engine->resolver.cache; - if (!kr_cache_is_open(cache)) { - strncpy(msg, "cache isn't open", sizeof(msg)); - goto finish; - } + (void)cache_assert_open(L); /* just check it in advance */ /* Check parameters */ int n = lua_gettop(L); diff --git a/daemon/engine.c b/daemon/engine.c index 0e73747b4..2a2afa313 100644 --- a/daemon/engine.c +++ b/daemon/engine.c @@ -1020,6 +1020,7 @@ struct engine *engine_luaget(lua_State *L) { lua_getglobal(L, "__engine"); struct engine *engine = lua_touserdata(L, -1); + if (!engine) luaL_error(L, "internal error, empty engine pointer"); lua_pop(L, 1); return engine; } diff --git a/daemon/lua/sandbox.lua b/daemon/lua/sandbox.lua index 4d8d30816..489184637 100644 --- a/daemon/lua/sandbox.lua +++ b/daemon/lua/sandbox.lua @@ -163,7 +163,7 @@ setmetatable(modules, { setmetatable(cache, { __index = function (t, k) local res = rawget(t, k) - if res and not rawget(t, 'current_size') then return res end + if not res and not rawget(t, 'current_size') then return res end -- Beware: t.get returns empty table on failure to find. -- That would be confusing here (breaking kresc), so return nil instead. res = t.get(k) diff --git a/tests/config/cache.test.lua b/tests/config/cache.test.lua index 39548a87d..646eed7d8 100644 --- a/tests/config/cache.test.lua +++ b/tests/config/cache.test.lua @@ -13,10 +13,15 @@ local function test_properties() is(cache.min_ttl(), 1, 'stored minimum TTL') end --- test if the stats work with reopening the cache +-- test if the stats work with reopening the cache and operations fail with closed cache local function test_stats() ok(cache.close(), 'cache can be closed') boom(cache.open, {100 * MB, 'invalid://'}, 'cache cannot be opened with invalid backend') + + boom(cache.clear, {}, '.clear() does not work on closed cache') + boom(cache.count, {}, '.count() does not work on closed cache') + boom(cache.get, { 'key' }, '.get(...) does not work on closed cache') + ok(cache.open(100 * MB), 'cache can be reopened') local s = cache.stats() is(type(s), 'table', 'stats returns a table')