From 129002fc0da921ef1274cd7a25d605b87691eaf5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= Date: Thu, 4 Apr 2019 17:04:27 +0200 Subject: [PATCH] modules: tidy (de)initialization code --- daemon/bindings/impl.h | 2 +- daemon/engine.c | 56 ++++++++++++++++++++++++---------- daemon/ffimodule.c | 63 +++++++++++++++++++-------------------- daemon/lua/sandbox.lua.in | 4 +++ lib/module.h | 30 +++++++++++++------ 5 files changed, 97 insertions(+), 58 deletions(-) diff --git a/daemon/bindings/impl.h b/daemon/bindings/impl.h index 860bc612f..2e5445e71 100644 --- a/daemon/bindings/impl.h +++ b/daemon/bindings/impl.h @@ -74,7 +74,7 @@ static inline int execute_callback(lua_State *L, int argc) { int ret = engine_pcall(L, argc); if (ret != 0) { - fprintf(stderr, "error: %s\n", lua_tostring(L, -1)); + kr_log_error("error: %s\n", lua_tostring(L, -1)); } /* Clear the stack, there may be event a/o enything returned */ lua_settop(L, 0); diff --git a/daemon/engine.c b/daemon/engine.c index 73823f9ff..7f3402109 100644 --- a/daemon/engine.c +++ b/daemon/engine.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -606,8 +607,8 @@ int engine_init(struct engine *engine, knot_mm_t *pool) /** Unregister a (found) module */ static void engine_unload(struct engine *engine, struct kr_module *module) { - auto_free char *name = strdup(module->name); - kr_module_unload(module); + auto_free char *name = module->name ? strdup(module->name) : NULL; + kr_module_unload(module); /* beware: lua/C mix, could be confusing */ /* Clear in Lua world, but not for embedded modules ('cache' in particular). */ if (name && !kr_module_get_embedded(name)) { lua_pushnil(engine->L); @@ -767,6 +768,7 @@ static size_t module_find(module_array_t *mod_list, const char *name) int engine_register(struct engine *engine, const char *name, const char *precedence, const char* ref) { if (engine == NULL || name == NULL) { + assert(!EINVAL); return kr_error(EINVAL); } /* Make sure module is unloaded */ @@ -785,33 +787,57 @@ int engine_register(struct engine *engine, const char *name, const char *precede if (!module) { return kr_error(ENOMEM); } - module->data = engine; - /* TODO: tidy and comment this section. */ + module->data = engine; /*< some outside modules may still use this value */ + int ret = kr_module_load(module, name, LIBDIR "/kres_modules"); - if (ret == kr_ok()) { - lua_getglobal(engine->L, "modules_create_table_for_c"); - lua_pushpointer(engine->L, module); - if (engine_pcall(engine->L, 1) != 0) { - lua_pop(engine->L, 1); + if (ret == 0) { + /* We have a C module, loaded and init() was called. + * Now we need to prepare the lua side. */ + lua_State *L = engine->L; + lua_getglobal(L, "modules_create_table_for_c"); + lua_pushpointer(L, module); + if (lua_isnil(L, -2)) { + /* When loading the three embedded modules, we don't + * have the "modules_*" lua function yet, but fortunately + * we don't need it there. Let's just check they're embedded. + * TODO: solve this better *without* breaking stuff. */ + lua_pop(L, 2); + if (module->lib != RTLD_DEFAULT) { + ret = kr_error(1); + lua_pushliteral(L, "missing modules_create_table_for_c()"); + } + } else { + ret = engine_pcall(L, 1); } - } - /* Load Lua module if not a binary */ - if (ret == kr_error(ENOENT)) { + if (ret) { + kr_log_error("[system] internal error when loading C module %s: %s\n", + module->name, lua_tostring(L, -1)); + lua_pop(L, 1); + assert(false); /* probably not critical, but weird */ + } + + } else if (ret == kr_error(ENOENT)) { + /* No luck with C module, so try to load and .init() lua module. */ ret = ffimodule_register_lua(engine, module, name); + if (ret != 0) { + kr_log_error("[system] failed to load module '%s'\n", name); + } + } else if (ret == kr_error(ENOTSUP)) { /* Print a more helpful message when module is linked against an old resolver ABI. */ - fprintf(stderr, "[system] module '%s' links to unsupported ABI, please rebuild it\n", name); + kr_log_error("[system] module '%s' links to unsupported ABI, please rebuild it\n", name); } + if (ret != 0) { - free(module); + engine_unload(engine, module); return ret; } + /* Push to the right place in engine->modules */ if (array_push(engine->modules, module) < 0) { engine_unload(engine, module); return kr_error(ENOMEM); } - /* Evaluate precedence operator */ if (precedence) { struct kr_module **arr = mod_list->at; size_t emplacement = mod_list->len; diff --git a/daemon/ffimodule.c b/daemon/ffimodule.c index f9f6b01ed..bd766f200 100644 --- a/daemon/ffimodule.c +++ b/daemon/ffimodule.c @@ -41,20 +41,6 @@ enum { /** Lua registry indices for functions that wrap layer callbacks (shared by all lua modules). */ static int l_ffi_wrap_slots[SLOT_count] = { 0 }; -/** @internal Helper for retrieving the right function entrypoint. */ -static inline lua_State *l_ffi_preface(struct kr_module *module, const char *call) { - lua_State *L = module->lib; - lua_getglobal(L, module->name); - lua_getfield(L, -1, call); - lua_remove(L, -2); - if (lua_isnil(L, -1)) { - lua_pop(L, 1); - return NULL; - } - lua_pushlightuserdata(L, module); - return L; -} - /** @internal Continue with coroutine. */ static void l_ffi_resume_cb(uv_idle_t *check) { @@ -91,30 +77,41 @@ static int l_ffi_call_mod(lua_State *L, int argc) if (lua_isnumber(L, -1)) { /* Return code */ status = lua_tointeger(L, -1); } else if (lua_isthread(L, -1)) { /* Continuations */ + /* TODO: unused, possibly in a bad shape. Meant KR_STATE_YIELD? */ + assert(!ENOTSUP); status = l_ffi_defer(lua_tothread(L, -1)); } lua_pop(L, 1); return status; } -static int l_ffi_init(struct kr_module *module) +/** Common part of calling modname.(de)init in lua. + * The function to call should be on top of the stack and it gets popped. */ +static int l_ffi_modcb(lua_State *L, struct kr_module *module) { - lua_State *L = l_ffi_preface(module, "init"); - if (!L) { - return 0; + if (lua_isnil(L, -1)) { + lua_pop(L, 1); /* .(de)init == nil, maybe even the module table doesn't exist */ + return kr_ok(); } - return l_ffi_call_mod(L, 1); + lua_getglobal(L, "modules_ffi_wrap_modcb"); + lua_insert(L, -2); /* swap with .(de)init */ + lua_pushpointer(L, module); + if (lua_pcall(L, 2, 0, 0) == 0) + return kr_ok(); + kr_log_error("error: %s\n", lua_tostring(L, -1)); + lua_pop(L, 1); + return kr_error(1); } static int l_ffi_deinit(struct kr_module *module) { - /* Deinit the module in Lua (if possible) */ - int ret = 0; - lua_State *L = module->lib; - if (l_ffi_preface(module, "deinit")) { - ret = l_ffi_call_mod(L, 1); - } - module->lib = NULL; + /* Call .deinit(), if it exists. */ + lua_State *L = the_worker->engine->L; + lua_getglobal(L, module->name); + lua_getfield(L, -1, "deinit"); + const int ret = l_ffi_modcb(L, module); + lua_pop(L, 1); /* the module's table */ + /* Free the layer API wrapper (unconst it) */ kr_layer_api_t* api = module->data; if (!api) { @@ -291,7 +288,6 @@ int ffimodule_register_lua(struct engine *engine, struct kr_module *module, cons /* Create FFI module with trampolined functions. */ memset(module, 0, sizeof(*module)); module->name = strdup(name); - module->init = &l_ffi_init; module->deinit = &l_ffi_deinit; /* Bake layer API if defined in module */ lua_getfield(L, -1, "layer"); @@ -300,10 +296,11 @@ int ffimodule_register_lua(struct engine *engine, struct kr_module *module, cons /* most likely not needed, but compatibility for now */ module->data = (void *)module->layer; } - module->lib = L; - lua_pop(L, 2); /* Clear the layer + module global */ - if (module->init) { - return module->init(module); - } - return kr_ok(); + lua_pop(L, 1); /* .layer table */ + + /* Now call .init(), if it exists. */ + lua_getfield(L, -1, "init"); + const int ret = l_ffi_modcb(L, module); + lua_pop(L, 1); /* the module's table */ + return ret; } diff --git a/daemon/lua/sandbox.lua.in b/daemon/lua/sandbox.lua.in index b753d1777..a932c4953 100644 --- a/daemon/lua/sandbox.lua.in +++ b/daemon/lua/sandbox.lua.in @@ -273,6 +273,10 @@ modules_ffi_layer_wrap_checkout = function (layer_cb, ctx_udata) local ctx = ffi.cast('kr_layer_t **', ctx_udata)[0] return layer_cb(ctx.state, ctx.req, ctx.pkt, ctx.dst, ctx.is_stream) end +modules_ffi_wrap_modcb = function (cb, kr_module_ud) -- this one isn't for layer + local kr_module = ffi.cast('struct kr_module **', kr_module_ud)[0] + return cb(kr_module) +end cache.clear = function (name, exact_name, rr_type, chunk_size, callback, prev_state) if name == nil or (name == '.' and not exact_name) then diff --git a/lib/module.h b/lib/module.h index 54261ff71..884985a95 100644 --- a/lib/module.h +++ b/lib/module.h @@ -49,19 +49,29 @@ typedef uint32_t (module_api_cb)(void); struct kr_module { char *name; - /** Constructor. Called after loading the module. @return error code. */ + /** Constructor. Called after loading the module. @return error code. + * Lua modules: not populated, called via lua directly. */ int (*init)(struct kr_module *self); + /** Destructor. Called before unloading the module. @return error code. */ int (*deinit)(struct kr_module *self); - /** Configure with encoded JSON (NULL if missing). @return error code. */ + + /** Configure with encoded JSON (NULL if missing). @return error code. + * Lua modules: not used and not useful from C. + * When called from lua, input is JSON, like for kr_prop_cb. */ int (*config)(struct kr_module *self, const char *input); - /** Packet processing API specs. May be NULL. See docs on that type. */ + + /** Packet processing API specs. May be NULL. See docs on that type. + * Owned by the module code. */ const kr_layer_api_t *layer; - /** List of properties. May be NULL. Terminated by { NULL, NULL, NULL }. */ + + /** List of properties. May be NULL. Terminated by { NULL, NULL, NULL }. + * Lua modules: not used and not useful. */ const struct kr_prop *props; - void *lib; /**< Shared library handle or RTLD_DEFAULT */ - void *data; /**< Custom data context. */ + /** dlopen() handle; RTLD_DEFAULT for embedded modules; NULL for lua modules. */ + void *lib; + void *data; /**< Custom data context. */ }; /** @@ -70,7 +80,8 @@ struct kr_module { * @param env pointer to the lua engine, i.e. struct engine *env (TODO: explicit type) * @param input parameter (NULL if missing/nil on lua level) * @return a free-form JSON output (malloc-ated) - * @note see l_trampoline() implementation for details about the input/output conversion. + * @note see modules_create_table_for_c() implementation for details + * about the input/output conversion. */ typedef char *(kr_prop_cb)(void *env, struct kr_module *self, const char *input); @@ -85,9 +96,9 @@ struct kr_prop { /** - * Load a C module instance into memory. + * Load a C module instance into memory. And call its init(). * - * @param module module structure + * @param module module structure. Will be overwritten except for ->data on success. * @param name module name * @param path module search path * @return 0 or an error @@ -99,6 +110,7 @@ int kr_module_load(struct kr_module *module, const char *name, const char *path) * Unload module instance. * * @param module module structure + * @note currently used even for lua modules */ KR_EXPORT void kr_module_unload(struct kr_module *module); -- 2.47.2