[Tarantool-patches] [PATCH v21 0/6] box: implement box.lib Lua module
Cyrill Gorcunov
gorcunov at gmail.com
Thu Apr 8 19:41:45 MSK 2021
v1-v3 are development ones and not sent.
v5 (by vlad):
- drop exists, list methods: they are redundant
- rename cfunc to cbox
- when create a function make it callable Lua object
- initialize cbox out of modules
- fix error in passing module name for reloading
- make api been cbox.func.[create|drop] and
cbox.module.reload
- fix test for OSX sake
v6 (by vlad):
- move module handling into module_cache file.
v7:
- development
v8:
- use rbtree for function instance storage, since
i don't like the idea of unexpected rehashing of
values in case of massive number of functions
allocated
- use reference counter and free function instance
if only load/unload are coupled
- keep a pointer to the function inside Lua object
so we don't need to lookup on every function call.
this force us to implement __gc method
- use new API and update docs
v9:
- development
v10:
- use hashes for function names lookup
- simply function loads counting
- use luaL_register_module and luaL_register_type for
easier methods registering
- carry functions as userdata object
v11:
- development
v12:
- switch to new API as been discussed in
https://lists.tarantool.org/tarantool-patches/e186c454-6765-4776-6433-f3f791ff4c27@tarantool.org/
v13:
- development
v14:
- switch to refs to carry module usage
- drop func_name structure renaming
- carry two hashes for backward compatibility with
functions created via box.schema.func help
- complete rework of cmod and most parts of
module_cache
- account for file statistics to invalidate
module cache
- new API for cmod, no more :reload, the :load
procedure uses cache invalidation
- update test cases
- still there is no GC test since I didn't
manage to deal with it
v15:
- report module state cached/orphan
- update test cases
- do not prevent functions lookup in orphan modules
- there was an idea to use box.shema.func cache as on
top of cmod's one, but this doesn't work because in case
if module doesnt exist in any caches we would put it into
into cmod's one as well but there wont be a module on
cmod level which would clean it up later (which makes
code a way more comple if we choose to track state
of modules).
v16:
- internal
v17:
- drop idea of unifying box.schema.func and cmod functions
cache, it brings more problems than solves due to too
different context of execution;
- make cmod self consistent, which shrink patch series
size ~1/5 in compare with previous attempts;
- improve tests to account internal states of modules
and functions (tt_dev key in reports).
v18:
- implement pass-through cache for modules loading, for
this sake 'struct module' uses cmod internally;
- improve tests to cover sole cmod case and a mixture
of box.schema.func and cmod to make sure the caches
are not corrupted.
v19:
- move module handling into separate subsystem;
- switch box.schema.func and cmod to use this shared
code to eliminate code duplication;
- update tests.
v20:
- fix potential nil dereference in schema_init while I'm in the code;
- during working on the series found a bug in module recovery,
fixed it in the series because my further work depends on it;
- main module which allows to create C functions now named as box.lib;
- structures renaming:
- box.schema.func uses struct schema_module to carry underlied
struct module;
- box.lib uses struct module directly and struct box_module_func
to carry cache of functions.
- name unification:
- schema_module variables are named as `mod`;
- box.schema.func api prefixed as `schema_module_x`;
- debug properties for module counting prefixed as `debug_x`;
- the test remains disabled because
- I run it locally all the time
- better to have it in repo and enable then since patching
test-run can take weeks, it is slow procedure
v21:
- rename functions and members from Vlad's comments
- use @base name for lowlevel module and functions
- use @module name for toplevel instances
- rename __schema_module_load to schema_do_module_load
- rename schema_func_c_load to func_c_load_from
- rename box_lib.[ch] to plain lib.[ch]
- cache free methods only clear the hash table itself
- in cache_put make sure there is an empty slot we're touching
- unify `const char *` usage
- use lua_isstring instead of LUA_TSTRING test
- optimize clearing of user data on Lua level via clear_udata
helper
- don't use tt_static hepers
- in schema_module_reload restore failing function immediately
and the proceed the remaining ones
- move ERRINJ_DYN_MODULE_COUNT to module_cache so that
old tests are passing
- more detailed testing of module loading
- make box.lib enabled by default even without box.cfg{}
I pushed it and since github actions are not that fast I ran
tests locally as well
Statistics:
* pass: 1250
* disabled: 70
issue https://github.com/tarantool/tarantool/issues/4642
branch gorcunov/gh-4642-func-ro-21
Cyrill Gorcunov (6):
box/func: fix modules functions restore
box/func: module_reload -- drop redundant argument
box/module_cache: introduce modules subsystem
box/schema.func: switch to new module api
box: implement box.lib module
test: add box.lib test
changelogs/unreleased/add-box-lib.md | 4 +
changelogs/unreleased/fix-module-reload.md | 8 +
src/box/CMakeLists.txt | 2 +
src/box/box.cc | 4 +-
src/box/call.c | 9 +-
src/box/func.c | 568 ++++++++-----------
src/box/func.h | 28 +-
src/box/func_def.h | 14 -
src/box/lua/init.c | 2 +
src/box/lua/lib.c | 606 +++++++++++++++++++++
src/box/lua/lib.h | 25 +
src/box/lua/load_cfg.lua | 1 +
src/box/module_cache.c | 491 +++++++++++++++++
src/box/module_cache.h | 205 +++++++
src/main.cc | 3 +
test/box/CMakeLists.txt | 8 +
test/box/cfunc1.c | 58 ++
test/box/cfunc2.c | 137 +++++
test/box/cfunc3.c | 25 +
test/box/cfunc4.c | 28 +
test/box/func_restore.result | 95 ++++
test/box/func_restore.test.lua | 50 ++
test/box/func_restore1.c | 33 ++
test/box/func_restore2.c | 27 +
test/box/func_restore3.c | 27 +
test/box/func_restore4.c | 27 +
test/box/lib.result | 530 ++++++++++++++++++
test/box/lib.test.lua | 203 +++++++
test/box/misc.result | 1 +
29 files changed, 2847 insertions(+), 372 deletions(-)
create mode 100644 changelogs/unreleased/add-box-lib.md
create mode 100644 changelogs/unreleased/fix-module-reload.md
create mode 100644 src/box/lua/lib.c
create mode 100644 src/box/lua/lib.h
create mode 100644 src/box/module_cache.c
create mode 100644 src/box/module_cache.h
create mode 100644 test/box/cfunc1.c
create mode 100644 test/box/cfunc2.c
create mode 100644 test/box/cfunc3.c
create mode 100644 test/box/cfunc4.c
create mode 100644 test/box/func_restore.result
create mode 100644 test/box/func_restore.test.lua
create mode 100644 test/box/func_restore1.c
create mode 100644 test/box/func_restore2.c
create mode 100644 test/box/func_restore3.c
create mode 100644 test/box/func_restore4.c
create mode 100644 test/box/lib.result
create mode 100644 test/box/lib.test.lua
base-commit: b88df4dc86c64029e26b3785761df209d0bfc84f
--
2.30.2
Cummulative diff from v20
---
diff --git a/changelogs/unreleased/add-box-lib.md b/changelogs/unreleased/add-box-lib.md
new file mode 100644
index 000000000..3fe7a1b01
--- /dev/null
+++ b/changelogs/unreleased/add-box-lib.md
@@ -0,0 +1,4 @@
+## feature/core
+
+* Introduce `box.lib` module which allows to load and execute
+ C stored procedures on read-only nodes (gh-4642).
diff --git a/changelogs/unreleased/fix-module-reload.md b/changelogs/unreleased/fix-module-reload.md
index 7e189617f..e44118aa8 100644
--- a/changelogs/unreleased/fix-module-reload.md
+++ b/changelogs/unreleased/fix-module-reload.md
@@ -1,4 +1,8 @@
## bugfix/core
-* Fix module reloading procedure which may crash in case if
- new module is corrupted (gh-4642).
+* Fix crash in case of reloading a compiled module when the
+ new module lacks some of functions which were present in the
+ former code. In turn this event triggers a fallback procedure
+ where we restore old functions but instead of restoring each
+ function we process a sole entry only leading to the crash
+ later when these restored functions are called (gh-5968).
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 95c65eb71..09f81afd8 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -196,7 +196,7 @@ add_library(box STATIC
lua/call.c
lua/cfg.cc
lua/console.c
- lua/box_lib.c
+ lua/lib.c
lua/serialize_lua.c
lua/tuple.c
lua/slab.c
diff --git a/src/box/func.c b/src/box/func.c
index 54d0cbc68..4119d2eec 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -32,7 +32,6 @@
#include "fiber.h"
#include "assoc.h"
#include "lua/call.h"
-#include "errinj.h"
#include "diag.h"
#include "port.h"
#include "schema.h"
@@ -55,7 +54,7 @@ struct func_name {
*/
struct schema_module {
/** Low level module instance. */
- struct module *module;
+ struct module *base;
/** List of imported functions. */
struct rlist funcs;
/** Reference counter. */
@@ -77,14 +76,14 @@ struct func_c {
/**
* A schema module the function belongs to.
*/
- struct schema_module *mod;
+ struct schema_module *module;
};
static void
-schema_module_ref(struct schema_module *mod);
+schema_module_ref(struct schema_module *module);
static void
-schema_module_unref(struct schema_module *mod);
+schema_module_unref(struct schema_module *module);
/***
* Split function name to symbol and package names.
@@ -127,10 +126,10 @@ schema_module_free(void)
{
while (mh_size(modules) > 0) {
mh_int_t i = mh_first(modules);
- struct schema_module *mod = mh_strnptr_node(modules, i)->val;
- schema_module_unref(mod);
+ mh_strnptr_del(modules, i, NULL);
}
mh_strnptr_delete(modules);
+ modules = NULL;
}
/**
@@ -149,16 +148,16 @@ cache_find(const char *name, const char *name_end)
* Save a module to the modules cache.
*/
static int
-cache_put(struct schema_module *mod)
+cache_put(struct schema_module *module)
{
- const char *str = mod->module->package;
- size_t len = mod->module->package_len;
+ const char *str = module->base->package;
+ size_t len = module->base->package_len;
const struct mh_strnptr_node_t strnode = {
.str = str,
.len = len,
.hash = mh_strn_hash(str, len),
- .val = mod,
+ .val = module,
};
if (mh_strnptr_put(modules, &strnode, NULL, NULL) == mh_end(modules)) {
@@ -172,27 +171,27 @@ cache_put(struct schema_module *mod)
* Update a module in the modules cache.
*/
static void
-cache_update(struct schema_module *mod)
+cache_update(struct schema_module *module)
{
- const char *str = mod->module->package;
- size_t len = mod->module->package_len;
+ const char *str = module->base->package;
+ size_t len = module->base->package_len;
mh_int_t i = mh_strnptr_find_inp(modules, str, len);
if (i == mh_end(modules))
panic("func: failed to update cache: %s", str);
mh_strnptr_node(modules, i)->str = str;
- mh_strnptr_node(modules, i)->val = mod;
+ mh_strnptr_node(modules, i)->val = module;
}
/**
* Delete a module from the module cache.
*/
static void
-cache_del(struct schema_module *mod)
+cache_del(struct schema_module *module)
{
- const char *str = mod->module->package;
- size_t len = mod->module->package_len;
+ const char *str = module->base->package;
+ size_t len = module->base->package_len;
mh_int_t i = mh_strnptr_find_inp(modules, str, len);
if (i != mh_end(modules)) {
@@ -202,72 +201,65 @@ cache_del(struct schema_module *mod)
* The module may be already reloaded so
* the cache carries a new entry instead.
*/
- if (v == mod)
+ if (v == module)
mh_strnptr_del(modules, i, NULL);
}
}
/** Delete a module. */
static void
-schema_module_delete(struct schema_module *mod)
+schema_module_delete(struct schema_module *module)
{
- struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
- if (e != NULL)
- --e->iparam;
- module_unload(mod->module);
- TRASH(mod);
- free(mod);
+ module_unload(module->base);
+ TRASH(module);
+ free(module);
}
/** Increment reference to a module. */
static void
-schema_module_ref(struct schema_module *mod)
+schema_module_ref(struct schema_module *module)
{
- assert(mod->refs >= 0);
- ++mod->refs;
+ assert(module->refs >= 0);
+ ++module->refs;
}
/** Decrement reference to a module and delete it if last one. */
static void
-schema_module_unref(struct schema_module *mod)
+schema_module_unref(struct schema_module *module)
{
- assert(mod->refs > 0);
- if (--mod->refs == 0) {
- cache_del(mod);
- schema_module_delete(mod);
+ assert(module->refs > 0);
+ if (--module->refs == 0) {
+ cache_del(module);
+ schema_module_delete(module);
}
}
static struct schema_module *
-__schema_module_load(const char *name, size_t len, bool force)
+schema_do_module_load(const char *name, size_t len, bool force)
{
- struct schema_module *mod = malloc(sizeof(*mod));
- if (mod != NULL) {
- mod->module = NULL;
- mod->refs = 0;
- rlist_create(&mod->funcs);
+ struct schema_module *module = malloc(sizeof(*module));
+ if (module != NULL) {
+ module->base = NULL;
+ module->refs = 0;
+ rlist_create(&module->funcs);
} else {
- diag_set(OutOfMemory, sizeof(*mod),
+ diag_set(OutOfMemory, sizeof(*module),
"malloc", "struct schema_module");
return NULL;
}
if (force)
- mod->module = module_load_force(name, len);
+ module->base = module_load_force(name, len);
else
- mod->module = module_load(name, len);
+ module->base = module_load(name, len);
- if (!mod->module) {
- free(mod);
+ if (module->base == NULL) {
+ free(module);
return NULL;
}
- struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
- if (e != NULL)
- ++e->iparam;
-
- schema_module_ref(mod);
- return mod;
+ schema_module_ref(module);
+ return module;
}
/**
@@ -276,7 +268,7 @@ __schema_module_load(const char *name, size_t len, bool force)
static struct schema_module *
schema_module_load(const char *name, const char *name_end)
{
- return __schema_module_load(name, name_end - name, false);
+ return schema_do_module_load(name, name_end - name, false);
}
/**
@@ -285,7 +277,7 @@ schema_module_load(const char *name, const char *name_end)
static struct schema_module *
schema_module_load_force(const char *name, const char *name_end)
{
- return __schema_module_load(name, name_end - name, true);
+ return schema_do_module_load(name, name_end - name, true);
}
static struct func_vtab func_c_vtab;
@@ -294,34 +286,33 @@ static struct func_vtab func_c_vtab;
static void
func_c_create(struct func_c *func_c)
{
- func_c->mod = NULL;
+ func_c->module = NULL;
func_c->base.vtab = &func_c_vtab;
rlist_create(&func_c->item);
module_func_create(&func_c->mf);
}
static int
-schema_func_c_load(struct schema_module *mod, const char *func_name,
- struct func_c *func)
+func_c_load_from(struct func_c *func, struct schema_module *module,
+ const char *func_name)
{
assert(module_func_is_empty(&func->mf));
- if (module_func_load(mod->module, func_name, &func->mf) != 0)
+ if (module_func_load(module->base, func_name, &func->mf) != 0)
return -1;
- func->mod = mod;
- rlist_move(&mod->funcs, &func->item);
- schema_module_ref(mod);
-
+ func->module = module;
+ rlist_move(&module->funcs, &func->item);
+ schema_module_ref(module);
return 0;
}
static void
-schema_func_c_unload(struct func_c *func)
+func_c_unload(struct func_c *func)
{
if (!module_func_is_empty(&func->mf)) {
rlist_del(&func->item);
- schema_module_unref(func->mod);
+ schema_module_unref(func->module);
module_func_unload(&func->mf);
func_c_create(func);
}
@@ -353,14 +344,16 @@ schema_module_reload(const char *package, const char *package_end)
rlist_foreach_entry_safe(func, &old->funcs, item, tmp_func) {
struct func_name name;
func_split_name(func->base.def->name, &name);
- schema_func_c_unload(func);
- if (schema_func_c_load(new, name.sym, func) != 0) {
+ func_c_unload(func);
+ if (func_c_load_from(func, new, name.sym) != 0) {
/*
- * WARN: A hack accessing new->funcs directly
- * to start restore from this failing function.
+ * We can restore failing function immediately
+ * and then all previously migrated ones.
*/
- rlist_move(&new->funcs, &func->item);
- goto restore;
+ if (func_c_load_from(func, old, name.sym) != 0)
+ goto restore_fail;
+ else
+ goto restore;
}
}
cache_update(new);
@@ -369,25 +362,28 @@ schema_module_reload(const char *package, const char *package_end)
return 0;
restore:
/*
- * Some old functions are not found int the new module,
- * thus restore all migrated functions back to original.
+ * Some old functions are not found in the new module,
+ * thus restore all migrated functions back to the original.
*/
rlist_foreach_entry_safe(func, &new->funcs, item, tmp_func) {
struct func_name name;
func_split_name(func->base.def->name, &name);
- schema_func_c_unload(func);
- if (schema_func_c_load(old, name.sym, func) != 0) {
- /*
- * Something strange was happen, an early loaden
- * function was not found in an old dso.
- */
- panic("Can't restore module function, "
- "server state is inconsistent");
- }
+ func_c_unload(func);
+ if (func_c_load_from(func, old, name.sym) != 0)
+ goto restore_fail;
}
schema_module_unref(old);
schema_module_unref(new);
return -1;
+
+restore_fail:
+ /*
+ * Something strange was happen, an early loaden
+ * function was not found in an old dso.
+ */
+ panic("Can't restore module function, "
+ "server state is inconsistent");
+ return -1;
}
static struct func *
@@ -454,7 +450,7 @@ func_c_destroy(struct func *base)
assert(base->vtab == &func_c_vtab);
assert(base != NULL && base->def->language == FUNC_LANGUAGE_C);
struct func_c *func = (struct func_c *) base;
- schema_func_c_unload(func);
+ func_c_unload(func);
TRASH(base);
free(func);
}
@@ -469,28 +465,28 @@ func_c_load(struct func_c *func)
struct func_name name;
func_split_name(func->base.def->name, &name);
- struct schema_module *cached, *mod;
+ struct schema_module *cached, *module;
cached = cache_find(name.package, name.package_end);
if (cached == NULL) {
- mod = schema_module_load(name.package, name.package_end);
- if (mod == NULL)
+ module = schema_module_load(name.package, name.package_end);
+ if (module == NULL)
return -1;
- if (cache_put(mod)) {
- schema_module_unref(mod);
+ if (cache_put(module)) {
+ schema_module_unref(module);
return -1;
}
} else {
- mod = cached;
- schema_module_ref(mod);
+ module = cached;
+ schema_module_ref(module);
}
- int rc = schema_func_c_load(mod, name.sym, func);
+ int rc = func_c_load_from(func, module, name.sym);
/*
* There is no explicit module loading in this
* interface so each function carries a reference
* by their own.
*/
- schema_module_unref(mod);
+ schema_module_unref(module);
return rc;
}
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 3fb433e7a..3a6d60864 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -60,7 +60,7 @@
#include "box/lua/cfg.h"
#include "box/lua/xlog.h"
#include "box/lua/console.h"
-#include "box/lua/box_lib.h"
+#include "box/lua/lib.h"
#include "box/lua/tuple.h"
#include "box/lua/execute.h"
#include "box/lua/key_def.h"
diff --git a/src/box/lua/box_lib.c b/src/box/lua/lib.c
similarity index 87%
rename from src/box/lua/box_lib.c
rename to src/box/lua/lib.c
index ce2ef8902..78aee37b5 100644
--- a/src/box/lua/box_lib.c
+++ b/src/box/lua/lib.c
@@ -11,10 +11,8 @@
#include "box/error.h"
#include "box/port.h"
-#include "tt_static.h"
-
#include "assoc.h"
-#include "box_lib.h"
+#include "lib.h"
#include "diag.h"
#include "module_cache.h"
@@ -25,7 +23,7 @@
*/
struct box_module_func {
/** C function to call. */
- struct module_func mf;
+ struct module_func base;
/** Number of references. */
int64_t refs;
/** Length of functon name in @a key. */
@@ -53,13 +51,20 @@ get_udata(struct lua_State *L, const char *uname)
return pptr != NULL ? *pptr : NULL;
}
-/** Set data to a new value. */
-static void
-set_udata(struct lua_State *L, const char *uname, void *ptr)
+/**
+ * Get pointer associated with an object and clear it
+ * returning previously associated data.
+ */
+static void *
+clear_udata(struct lua_State *L, const char *uname)
{
void **pptr = luaL_testudata(L, 1, uname);
- assert(pptr != NULL);
- *pptr = ptr;
+ if (pptr != NULL) {
+ void *ptr = *pptr;
+ *pptr = NULL;
+ return ptr;
+ }
+ return NULL;
}
/** Setup a new data and associate it with an object. */
@@ -129,7 +134,7 @@ cache_del(struct box_module_func *cf)
static int
lbox_module_load(struct lua_State *L)
{
- const char msg_noname[] = "Expects box.lib.load(\'name\') "
+ const char *msg_noname = "Expects box.lib.load(\'name\') "
"but no name passed";
if (lua_gettop(L) != 1 || !lua_isstring(L, 1)) {
@@ -173,14 +178,13 @@ lbox_module_unload(struct lua_State *L)
return luaT_error(L);
}
- struct module *m = get_udata(L, uname_lib);
+ struct module *m = clear_udata(L, uname_lib);
if (m == NULL) {
diag_set(IllegalParams, "The module is unloaded");
return luaT_error(L);
}
-
- set_udata(L, uname_lib, NULL);
module_unload(m);
+
lua_pushboolean(L, true);
return 1;
}
@@ -189,6 +193,9 @@ lbox_module_unload(struct lua_State *L)
static int
lbox_module_index(struct lua_State *L)
{
+ /*
+ * Process metamethods such as "module:load" first.
+ */
lua_getmetatable(L, 1);
lua_pushvalue(L, 2);
lua_rawget(L, -2);
@@ -202,7 +209,7 @@ lbox_module_index(struct lua_State *L)
}
const char *key = lua_tostring(L, 2);
- if (key == NULL || lua_type(L, 2) != LUA_TSTRING) {
+ if (key == NULL || !lua_isstring(L, 2)) {
diag_set(IllegalParams,
"Bad params, use __index(<key>)");
return luaT_error(L);
@@ -220,7 +227,8 @@ lbox_module_index(struct lua_State *L)
lua_pushnumber(L, m->refs);
return 1;
} else if (strcmp(key, "debug_ptr") == 0) {
- const char *s = tt_sprintf("%p", m);
+ char s[64];
+ snprintf(s, sizeof(s), "%p", m);
lua_pushstring(L, s);
return 1;
}
@@ -247,11 +255,9 @@ lbox_module_serialize(struct lua_State *L)
static int
lbox_module_gc(struct lua_State *L)
{
- struct module *m = get_udata(L, uname_lib);
- if (m != NULL) {
- set_udata(L, uname_lib, NULL);
+ struct module *m = clear_udata(L, uname_lib);
+ if (m != NULL)
module_unload(m);
- }
return 0;
}
@@ -267,7 +273,7 @@ box_module_func_ref(struct box_module_func *cf)
static void
box_module_func_delete(struct box_module_func *cf)
{
- assert(module_func_is_empty(&cf->mf));
+ assert(module_func_is_empty(&cf->base));
TRASH(cf);
free(cf);
}
@@ -278,7 +284,7 @@ box_module_func_unref(struct box_module_func *cf)
{
assert(cf->refs > 0);
if (--cf->refs == 0) {
- module_func_unload(&cf->mf);
+ module_func_unload(&cf->base);
cache_del(cf);
box_module_func_delete(cf);
}
@@ -315,17 +321,17 @@ box_module_func_new(struct module *m, const char *key, size_t len, size_t sym_le
cf->sym_len = sym_len;
cf->refs = 0;
- module_func_create(&cf->mf);
+ module_func_create(&cf->base);
memcpy(cf->key, key, len);
cf->key[len] = '\0';
- if (module_func_load(m, box_module_func_name(cf), &cf->mf) != 0) {
+ if (module_func_load(m, box_module_func_name(cf), &cf->base) != 0) {
box_module_func_delete(cf);
return NULL;
}
if (cache_put(cf) != 0) {
- module_func_unload(&cf->mf);
+ module_func_unload(&cf->base);
box_module_func_delete(cf);
return NULL;
}
@@ -360,7 +366,7 @@ static int
lbox_module_load_func(struct lua_State *L)
{
const char *method = "function = module:load";
- const char fmt_noname[] = "Expects %s(\'name\') but no name passed";
+ const char *fmt_noname = "Expects %s(\'name\') but no name passed";
if (lua_gettop(L) != 2 || !lua_isstring(L, 2)) {
diag_set(IllegalParams, fmt_noname, method);
@@ -377,10 +383,15 @@ lbox_module_load_func(struct lua_State *L)
size_t sym_len;
const char *sym = lua_tolstring(L, 2, &sym_len);
+ const size_t max_sym_len = 512;
if (sym_len < 1) {
diag_set(IllegalParams, fmt_noname, method);
return luaT_error(L);
+ } else if (sym_len > max_sym_len) {
+ diag_set(IllegalParams, "Symbol \'%s\' is too long (max %zd)",
+ sym, max_sym_len);
+ return luaT_error(L);
}
/*
@@ -388,9 +399,14 @@ lbox_module_load_func(struct lua_State *L)
* since the hash is global it should be unique
* per module. The symbol (function name) is the
* last part of the hash key.
+ *
+ * The key's buffer should be big enough to keep
+ * the longest package path plus symbol name and
+ * the pointer.
*/
- const char *key = tt_sprintf("%p.%s.%s", (void *)m,
- m->package, sym);
+ char key[PATH_MAX + 2 * max_sym_len];
+ snprintf(key, sizeof(key), "%p.%s.%s",
+ (void *)m, m->package, sym);
size_t len = strlen(key);
struct box_module_func *cf = cache_find(key, len);
@@ -426,13 +442,11 @@ lbox_func_unload(struct lua_State *L)
return luaT_error(L);
}
- struct box_module_func *cf = get_udata(L, uname_func);
+ struct box_module_func *cf = clear_udata(L, uname_func);
if (cf == NULL) {
diag_set(IllegalParams, "The function is unloaded");
return luaT_error(L);
}
-
- set_udata(L, uname_func, NULL);
box_module_func_unref(cf);
lua_pushboolean(L, true);
@@ -443,6 +457,9 @@ lbox_func_unload(struct lua_State *L)
static int
lbox_func_index(struct lua_State *L)
{
+ /*
+ * Process metamethods such as "func:unload" first.
+ */
lua_getmetatable(L, 1);
lua_pushvalue(L, 2);
lua_rawget(L, -2);
@@ -456,7 +473,7 @@ lbox_func_index(struct lua_State *L)
}
const char *key = lua_tostring(L, 2);
- if (key == NULL || lua_type(L, 2) != LUA_TSTRING) {
+ if (key == NULL || !lua_isstring(L, 2)) {
diag_set(IllegalParams,
"Bad params, use __index(<key>)");
return luaT_error(L);
@@ -477,11 +494,12 @@ lbox_func_index(struct lua_State *L)
lua_pushstring(L, cf->key);
return 1;
} else if (strcmp(key, "debug_module_ptr") == 0) {
- const char *s = tt_sprintf("%p", cf->mf.module);
+ char s[64];
+ snprintf(s, sizeof(s), "%p", cf->base.module);
lua_pushstring(L, s);
return 1;
} else if (strcmp(key, "debug_module_refs") == 0) {
- lua_pushnumber(L, cf->mf.module->refs);
+ lua_pushnumber(L, cf->base.module->refs);
return 1;
}
return 0;
@@ -507,11 +525,9 @@ lbox_func_serialize(struct lua_State *L)
static int
lbox_func_gc(struct lua_State *L)
{
- struct box_module_func *cf = get_udata(L, uname_func);
- if (cf != NULL) {
- set_udata(L, uname_func, NULL);
+ struct box_module_func *cf = clear_udata(L, uname_func);
+ if (cf != NULL)
box_module_func_unref(cf);
- }
return 0;
}
@@ -539,7 +555,7 @@ lbox_func_call(struct lua_State *L)
struct port ret;
- if (module_func_call(&cf->mf, &args, &ret) != 0) {
+ if (module_func_call(&cf->base, &args, &ret) != 0) {
port_destroy(&args);
return luaT_error(L);
}
diff --git a/src/box/lua/box_lib.h b/src/box/lua/lib.h
similarity index 100%
rename from src/box/lua/box_lib.h
rename to src/box/lua/lib.h
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 44bb95ed1..374524973 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -597,6 +597,7 @@ local box_cfg_guard_whitelist = {
error = true;
internal = true;
index = true;
+ lib = true;
session = true;
tuple = true;
runtime = true;
diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index 2cd2f2e8b..3dcb40566 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -10,9 +10,13 @@
#include <dlfcn.h>
#include <lua.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
#include "assoc.h"
#include "diag.h"
#include "fiber.h"
+#include "errinj.h"
#include "module_cache.h"
#include "box/error.h"
@@ -26,7 +30,7 @@ static struct mh_strnptr_t *module_cache = NULL;
/**
* Helpers for cache manipulations.
*/
-static void *
+static struct module *
cache_find(const char *str, size_t len)
{
mh_int_t e = mh_strnptr_find_inp(module_cache, str, len);
@@ -59,12 +63,21 @@ cache_put(struct module *m)
.val = m,
};
- mh_int_t e = mh_strnptr_put(module_cache, &nd, NULL, NULL);
+ struct mh_strnptr_node_t prev;
+ struct mh_strnptr_node_t *prev_ptr = &prev;
+
+ mh_int_t e = mh_strnptr_put(module_cache, &nd, &prev_ptr, NULL);
if (e == mh_end(module_cache)) {
diag_set(OutOfMemory, sizeof(nd), "malloc",
"module_cache node");
return -1;
}
+
+ /*
+ * Just to make sure we haven't replaced something, the
+ * entries must be explicitly deleted.
+ */
+ assert(prev_ptr == NULL);
return 0;
}
@@ -160,6 +173,9 @@ module_unref(struct module *m)
{
assert(m->refs > 0);
if (--m->refs == 0) {
+ struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
+ if (e != NULL)
+ --e->iparam;
cache_del(m);
dlclose(m->handle);
TRASH(m);
@@ -359,6 +375,10 @@ module_new(const char *package, size_t package_len,
goto error;
}
+ struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
+ if (e != NULL)
+ ++e->iparam;
+
module_ref(m);
return m;
@@ -450,13 +470,10 @@ module_unload(struct module *m)
void
module_free(void)
{
- mh_int_t e;
-
- mh_foreach(module_cache, e) {
- struct module *m = mh_strnptr_node(module_cache, e)->val;
- module_unload(m);
+ while (mh_size(module_cache) > 0) {
+ mh_int_t e = mh_first(module_cache);
+ mh_strnptr_del(module_cache, e, NULL);
}
-
mh_strnptr_delete(module_cache);
module_cache = NULL;
}
diff --git a/src/box/module_cache.h b/src/box/module_cache.h
index 18eb3866a..24fdf8d85 100644
--- a/src/box/module_cache.h
+++ b/src/box/module_cache.h
@@ -8,9 +8,6 @@
#include <stdint.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-
#include "trivia/config.h"
#if defined(__cplusplus)
diff --git a/test/box/CMakeLists.txt b/test/box/CMakeLists.txt
index c9dcbbb7b..51db15653 100644
--- a/test/box/CMakeLists.txt
+++ b/test/box/CMakeLists.txt
@@ -4,6 +4,8 @@ build_module(reload1 reload1.c)
build_module(reload2 reload2.c)
build_module(func_restore1 func_restore1.c)
build_module(func_restore2 func_restore2.c)
+build_module(func_restore3 func_restore3.c)
+build_module(func_restore4 func_restore4.c)
build_module(tuple_bench tuple_bench.c)
build_module(cfunc1 cfunc1.c)
build_module(cfunc2 cfunc2.c)
diff --git a/test/box/func_restore.result b/test/box/func_restore.result
index e679f4eb8..7cd9e67c4 100644
--- a/test/box/func_restore.result
+++ b/test/box/func_restore.result
@@ -1,7 +1,7 @@
-- test-run result file version 2
--
--- Test that function can be restored to the
--- former values when new module can't be
+-- Test that compiled C function can be restored
+-- to the former values when new module can't be
-- loaded for some reason (say there some
-- missing functions).
--
@@ -26,9 +26,6 @@ path_func_restore = fio.pathjoin(build_path, "test/box/func_restore.") .. ext
path_func_good = fio.pathjoin(build_path, "test/box/func_restore1.") .. ext
| ---
| ...
-path_func_bad = fio.pathjoin(build_path, "test/box/func_restore2.") .. ext
- | ---
- | ...
_ = pcall(fio.unlink(path_func_restore))
| ---
@@ -58,8 +55,6 @@ box.schema.user.grant('guest', 'execute', 'function', 'func_restore.echo_1')
| ---
| ...
--- Order *does* matter since we bind functions in
--- a list where entries are added to the top.
box.func['func_restore.echo_3']:call()
| ---
| - 3
@@ -73,31 +68,28 @@ box.func['func_restore.echo_1']:call()
| - 1
| ...
-_ = pcall(fio.unlink(path_func_restore))
- | ---
- | ...
-fio.symlink(path_func_bad, path_func_restore)
+function run_restore(path) \
+ _ = pcall(fio.unlink(path_func_restore)) \
+ fio.symlink(path, path_func_restore) \
+ \
+ local ok, _ = pcall(box.schema.func.reload, "func_restore") \
+ assert(not ok) \
+ \
+ box.func['func_restore.echo_1']:call() \
+ box.func['func_restore.echo_2']:call() \
+ box.func['func_restore.echo_3']:call() \
+end
| ---
- | - true
| ...
-ok, _ = pcall(box.schema.func.reload, "func_restore")
+bad_modules = { \
+ fio.pathjoin(build_path, "test/box/func_restore2.") .. ext, \
+ fio.pathjoin(build_path, "test/box/func_restore3.") .. ext, \
+ fio.pathjoin(build_path, "test/box/func_restore4.") .. ext, \
+}
| ---
| ...
-assert(not ok)
- | ---
- | - true
- | ...
-box.func['func_restore.echo_1']:call()
+for k, v in ipairs(bad_modules) do run_restore(v) end
| ---
- | - 1
- | ...
-box.func['func_restore.echo_2']:call()
- | ---
- | - 2
- | ...
-box.func['func_restore.echo_3']:call()
- | ---
- | - 3
| ...
diff --git a/test/box/func_restore.test.lua b/test/box/func_restore.test.lua
index 8a38a6074..4bd05769d 100644
--- a/test/box/func_restore.test.lua
+++ b/test/box/func_restore.test.lua
@@ -1,6 +1,6 @@
--
--- Test that function can be restored to the
--- former values when new module can't be
+-- Test that compiled C function can be restored
+-- to the former values when new module can't be
-- loaded for some reason (say there some
-- missing functions).
--
@@ -13,7 +13,6 @@ ext = (jit.os == "OSX" and "dylib" or "so")
path_func_restore = fio.pathjoin(build_path, "test/box/func_restore.") .. ext
path_func_good = fio.pathjoin(build_path, "test/box/func_restore1.") .. ext
-path_func_bad = fio.pathjoin(build_path, "test/box/func_restore2.") .. ext
_ = pcall(fio.unlink(path_func_restore))
fio.symlink(path_func_good, path_func_restore)
@@ -26,18 +25,26 @@ box.schema.user.grant('guest', 'execute', 'function', 'func_restore.echo_3')
box.schema.user.grant('guest', 'execute', 'function', 'func_restore.echo_2')
box.schema.user.grant('guest', 'execute', 'function', 'func_restore.echo_1')
--- Order *does* matter since we bind functions in
--- a list where entries are added to the top.
box.func['func_restore.echo_3']:call()
box.func['func_restore.echo_2']:call()
box.func['func_restore.echo_1']:call()
-_ = pcall(fio.unlink(path_func_restore))
-fio.symlink(path_func_bad, path_func_restore)
-
-ok, _ = pcall(box.schema.func.reload, "func_restore")
-assert(not ok)
-
-box.func['func_restore.echo_1']:call()
-box.func['func_restore.echo_2']:call()
-box.func['func_restore.echo_3']:call()
+function run_restore(path) \
+ _ = pcall(fio.unlink(path_func_restore)) \
+ fio.symlink(path, path_func_restore) \
+ \
+ local ok, _ = pcall(box.schema.func.reload, "func_restore") \
+ assert(not ok) \
+ \
+ box.func['func_restore.echo_1']:call() \
+ box.func['func_restore.echo_2']:call() \
+ box.func['func_restore.echo_3']:call() \
+end
+
+bad_modules = { \
+ fio.pathjoin(build_path, "test/box/func_restore2.") .. ext, \
+ fio.pathjoin(build_path, "test/box/func_restore3.") .. ext, \
+ fio.pathjoin(build_path, "test/box/func_restore4.") .. ext, \
+}
+
+for k, v in ipairs(bad_modules) do run_restore(v) end
diff --git a/test/box/func_restore1.c b/test/box/func_restore1.c
index ffd69fd2b..891ec0136 100644
--- a/test/box/func_restore1.c
+++ b/test/box/func_restore1.c
@@ -14,7 +14,6 @@ echo_num(box_function_ctx_t *ctx, const char *args,
return 0;
}
-
int
echo_1(box_function_ctx_t *ctx, const char *args, const char *args_end)
{
diff --git a/test/box/func_restore2.c b/test/box/func_restore2.c
index 0d77e78b2..d9d6de8df 100644
--- a/test/box/func_restore2.c
+++ b/test/box/func_restore2.c
@@ -14,7 +14,6 @@ echo_num(box_function_ctx_t *ctx, const char *args,
return 0;
}
-
int
echo_1(box_function_ctx_t *ctx, const char *args, const char *args_end)
{
diff --git a/test/box/func_restore3.c b/test/box/func_restore3.c
new file mode 100644
index 000000000..e38b44400
--- /dev/null
+++ b/test/box/func_restore3.c
@@ -0,0 +1,27 @@
+#include <stdio.h>
+#include <stdbool.h>
+#include <msgpuck.h>
+
+#include "module.h"
+
+static int
+echo_num(box_function_ctx_t *ctx, const char *args,
+ const char *args_end, unsigned int num)
+{
+ char res[16];
+ char *end = mp_encode_uint(res, num);
+ box_return_mp(ctx, res, end);
+ return 0;
+}
+
+int
+echo_2(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+ return echo_num(ctx, args, args_end, 2);
+}
+
+int
+echo_3(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+ return echo_num(ctx, args, args_end, 3);
+}
diff --git a/test/box/func_restore4.c b/test/box/func_restore4.c
new file mode 100644
index 000000000..4e97ff0a0
--- /dev/null
+++ b/test/box/func_restore4.c
@@ -0,0 +1,27 @@
+#include <stdio.h>
+#include <stdbool.h>
+#include <msgpuck.h>
+
+#include "module.h"
+
+static int
+echo_num(box_function_ctx_t *ctx, const char *args,
+ const char *args_end, unsigned int num)
+{
+ char res[16];
+ char *end = mp_encode_uint(res, num);
+ box_return_mp(ctx, res, end);
+ return 0;
+}
+
+int
+echo_1(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+ return echo_num(ctx, args, args_end, 1);
+}
+
+int
+echo_3(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+ return echo_num(ctx, args, args_end, 3);
+}
More information about the Tarantool-patches
mailing list