Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module
@ 2021-02-02 22:11 Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 01/12] box/func: factor out c function entry structure Cyrill Gorcunov via Tarantool-patches
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:11 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

The series implements a bare minimum. Vlad, please take a look
once time permit.

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

branch gorcunov/gh-4642-func-ro-14
issue https://github.com/tarantool/tarantool/issues/4642

Cyrill Gorcunov (12):
  box/func: factor out c function entry structure
  module_cache: move module handling into own subsystem
  module_cache: direct update a cache value on reload
  module_cache: rename calls to ref in module structure
  module_cache: add comment about weird resolving
  module_cache: module_reload - drop redundant parameter
  module_cache: use references as a main usage counter
  module_cache: make module to carry hash it belongs to
  module_cache: use own hash for box.schema.func requests
  module_cache: provide module_load/unload calls
  box/cmod: implement cmod Lua module
  test: box/cfunc -- add cmod test

 src/box/CMakeLists.txt  |   2 +
 src/box/call.c          |  10 +-
 src/box/func.c          | 491 +-------------------------
 src/box/func.h          |  41 +--
 src/box/func_def.h      |  14 -
 src/box/lua/cmod.c      | 600 +++++++++++++++++++++++++++++++
 src/box/lua/cmod.h      |  24 ++
 src/box/lua/init.c      |   2 +
 src/box/module_cache.c  | 758 ++++++++++++++++++++++++++++++++++++++++
 src/box/module_cache.h  | 169 +++++++++
 test/box/CMakeLists.txt |   2 +
 test/box/cfunc1.c       |  58 +++
 test/box/cfunc2.c       | 137 ++++++++
 test/box/cmod.result    | 312 +++++++++++++++++
 test/box/cmod.test.lua  | 123 +++++++
 test/box/suite.ini      |   2 +-
 16 files changed, 2200 insertions(+), 545 deletions(-)
 create mode 100644 src/box/lua/cmod.c
 create mode 100644 src/box/lua/cmod.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/cmod.result
 create mode 100644 test/box/cmod.test.lua


base-commit: 7722a81c6a5a3c57c27a688c957fb187d81850f0
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 01/12] box/func: factor out c function entry structure
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:11 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 02/12] module_cache: move module handling into own subsystem Cyrill Gorcunov via Tarantool-patches
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:11 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

Currently func_c structure is a wrapper over struct func
which in turn handles function definition, credentials and etc,
mostly to handle internal "_func" space.

Such tight bound doesn't allow to reuse func_c in any
other context. But we're going to support C function
execution in read-only instances and for this we better
reuse already existing structures as much as possible
instead of breeding new ones.

Thus lets extract module symbols into module_sym structure,
this allows us to reuse module path cache in other code.

While extracting the structure rename "func" member to
"addr" since this exactly what the member represents:
an address of symbol in memory.

Same time to be completely independent of "_func" specific lets
carry symbolic name inplace (ie member "name" is kind of redundant
when module_sym is used for "_func" handling where we can retrieve
the name via function definition but such definition won't be
available for non-persistent C functions which we will support
in next patches).

The new structure allows us to load/unload general symbols via
module_sym_load() and module_sym_unload() helpers.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/func.c | 190 ++++++++++++++++++++++++-------------------------
 src/box/func.h |  42 +++++++++++
 2 files changed, 134 insertions(+), 98 deletions(-)

diff --git a/src/box/func.c b/src/box/func.c
index 9909cee45..8030aa07c 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -59,19 +59,8 @@ struct func_name {
 struct func_c {
 	/** Function object base class. */
 	struct func base;
-	/**
-	 * Anchor for module membership.
-	 */
-	struct rlist item;
-	/**
-	 * For C functions, the body of the function.
-	 */
-	box_function_f func;
-	/**
-	 * Each stored function keeps a handle to the
-	 * dynamic library for the C callback.
-	 */
-	struct module *module;
+	/** C function module symbol. */
+	struct module_sym mod_sym;
 };
 
 /***
@@ -371,6 +360,73 @@ module_sym(struct module *module, const char *name)
 	return f;
 }
 
+int
+module_sym_load(struct module_sym *mod_sym)
+{
+	assert(mod_sym->addr == NULL);
+
+	struct func_name name;
+	func_split_name(mod_sym->name, &name);
+
+	/*
+	 * In case if module has been loaded already by
+	 * some previous call we can eliminate redundant
+	 * loading and take it from the cache.
+	 */
+	struct module *cached, *module;
+	cached = module_cache_find(name.package, name.package_end);
+	if (cached == NULL) {
+		module = module_load(name.package, name.package_end);
+		if (module == NULL)
+			return -1;
+		if (module_cache_put(module) != 0) {
+			module_delete(module);
+			return -1;
+		}
+	} else {
+		module = cached;
+	}
+
+	mod_sym->addr = module_sym(module, name.sym);
+	if (mod_sym->addr == NULL) {
+		if (cached == NULL) {
+			/*
+			 * In case if it was a first load we should
+			 * clean the cache immediately otherwise
+			 * the module continue being referenced even
+			 * if there will be no use of it.
+			 *
+			 * Note the module_sym set an error thus be
+			 * careful to not wipe it.
+			 */
+			module_cache_del(name.package, name.package_end);
+			module_delete(module);
+		}
+		return -1;
+	}
+	mod_sym->module = module;
+	rlist_add(&module->funcs, &mod_sym->item);
+	return 0;
+}
+
+void
+module_sym_unload(struct module_sym *mod_sym)
+{
+	if (mod_sym->module == NULL)
+		return;
+
+	rlist_del(&mod_sym->item);
+	if (rlist_empty(&mod_sym->module->funcs)) {
+		struct func_name name;
+		func_split_name(mod_sym->name, &name);
+		module_cache_del(name.package, name.package_end);
+	}
+	module_gc(mod_sym->module);
+
+	mod_sym->module = NULL;
+	mod_sym->addr = NULL;
+}
+
 int
 module_reload(const char *package, const char *package_end, struct module **module)
 {
@@ -385,15 +441,15 @@ module_reload(const char *package, const char *package_end, struct module **modu
 	if (new_module == NULL)
 		return -1;
 
-	struct func_c *func, *tmp_func;
-	rlist_foreach_entry_safe(func, &old_module->funcs, item, tmp_func) {
+	struct module_sym *mod_sym, *tmp;
+	rlist_foreach_entry_safe(mod_sym, &old_module->funcs, item, tmp) {
 		struct func_name name;
-		func_split_name(func->base.def->name, &name);
-		func->func = module_sym(new_module, name.sym);
-		if (func->func == NULL)
+		func_split_name(mod_sym->name, &name);
+		mod_sym->addr = module_sym(new_module, name.sym);
+		if (mod_sym->addr == NULL)
 			goto restore;
-		func->module = new_module;
-		rlist_move(&new_module->funcs, &func->item);
+		mod_sym->module = new_module;
+		rlist_move(&new_module->funcs, &mod_sym->item);
 	}
 	module_cache_del(package, package_end);
 	if (module_cache_put(new_module) != 0)
@@ -408,9 +464,9 @@ module_reload(const char *package, const char *package_end, struct module **modu
 	 */
 	do {
 		struct func_name name;
-		func_split_name(func->base.def->name, &name);
-		func->func = module_sym(old_module, name.sym);
-		if (func->func == NULL) {
+		func_split_name(mod_sym->name, &name);
+		mod_sym->addr = module_sym(old_module, name.sym);
+		if (mod_sym->addr == NULL) {
 			/*
 			 * Something strange was happen, an early loaden
 			 * function was not found in an old dso.
@@ -418,10 +474,11 @@ module_reload(const char *package, const char *package_end, struct module **modu
 			panic("Can't restore module function, "
 			      "server state is inconsistent");
 		}
-		func->module = old_module;
-		rlist_move(&old_module->funcs, &func->item);
-	} while (func != rlist_first_entry(&old_module->funcs,
-					   struct func_c, item));
+		mod_sym->module = old_module;
+		rlist_move(&old_module->funcs, &mod_sym->item);
+	} while (mod_sym != rlist_first_entry(&old_module->funcs,
+					      struct module_sym,
+					      item));
 	assert(rlist_empty(&new_module->funcs));
 	module_delete(new_module);
 	return -1;
@@ -484,94 +541,31 @@ func_c_new(MAYBE_UNUSED struct func_def *def)
 		return NULL;
 	}
 	func->base.vtab = &func_c_vtab;
-	func->func = NULL;
-	func->module = NULL;
+	func->mod_sym.addr = NULL;
+	func->mod_sym.module = NULL;
+	func->mod_sym.name = def->name;
 	return &func->base;
 }
 
-static void
-func_c_unload(struct func_c *func)
-{
-	if (func->module) {
-		rlist_del(&func->item);
-		if (rlist_empty(&func->module->funcs)) {
-			struct func_name name;
-			func_split_name(func->base.def->name, &name);
-			module_cache_del(name.package, name.package_end);
-		}
-		module_gc(func->module);
-	}
-	func->module = NULL;
-	func->func = NULL;
-}
-
 static void
 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;
-	func_c_unload(func);
+	module_sym_unload(&func->mod_sym);
 	TRASH(base);
 	free(func);
 }
 
-/**
- * Resolve func->func (find the respective DLL and fetch the
- * symbol from it).
- */
-static int
-func_c_load(struct func_c *func)
-{
-	assert(func->func == NULL);
-
-	struct func_name name;
-	func_split_name(func->base.def->name, &name);
-
-	struct module *cached, *module;
-	cached = module_cache_find(name.package, name.package_end);
-	if (cached == NULL) {
-		module = module_load(name.package, name.package_end);
-		if (module == NULL)
-			return -1;
-		if (module_cache_put(module)) {
-			module_delete(module);
-			return -1;
-		}
-	} else {
-		module = cached;
-	}
-
-	func->func = module_sym(module, name.sym);
-	if (func->func == NULL) {
-		if (cached == NULL) {
-			/*
-			 * In case if it was a first load we should
-			 * clean the cache immediately otherwise
-			 * the module continue being referenced even
-			 * if there will be no use of it.
-			 *
-			 * Note the module_sym set an error thus be
-			 * careful to not wipe it.
-			 */
-			module_cache_del(name.package, name.package_end);
-			module_delete(module);
-		}
-		return -1;
-	}
-	func->module = module;
-	rlist_add(&module->funcs, &func->item);
-	return 0;
-}
-
 int
 func_c_call(struct func *base, struct port *args, struct port *ret)
 {
 	assert(base->vtab == &func_c_vtab);
 	assert(base != NULL && base->def->language == FUNC_LANGUAGE_C);
 	struct func_c *func = (struct func_c *) base;
-	if (func->func == NULL) {
-		if (func_c_load(func) != 0)
+	if (func->mod_sym.addr == NULL) {
+		if (module_sym_load(&func->mod_sym) != 0)
 			return -1;
 	}
 
@@ -586,10 +580,10 @@ func_c_call(struct func *base, struct port *args, struct port *ret)
 	box_function_ctx_t ctx = { ret };
 
 	/* Module can be changed after function reload. */
-	struct module *module = func->module;
+	struct module *module = func->mod_sym.module;
 	assert(module != NULL);
 	++module->calls;
-	int rc = func->func(&ctx, data, data + data_sz);
+	int rc = func->mod_sym.addr(&ctx, data, data + data_sz);
 	--module->calls;
 	module_gc(module);
 	region_truncate(region, region_svp);
diff --git a/src/box/func.h b/src/box/func.h
index 581e468cb..9a7f17446 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -58,6 +58,28 @@ struct module {
 	char package[0];
 };
 
+/**
+ * Callable symbol bound to a module.
+ */
+struct module_sym {
+	/**
+	 * Anchor for module membership.
+	 */
+	struct rlist item;
+	/**
+	 * For C functions, address of the function.
+	 */
+	box_function_f addr;
+	/**
+	 * A module the symbol belongs to.
+	 */
+	struct module *module;
+	/**
+	 * Function name definition.
+	 */
+	char *name;
+};
+
 /** Virtual method table for func object. */
 struct func_vtab {
 	/** Call function with given arguments. */
@@ -108,6 +130,26 @@ func_delete(struct func *func);
 int
 func_call(struct func *func, struct port *args, struct port *ret);
 
+/**
+ * Resolve C entry (find the respective DLL and fetch the
+ * symbol from it).
+ *
+ * @param mod_sym module symbol pointer.
+ * @retval -1 on error.
+ * @retval 0 on success.
+ */
+int
+module_sym_load(struct module_sym *mod_sym);
+
+/**
+ * Unload module symbol and drop it from the package
+ * cache if there is no users left.
+ *
+ * @param mod_sym module symbol pointer.
+ */
+void
+module_sym_unload(struct module_sym *mod_sym);
+
 /**
  * Reload dynamically loadable module.
  *
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 02/12] module_cache: move module handling into own subsystem
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 01/12] box/func: factor out c function entry structure Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:11 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 03/12] module_cache: direct update a cache value on reload Cyrill Gorcunov via Tarantool-patches
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:11 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

The module handling should not be bound to particular
function implementation (we will have two users: already
existing functions for "_func" space, and a new upcoming
one which will be serving functions on read only instances).

For this sake all module related code is moved to
module_cache file where we do symbol resolving, calling
and tracking of symbol usage.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/CMakeLists.txt |   1 +
 src/box/func.c         | 473 +-----------------------------------
 src/box/func.h         |  83 +------
 src/box/func_def.h     |  14 --
 src/box/module_cache.c | 535 +++++++++++++++++++++++++++++++++++++++++
 src/box/module_cache.h | 139 +++++++++++
 6 files changed, 679 insertions(+), 566 deletions(-)
 create mode 100644 src/box/module_cache.c
 create mode 100644 src/box/module_cache.h

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 19203f770..339e2c8a9 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -187,6 +187,7 @@ add_library(box STATIC
     sql_stmt_cache.c
     wal.c
     call.c
+    module_cache.c
     merger.c
     ibuf.c
     ${sql_sources}
diff --git a/src/box/func.c b/src/box/func.c
index 8030aa07c..4e7025fdb 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -31,30 +31,12 @@
 #include "func.h"
 #include "fiber.h"
 #include "trivia/config.h"
-#include "assoc.h"
 #include "lua/utils.h"
 #include "lua/call.h"
-#include "error.h"
-#include "errinj.h"
 #include "diag.h"
 #include "port.h"
 #include "schema.h"
 #include "session.h"
-#include "libeio/eio.h"
-#include <fcntl.h>
-#include <dlfcn.h>
-
-/**
- * Parsed symbol and package names.
- */
-struct func_name {
-	/** Null-terminated symbol name, e.g. "func" for "mod.submod.func" */
-	const char *sym;
-	/** Package name, e.g. "mod.submod" for "mod.submod.func" */
-	const char *package;
-	/** A pointer to the last character in ->package + 1 */
-	const char *package_end;
-};
 
 struct func_c {
 	/** Function object base class. */
@@ -63,427 +45,6 @@ struct func_c {
 	struct module_sym mod_sym;
 };
 
-/***
- * Split function name to symbol and package names.
- * For example, str = foo.bar.baz => sym = baz, package = foo.bar
- * @param str function name, e.g. "module.submodule.function".
- * @param[out] name parsed symbol and package names.
- */
-static void
-func_split_name(const char *str, struct func_name *name)
-{
-	name->package = str;
-	name->package_end = strrchr(str, '.');
-	if (name->package_end != NULL) {
-		/* module.submodule.function => module.submodule, function */
-		name->sym = name->package_end + 1; /* skip '.' */
-	} else {
-		/* package == function => function, function */
-		name->sym = name->package;
-		name->package_end = str + strlen(str);
-	}
-}
-
-/**
- * Arguments for luaT_module_find used by lua_cpcall()
- */
-struct module_find_ctx {
-	const char *package;
-	const char *package_end;
-	char *path;
-	size_t path_len;
-};
-
-/**
- * A cpcall() helper for module_find()
- */
-static int
-luaT_module_find(lua_State *L)
-{
-	struct module_find_ctx *ctx = (struct module_find_ctx *)
-		lua_topointer(L, 1);
-
-	/*
-	 * Call package.searchpath(name, package.cpath) and use
-	 * the path to the function in dlopen().
-	 */
-	lua_getglobal(L, "package");
-
-	lua_getfield(L, -1, "search");
-
-	/* Argument of search: name */
-	lua_pushlstring(L, ctx->package, ctx->package_end - ctx->package);
-
-	lua_call(L, 1, 1);
-	if (lua_isnil(L, -1))
-		return luaL_error(L, "module not found");
-	/* Convert path to absolute */
-	char resolved[PATH_MAX];
-	if (realpath(lua_tostring(L, -1), resolved) == NULL) {
-		diag_set(SystemError, "realpath");
-		return luaT_error(L);
-	}
-
-	snprintf(ctx->path, ctx->path_len, "%s", resolved);
-	return 0;
-}
-
-/**
- * Find path to module using Lua's package.cpath
- * @param package package name
- * @param package_end a pointer to the last byte in @a package + 1
- * @param[out] path path to shared library
- * @param path_len size of @a path buffer
- * @retval 0 on success
- * @retval -1 on error, diag is set
- */
-static int
-module_find(const char *package, const char *package_end, char *path,
-	    size_t path_len)
-{
-	struct module_find_ctx ctx = { package, package_end, path, path_len };
-	lua_State *L = tarantool_L;
-	int top = lua_gettop(L);
-	if (luaT_cpcall(L, luaT_module_find, &ctx) != 0) {
-		int package_len = (int) (package_end - package);
-		diag_set(ClientError, ER_LOAD_MODULE, package_len, package,
-			 lua_tostring(L, -1));
-		lua_settop(L, top);
-		return -1;
-	}
-	assert(top == lua_gettop(L)); /* cpcall discard results */
-	return 0;
-}
-
-static struct mh_strnptr_t *modules = NULL;
-
-static void
-module_gc(struct module *module);
-
-int
-module_init(void)
-{
-	modules = mh_strnptr_new();
-	if (modules == NULL) {
-		diag_set(OutOfMemory, sizeof(*modules), "malloc",
-			  "modules hash table");
-		return -1;
-	}
-	return 0;
-}
-
-void
-module_free(void)
-{
-	while (mh_size(modules) > 0) {
-		mh_int_t i = mh_first(modules);
-		struct module *module =
-			(struct module *) mh_strnptr_node(modules, i)->val;
-		/* Can't delete modules if they have active calls */
-		module_gc(module);
-	}
-	mh_strnptr_delete(modules);
-}
-
-/**
- * Look up a module in the modules cache.
- */
-static struct module *
-module_cache_find(const char *name, const char *name_end)
-{
-	mh_int_t i = mh_strnptr_find_inp(modules, name, name_end - name);
-	if (i == mh_end(modules))
-		return NULL;
-	return (struct module *)mh_strnptr_node(modules, i)->val;
-}
-
-/**
- * Save module to the module cache.
- */
-static inline int
-module_cache_put(struct module *module)
-{
-	size_t package_len = strlen(module->package);
-	uint32_t name_hash = mh_strn_hash(module->package, package_len);
-	const struct mh_strnptr_node_t strnode = {
-		module->package, package_len, name_hash, module};
-
-	if (mh_strnptr_put(modules, &strnode, NULL, NULL) == mh_end(modules)) {
-		diag_set(OutOfMemory, sizeof(strnode), "malloc", "modules");
-		return -1;
-	}
-	return 0;
-}
-
-/**
- * Delete a module from the module cache
- */
-static void
-module_cache_del(const char *name, const char *name_end)
-{
-	mh_int_t i = mh_strnptr_find_inp(modules, name, name_end - name);
-	if (i == mh_end(modules))
-		return;
-	mh_strnptr_del(modules, i, NULL);
-}
-
-/*
- * Load a dso.
- * Create a new symlink based on temporary directory and try to
- * load via this symink to load a dso twice for cases of a function
- * reload.
- */
-static struct module *
-module_load(const char *package, const char *package_end)
-{
-	char path[PATH_MAX];
-	if (module_find(package, package_end, path, sizeof(path)) != 0)
-		return NULL;
-
-	int package_len = package_end - package;
-	struct module *module = (struct module *)
-		malloc(sizeof(*module) + package_len + 1);
-	if (module == NULL) {
-		diag_set(OutOfMemory, sizeof(struct module) + package_len + 1,
-			 "malloc", "struct module");
-		return NULL;
-	}
-	memcpy(module->package, package, package_len);
-	module->package[package_len] = 0;
-	rlist_create(&module->funcs);
-	module->calls = 0;
-
-	const char *tmpdir = getenv("TMPDIR");
-	if (tmpdir == NULL)
-		tmpdir = "/tmp";
-	char dir_name[PATH_MAX];
-	int rc = snprintf(dir_name, sizeof(dir_name), "%s/tntXXXXXX", tmpdir);
-	if (rc < 0 || (size_t) rc >= sizeof(dir_name)) {
-		diag_set(SystemError, "failed to generate path to tmp dir");
-		goto error;
-	}
-
-	if (mkdtemp(dir_name) == NULL) {
-		diag_set(SystemError, "failed to create unique dir name: %s",
-			 dir_name);
-		goto error;
-	}
-	char load_name[PATH_MAX];
-	rc = snprintf(load_name, sizeof(load_name), "%s/%.*s." TARANTOOL_LIBEXT,
-		      dir_name, package_len, package);
-	if (rc < 0 || (size_t) rc >= sizeof(dir_name)) {
-		diag_set(SystemError, "failed to generate path to DSO");
-		goto error;
-	}
-
-	struct stat st;
-	if (stat(path, &st) < 0) {
-		diag_set(SystemError, "failed to stat() module %s", path);
-		goto error;
-	}
-
-	int source_fd = open(path, O_RDONLY);
-	if (source_fd < 0) {
-		diag_set(SystemError, "failed to open module %s file for" \
-			 " reading", path);
-		goto error;
-	}
-	int dest_fd = open(load_name, O_WRONLY|O_CREAT|O_TRUNC,
-			   st.st_mode & 0777);
-	if (dest_fd < 0) {
-		diag_set(SystemError, "failed to open file %s for writing ",
-			 load_name);
-		close(source_fd);
-		goto error;
-	}
-
-	off_t ret = eio_sendfile_sync(dest_fd, source_fd, 0, st.st_size);
-	close(source_fd);
-	close(dest_fd);
-	if (ret != st.st_size) {
-		diag_set(SystemError, "failed to copy DSO %s to %s",
-			 path, load_name);
-		goto error;
-	}
-
-	module->handle = dlopen(load_name, RTLD_NOW | RTLD_LOCAL);
-	if (unlink(load_name) != 0)
-		say_warn("failed to unlink dso link %s", load_name);
-	if (rmdir(dir_name) != 0)
-		say_warn("failed to delete temporary dir %s", dir_name);
-	if (module->handle == NULL) {
-		diag_set(ClientError, ER_LOAD_MODULE, package_len,
-			  package, dlerror());
-		goto error;
-	}
-	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
-	if (e != NULL)
-		++e->iparam;
-	return module;
-error:
-	free(module);
-	return NULL;
-}
-
-static void
-module_delete(struct module *module)
-{
-	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
-	if (e != NULL)
-		--e->iparam;
-	dlclose(module->handle);
-	TRASH(module);
-	free(module);
-}
-
-/*
- * Check if a dso is unused and can be closed.
- */
-static void
-module_gc(struct module *module)
-{
-	if (rlist_empty(&module->funcs) && module->calls == 0)
-		module_delete(module);
-}
-
-/*
- * Import a function from the module.
- */
-static box_function_f
-module_sym(struct module *module, const char *name)
-{
-	box_function_f f = (box_function_f)dlsym(module->handle, name);
-	if (f == NULL) {
-		diag_set(ClientError, ER_LOAD_FUNCTION, name, dlerror());
-		return NULL;
-	}
-	return f;
-}
-
-int
-module_sym_load(struct module_sym *mod_sym)
-{
-	assert(mod_sym->addr == NULL);
-
-	struct func_name name;
-	func_split_name(mod_sym->name, &name);
-
-	/*
-	 * In case if module has been loaded already by
-	 * some previous call we can eliminate redundant
-	 * loading and take it from the cache.
-	 */
-	struct module *cached, *module;
-	cached = module_cache_find(name.package, name.package_end);
-	if (cached == NULL) {
-		module = module_load(name.package, name.package_end);
-		if (module == NULL)
-			return -1;
-		if (module_cache_put(module) != 0) {
-			module_delete(module);
-			return -1;
-		}
-	} else {
-		module = cached;
-	}
-
-	mod_sym->addr = module_sym(module, name.sym);
-	if (mod_sym->addr == NULL) {
-		if (cached == NULL) {
-			/*
-			 * In case if it was a first load we should
-			 * clean the cache immediately otherwise
-			 * the module continue being referenced even
-			 * if there will be no use of it.
-			 *
-			 * Note the module_sym set an error thus be
-			 * careful to not wipe it.
-			 */
-			module_cache_del(name.package, name.package_end);
-			module_delete(module);
-		}
-		return -1;
-	}
-	mod_sym->module = module;
-	rlist_add(&module->funcs, &mod_sym->item);
-	return 0;
-}
-
-void
-module_sym_unload(struct module_sym *mod_sym)
-{
-	if (mod_sym->module == NULL)
-		return;
-
-	rlist_del(&mod_sym->item);
-	if (rlist_empty(&mod_sym->module->funcs)) {
-		struct func_name name;
-		func_split_name(mod_sym->name, &name);
-		module_cache_del(name.package, name.package_end);
-	}
-	module_gc(mod_sym->module);
-
-	mod_sym->module = NULL;
-	mod_sym->addr = NULL;
-}
-
-int
-module_reload(const char *package, const char *package_end, struct module **module)
-{
-	struct module *old_module = module_cache_find(package, package_end);
-	if (old_module == NULL) {
-		/* Module wasn't loaded - do nothing. */
-		*module = NULL;
-		return 0;
-	}
-
-	struct module *new_module = module_load(package, package_end);
-	if (new_module == NULL)
-		return -1;
-
-	struct module_sym *mod_sym, *tmp;
-	rlist_foreach_entry_safe(mod_sym, &old_module->funcs, item, tmp) {
-		struct func_name name;
-		func_split_name(mod_sym->name, &name);
-		mod_sym->addr = module_sym(new_module, name.sym);
-		if (mod_sym->addr == NULL)
-			goto restore;
-		mod_sym->module = new_module;
-		rlist_move(&new_module->funcs, &mod_sym->item);
-	}
-	module_cache_del(package, package_end);
-	if (module_cache_put(new_module) != 0)
-		goto restore;
-	module_gc(old_module);
-	*module = new_module;
-	return 0;
-restore:
-	/*
-	 * Some old-dso func can't be load from new module, restore old
-	 * functions.
-	 */
-	do {
-		struct func_name name;
-		func_split_name(mod_sym->name, &name);
-		mod_sym->addr = module_sym(old_module, name.sym);
-		if (mod_sym->addr == NULL) {
-			/*
-			 * 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");
-		}
-		mod_sym->module = old_module;
-		rlist_move(&old_module->funcs, &mod_sym->item);
-	} while (mod_sym != rlist_first_entry(&old_module->funcs,
-					      struct module_sym,
-					      item));
-	assert(rlist_empty(&new_module->funcs));
-	module_delete(new_module);
-	return -1;
-}
-
 static struct func *
 func_c_new(struct func_def *def);
 
@@ -563,39 +124,9 @@ func_c_call(struct func *base, struct port *args, struct port *ret)
 {
 	assert(base->vtab == &func_c_vtab);
 	assert(base != NULL && base->def->language == FUNC_LANGUAGE_C);
-	struct func_c *func = (struct func_c *) base;
-	if (func->mod_sym.addr == NULL) {
-		if (module_sym_load(&func->mod_sym) != 0)
-			return -1;
-	}
-
-	struct region *region = &fiber()->gc;
-	size_t region_svp = region_used(region);
-	uint32_t data_sz;
-	const char *data = port_get_msgpack(args, &data_sz);
-	if (data == NULL)
-		return -1;
-
-	port_c_create(ret);
-	box_function_ctx_t ctx = { ret };
 
-	/* Module can be changed after function reload. */
-	struct module *module = func->mod_sym.module;
-	assert(module != NULL);
-	++module->calls;
-	int rc = func->mod_sym.addr(&ctx, data, data + data_sz);
-	--module->calls;
-	module_gc(module);
-	region_truncate(region, region_svp);
-	if (rc != 0) {
-		if (diag_last_error(&fiber()->diag) == NULL) {
-			/* Stored procedure forget to set diag  */
-			diag_set(ClientError, ER_PROC_C, "unknown error");
-		}
-		port_destroy(ret);
-		return -1;
-	}
-	return rc;
+	struct func_c *func = (struct func_c *)base;
+	return module_sym_call(&func->mod_sym, args, ret);
 }
 
 static struct func_vtab func_c_vtab = {
diff --git a/src/box/func.h b/src/box/func.h
index 9a7f17446..11a466b28 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -34,7 +34,8 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <stdbool.h>
-#include "small/rlist.h"
+
+#include "module_cache.h"
 #include "func_def.h"
 #include "user_def.h"
 
@@ -44,42 +45,6 @@ extern "C" {
 
 struct func;
 
-/**
- * Dynamic shared module.
- */
-struct module {
-	/** Module dlhandle. */
-	void *handle;
-	/** List of imported functions. */
-	struct rlist funcs;
-	/** Count of active calls. */
-	size_t calls;
-	/** Module's package name. */
-	char package[0];
-};
-
-/**
- * Callable symbol bound to a module.
- */
-struct module_sym {
-	/**
-	 * Anchor for module membership.
-	 */
-	struct rlist item;
-	/**
-	 * For C functions, address of the function.
-	 */
-	box_function_f addr;
-	/**
-	 * A module the symbol belongs to.
-	 */
-	struct module *module;
-	/**
-	 * Function name definition.
-	 */
-	char *name;
-};
-
 /** Virtual method table for func object. */
 struct func_vtab {
 	/** Call function with given arguments. */
@@ -106,18 +71,6 @@ struct func {
 	struct access access[BOX_USER_MAX];
 };
 
-/**
- * Initialize modules subsystem.
- */
-int
-module_init(void);
-
-/**
- * Cleanup modules subsystem.
- */
-void
-module_free(void);
-
 struct func *
 func_new(struct func_def *def);
 
@@ -130,38 +83,6 @@ func_delete(struct func *func);
 int
 func_call(struct func *func, struct port *args, struct port *ret);
 
-/**
- * Resolve C entry (find the respective DLL and fetch the
- * symbol from it).
- *
- * @param mod_sym module symbol pointer.
- * @retval -1 on error.
- * @retval 0 on success.
- */
-int
-module_sym_load(struct module_sym *mod_sym);
-
-/**
- * Unload module symbol and drop it from the package
- * cache if there is no users left.
- *
- * @param mod_sym module symbol pointer.
- */
-void
-module_sym_unload(struct module_sym *mod_sym);
-
-/**
- * Reload dynamically loadable module.
- *
- * @param package name begin pointer.
- * @param package_end package_end name end pointer.
- * @param[out] module a pointer to store module object on success.
- * @retval -1 on error.
- * @retval 0 on success.
- */
-int
-module_reload(const char *package, const char *package_end, struct module **module);
-
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/box/func_def.h b/src/box/func_def.h
index d99d89190..75cd6a0d3 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -168,20 +168,6 @@ func_def_dup(struct func_def *def);
 int
 func_def_check(struct func_def *def);
 
-/**
- * API of C stored function.
- */
-
-struct port;
-
-struct box_function_ctx {
-	struct port *port;
-};
-
-typedef struct box_function_ctx box_function_ctx_t;
-typedef int (*box_function_f)(box_function_ctx_t *ctx,
-	     const char *args, const char *args_end);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/box/module_cache.c b/src/box/module_cache.c
new file mode 100644
index 000000000..a66b84efb
--- /dev/null
+++ b/src/box/module_cache.c
@@ -0,0 +1,535 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
+ */
+
+#include <dlfcn.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "assoc.h"
+#include "diag.h"
+#include "error.h"
+#include "errinj.h"
+#include "fiber.h"
+#include "port.h"
+
+#include "error.h"
+#include "lua/utils.h"
+#include "libeio/eio.h"
+
+#include "module_cache.h"
+
+/** Modules name to descriptor hash. */
+static struct mh_strnptr_t *mod_hash = NULL;
+
+/**
+ * Parsed symbol and package names.
+ */
+struct func_name {
+	/**
+	 * Null-terminated symbol name, e.g.
+	 * "func" for "mod.submod.func".
+	 */
+	const char *sym;
+	/**
+	 * Package name, e.g. "mod.submod" for
+	 * "mod.submod.func".
+	 */
+	const char *package;
+	/**
+	 * A pointer to the last character in ->package + 1.
+	 */
+	const char *package_end;
+};
+
+/***
+ * Split function name to symbol and package names.
+ *
+ * For example, str = foo.bar.baz => sym = baz, package = foo.bar
+ *
+ * @param str function name, e.g. "module.submodule.function".
+ * @param[out] name parsed symbol and a package name.
+ */
+static void
+func_split_name(const char *str, struct func_name *name)
+{
+	name->package = str;
+	name->package_end = strrchr(str, '.');
+	if (name->package_end != NULL) {
+		/* module.submodule.function => module.submodule, function */
+		name->sym = name->package_end + 1; /* skip '.' */
+	} else {
+		/* package == function => function, function */
+		name->sym = name->package;
+		name->package_end = str + strlen(str);
+	}
+}
+
+/**
+ * Look up a module in the modules cache.
+ */
+static struct module *
+module_cache_find(const char *name, const char *name_end)
+{
+	mh_int_t e = mh_strnptr_find_inp(mod_hash, name, name_end - name);
+	if (e == mh_end(mod_hash))
+		return NULL;
+	return mh_strnptr_node(mod_hash, e)->val;
+}
+
+/**
+ * Save a module to the modules cache.
+ */
+static int
+module_cache_add(struct module *module)
+{
+	size_t package_len = strlen(module->package);
+	const struct mh_strnptr_node_t nd = {
+		.str	= module->package,
+		.len	= package_len,
+		.hash	= mh_strn_hash(module->package, package_len),
+		.val	= module,
+	};
+
+	if (mh_strnptr_put(mod_hash, &nd, NULL, NULL) == mh_end(mod_hash)) {
+		diag_set(OutOfMemory, sizeof(nd), "malloc",
+			 "module cache node");
+		return -1;
+	}
+	return 0;
+}
+
+/**
+ * Delete a module from the modules cache.
+ */
+static void
+module_cache_del(const char *name, const char *name_end)
+{
+	mh_int_t e = mh_strnptr_find_inp(mod_hash, name, name_end - name);
+	if (e != mh_end(mod_hash))
+		mh_strnptr_del(mod_hash, e, NULL);
+}
+
+/**
+ * Arguments for luaT_module_find used by lua_cpcall().
+ */
+struct module_find_ctx {
+	const char *package;
+	const char *package_end;
+	char *path;
+	size_t path_len;
+};
+
+/**
+ * A cpcall() helper for module_find().
+ */
+static int
+luaT_module_find(lua_State *L)
+{
+	struct module_find_ctx *ctx = (void *)lua_topointer(L, 1);
+
+	/*
+	 * Call package.searchpath(name, package.cpath) and use
+	 * the path to the function in dlopen().
+	 */
+	lua_getglobal(L, "package");
+	lua_getfield(L, -1, "search");
+
+	/* Argument of search: name */
+	lua_pushlstring(L, ctx->package, ctx->package_end - ctx->package);
+
+	lua_call(L, 1, 1);
+	if (lua_isnil(L, -1))
+		return luaL_error(L, "module not found");
+
+	/* Convert path to absolute */
+	char resolved[PATH_MAX];
+	if (realpath(lua_tostring(L, -1), resolved) == NULL) {
+		diag_set(SystemError, "realpath");
+		return luaT_error(L);
+	}
+
+	snprintf(ctx->path, ctx->path_len, "%s", resolved);
+	return 0;
+}
+
+/**
+ * Find a path to a module using Lua's package.cpath.
+ *
+ * @param package package name
+ * @param package_end a pointer to the last byte in @a package + 1
+ * @param[out] path path to shared library
+ * @param path_len size of @a path buffer
+ *
+ * @retval 0 on success
+ * @retval -1 on error, diag is set
+ */
+static int
+module_find(const char *package, const char *package_end,
+	    char *path, size_t path_len)
+{
+	struct module_find_ctx ctx = {
+		.package	= package,
+		.package_end	= package_end,
+		.path		= path,
+		.path_len	= path_len,
+	};
+	lua_State *L = tarantool_L;
+	int top = lua_gettop(L);
+	if (luaT_cpcall(L, luaT_module_find, &ctx) != 0) {
+		diag_set(ClientError, ER_LOAD_MODULE,
+			 (int)(package_end - package),
+			 package, lua_tostring(L, -1));
+		lua_settop(L, top);
+		return -1;
+	}
+	assert(top == lua_gettop(L)); /* cpcall discard results */
+	return 0;
+}
+
+/**
+ * Load dynamic shared object, ie module library.
+ *
+ * Create a new symlink based on temporary directory
+ * and try to load via this symink to load a dso twice
+ * for cases of a function reload.
+ */
+static struct module *
+module_load(const char *package, const char *package_end)
+{
+	char path[PATH_MAX];
+	if (module_find(package, package_end, path, sizeof(path)) != 0)
+		return NULL;
+
+	int package_len = package_end - package;
+	struct module *module = malloc(sizeof(*module) + package_len + 1);
+	if (module == NULL) {
+		diag_set(OutOfMemory, sizeof(*module) + package_len + 1,
+			 "malloc", "struct module");
+		return NULL;
+	}
+	memcpy(module->package, package, package_len);
+	module->package[package_len] = 0;
+	rlist_create(&module->funcs_list);
+	module->calls = 0;
+
+	const char *tmpdir = getenv("TMPDIR");
+	if (tmpdir == NULL)
+		tmpdir = "/tmp";
+
+	char dir_name[PATH_MAX];
+	int rc = snprintf(dir_name, sizeof(dir_name), "%s/tntXXXXXX", tmpdir);
+	if (rc < 0 || (size_t)rc >= sizeof(dir_name)) {
+		diag_set(SystemError, "failed to generate path to tmp dir");
+		goto error;
+	}
+
+	if (mkdtemp(dir_name) == NULL) {
+		diag_set(SystemError, "failed to create unique dir name: %s",
+			 dir_name);
+		goto error;
+	}
+
+	char load_name[PATH_MAX];
+	rc = snprintf(load_name, sizeof(load_name), "%s/%.*s." TARANTOOL_LIBEXT,
+		      dir_name, package_len, package);
+	if (rc < 0 || (size_t)rc >= sizeof(dir_name)) {
+		diag_set(SystemError, "failed to generate path to DSO");
+		goto error;
+	}
+
+	struct stat st;
+	if (stat(path, &st) < 0) {
+		diag_set(SystemError, "failed to stat() module %s", path);
+		goto error;
+	}
+
+	int source_fd = open(path, O_RDONLY);
+	if (source_fd < 0) {
+		diag_set(SystemError, "failed to open module %s", path);
+		goto error;
+	}
+
+	int dest_fd = open(load_name, O_WRONLY | O_CREAT | O_TRUNC,
+			   st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO));
+	if (dest_fd < 0) {
+		diag_set(SystemError, "failed to open file %s for writing ",
+			 load_name);
+		close(source_fd);
+		goto error;
+	}
+
+	off_t ret = eio_sendfile_sync(dest_fd, source_fd, 0, st.st_size);
+	close(source_fd);
+	close(dest_fd);
+	if (ret != st.st_size) {
+		diag_set(SystemError, "failed to copy DSO %s to %s",
+			 path, load_name);
+		goto error;
+	}
+
+	module->handle = dlopen(load_name, RTLD_NOW | RTLD_LOCAL);
+	if (unlink(load_name) != 0)
+		say_warn("failed to unlink dso link %s", load_name);
+	if (rmdir(dir_name) != 0)
+		say_warn("failed to delete temporary dir %s", dir_name);
+	if (module->handle == NULL) {
+		diag_set(ClientError, ER_LOAD_MODULE, package_len,
+			  package, dlerror());
+		goto error;
+	}
+
+	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
+	if (e != NULL)
+		++e->iparam;
+	return module;
+
+error:
+	free(module);
+	return NULL;
+}
+
+/**
+ * Delete a module.
+ */
+static void
+module_delete(struct module *module)
+{
+	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
+	if (e != NULL)
+		--e->iparam;
+	dlclose(module->handle);
+	TRASH(module);
+	free(module);
+}
+
+/**
+ * Check if a module is unused and delete it then.
+ */
+static void
+module_gc(struct module *module)
+{
+	if (rlist_empty(&module->funcs_list) && module->calls == 0)
+		module_delete(module);
+}
+
+/**
+ * Import a function from a module.
+ */
+static box_function_f
+module_sym(struct module *module, const char *name)
+{
+	box_function_f f = dlsym(module->handle, name);
+	if (f == NULL) {
+		diag_set(ClientError, ER_LOAD_FUNCTION, name, dlerror());
+		return NULL;
+	}
+	return f;
+}
+
+int
+module_sym_load(struct module_sym *mod_sym)
+{
+	assert(mod_sym->addr == NULL);
+
+	struct func_name name;
+	func_split_name(mod_sym->name, &name);
+
+	/*
+	 * In case if module has been loaded already by
+	 * some previous call we can eliminate redundant
+	 * loading and take it from the cache.
+	 */
+	struct module *cached, *module;
+	cached = module_cache_find(name.package, name.package_end);
+	if (cached == NULL) {
+		module = module_load(name.package, name.package_end);
+		if (module == NULL)
+			return -1;
+		if (module_cache_add(module) != 0) {
+			module_delete(module);
+			return -1;
+		}
+	} else {
+		module = cached;
+	}
+
+	mod_sym->addr = module_sym(module, name.sym);
+	if (mod_sym->addr == NULL) {
+		if (cached == NULL) {
+			/*
+			 * In case if it was a first load we should
+			 * clean the cache immediately otherwise
+			 * the module continue being referenced even
+			 * if there will be no use of it.
+			 *
+			 * Note the module_sym set an error thus be
+			 * careful to not wipe it.
+			 */
+			module_cache_del(name.package, name.package_end);
+			module_delete(module);
+		}
+		return -1;
+	}
+
+	mod_sym->module = module;
+	rlist_add(&module->funcs_list, &mod_sym->item);
+	return 0;
+}
+
+void
+module_sym_unload(struct module_sym *mod_sym)
+{
+	if (mod_sym->module == NULL)
+		return;
+
+	rlist_del(&mod_sym->item);
+	if (rlist_empty(&mod_sym->module->funcs_list)) {
+		struct func_name name;
+		func_split_name(mod_sym->name, &name);
+		module_cache_del(name.package, name.package_end);
+	}
+	module_gc(mod_sym->module);
+
+	mod_sym->module = NULL;
+	mod_sym->addr = NULL;
+}
+
+int
+module_sym_call(struct module_sym *mod_sym, struct port *args,
+		struct port *ret)
+{
+	if (mod_sym->addr == NULL) {
+		if (module_sym_load(mod_sym) != 0)
+			return -1;
+	}
+
+	struct region *region = &fiber()->gc;
+	size_t region_svp = region_used(region);
+
+	uint32_t data_sz;
+	const char *data = port_get_msgpack(args, &data_sz);
+	if (data == NULL)
+		return -1;
+
+	port_c_create(ret);
+	box_function_ctx_t ctx = {
+		.port = ret,
+	};
+
+	/*
+	 * Module can be changed after function reload. Also
+	 * keep in mind that stored C procedure may yield inside.
+	 */
+	struct module *module = mod_sym->module;
+	assert(module != NULL);
+	++module->calls;
+	int rc = mod_sym->addr(&ctx, data, data + data_sz);
+	--module->calls;
+	module_gc(module);
+	region_truncate(region, region_svp);
+
+	if (rc != 0) {
+		if (diag_last_error(&fiber()->diag) == NULL) {
+			/* Stored procedure forget to set diag  */
+			diag_set(ClientError, ER_PROC_C, "unknown error");
+		}
+		port_destroy(ret);
+		return -1;
+	}
+
+	return rc;
+}
+
+int
+module_reload(const char *package, const char *package_end,
+	      struct module **module)
+{
+	struct module *old = module_cache_find(package, package_end);
+	if (old == NULL) {
+		/* Module wasn't loaded - do nothing. */
+		*module = NULL;
+		return 0;
+	}
+
+	struct module *new = module_load(package, package_end);
+	if (new == NULL)
+		return -1;
+
+	struct module_sym *mod_sym, *tmp;
+	rlist_foreach_entry_safe(mod_sym, &old->funcs_list, item, tmp) {
+		struct func_name name;
+		func_split_name(mod_sym->name, &name);
+
+		mod_sym->addr = module_sym(new, name.sym);
+		if (mod_sym->addr == NULL) {
+			say_error("module: reload %s, symbol %s not found",
+				  package, name.sym);
+			goto restore;
+		}
+
+		mod_sym->module = new;
+		rlist_move(&new->funcs_list, &mod_sym->item);
+	}
+
+	module_cache_del(package, package_end);
+	if (module_cache_add(new) != 0)
+		goto restore;
+
+	module_gc(old);
+	*module = new;
+	return 0;
+
+restore:
+	/*
+	 * Some old-dso func can't be load from new module,
+	 * restore old functions.
+	 */
+	do {
+		struct func_name name;
+		func_split_name(mod_sym->name, &name);
+		mod_sym->addr = module_sym(old, name.sym);
+		if (mod_sym->addr == NULL) {
+			/*
+			 * 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");
+		}
+		mod_sym->module = old;
+		rlist_move(&old->funcs_list, &mod_sym->item);
+	} while (mod_sym != rlist_first_entry(&old->funcs_list,
+					      struct module_sym,
+					      item));
+	assert(rlist_empty(&new->funcs_list));
+	module_delete(new);
+	return -1;
+}
+
+int
+module_init(void)
+{
+	mod_hash = mh_strnptr_new();
+	if (mod_hash == NULL) {
+		diag_set(OutOfMemory, sizeof(*mod_hash),
+			 "malloc", "modules hash table");
+		return -1;
+	}
+	return 0;
+}
+
+void
+module_free(void)
+{
+	while (mh_size(mod_hash) > 0) {
+		mh_int_t i = mh_first(mod_hash);
+		struct module *m = mh_strnptr_node(mod_hash, i)->val;
+		module_gc(m);
+	}
+	mh_strnptr_delete(mod_hash);
+	mod_hash = NULL;
+}
diff --git a/src/box/module_cache.h b/src/box/module_cache.h
new file mode 100644
index 000000000..0f5d2b64a
--- /dev/null
+++ b/src/box/module_cache.h
@@ -0,0 +1,139 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
+ */
+
+#pragma once
+
+#include "small/rlist.h"
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+
+/**
+ * API of C stored function.
+ */
+
+struct port;
+
+struct box_function_ctx {
+	struct port *port;
+};
+
+typedef struct box_function_ctx box_function_ctx_t;
+typedef int (*box_function_f)(box_function_ctx_t *ctx,
+			      const char *args,
+			      const char *args_end);
+
+/**
+ * Dynamic shared module.
+ */
+struct module {
+	/**
+	 * Module dlhandle.
+	 */
+	void *handle;
+	/**
+	 * List of associated symbols (functions).
+	 */
+	struct rlist funcs_list;
+	/**
+	 * Count of active calls.
+	 */
+	size_t calls;
+	/**
+	 * Module's package name.
+	 */
+	char package[0];
+};
+
+/**
+ * Callable symbol bound to a module.
+ */
+struct module_sym {
+	/**
+	 * Anchor for module::funcs_list.
+	 */
+	struct rlist item;
+	/**
+	 * For C functions, address of the function.
+	 */
+	box_function_f addr;
+	/**
+	 * A module the symbol belongs to.
+	 */
+	struct module *module;
+	/**
+	 * Symbol (function) name definition.
+	 */
+	char *name;
+};
+
+/**
+ * Load a new module symbol.
+ *
+ * @param mod_sym symbol to load.
+ *
+ * @returns 0 on succse, -1 otherwise, diag is set.
+ */
+int
+module_sym_load(struct module_sym *mod_sym);
+
+/**
+ * Unload a module's symbol.
+ *
+ * @param mod_sym symbol to unload.
+ */
+void
+module_sym_unload(struct module_sym *mod_sym);
+
+/**
+ * Execute a module symbol (run a function).
+ *
+ * The function packs function arguments into a message pack
+ * and send it as a function argument. Function may return
+ * results via @a ret stream.
+ *
+ * @param mod_sym module symbol to run.
+ * @param args function arguments.
+ * @param[out] ret execution results.
+ *
+ * @returns 0 on success, -1 otherwise, diag is set.
+ */
+int
+module_sym_call(struct module_sym *mod_sym, struct port *args,
+		struct port *ret);
+
+/**
+ * Reload a module and all associated symbols.
+ *
+ * @param package shared library path start.
+ * @param package_end shared library path end.
+ * @param[out] module pointer to the reloaded module.
+ *
+ * @return 0 on succes, -1 otherwise, diag is set.
+ */
+int
+module_reload(const char *package, const char *package_end,
+	      struct module **module);
+
+/**
+ * Initialize modules subsystem.
+ *
+ * @return 0 on succes, -1 otherwise, diag is set.
+ */
+int
+module_init(void);
+
+/**
+ * Free modules subsystem.
+ */
+void
+module_free(void);
+
+#if defined(__cplusplus)
+}
+#endif /* defined(__plusplus) */
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 03/12] module_cache: direct update a cache value on reload
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 01/12] box/func: factor out c function entry structure Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 02/12] module_cache: move module handling into own subsystem Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:11 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 04/12] module_cache: rename calls to ref in module structure Cyrill Gorcunov via Tarantool-patches
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:11 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

Currently when we reload modules we remove old instance
from the module cache and then try to insert a new one
back. Note that module cache is a string based hash table:
we lookup for a pointer to the module via package name.

Thus when reload initiated we simply remove same key
from hash and put it back with a new pointer. This is
too complex and completely useless. Instead we should
simply update the value associated this the key in the
hash table. For this sake we introduce module_cache_update
helper.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/module_cache.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index a66b84efb..b3bfb4963 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -102,6 +102,22 @@ module_cache_add(struct module *module)
 	return 0;
 }
 
+/**
+ * Update the module cache. Since the lookup is string
+ * key based it is safe to just update the value.
+ */
+static int
+module_cache_update(const char *name, const char *name_end,
+		    struct module *module)
+{
+	mh_int_t e = mh_strnptr_find_inp(mod_hash, name, name_end - name);
+	if (e == mh_end(mod_hash))
+		return -1;
+	mh_strnptr_node(mod_hash, e)->str = module->package;
+	mh_strnptr_node(mod_hash, e)->val = module;
+	return 0;
+}
+
 /**
  * Delete a module from the modules cache.
  */
@@ -475,9 +491,15 @@ module_reload(const char *package, const char *package_end,
 		rlist_move(&new->funcs_list, &mod_sym->item);
 	}
 
-	module_cache_del(package, package_end);
-	if (module_cache_add(new) != 0)
-		goto restore;
+	if (module_cache_update(package, package_end, new) != 0) {
+		/*
+		 * Module cache must be consistent at this moment,
+		 * we've looked up for the package recently. If
+		 * someone has updated the cache in unexpected way
+		 * the consistency is lost and we must not continue.
+		 */
+		panic("module: can't update module cache (%s)", package);
+	}
 
 	module_gc(old);
 	*module = new;
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 04/12] module_cache: rename calls to ref in module structure
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (2 preceding siblings ...)
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 03/12] module_cache: direct update a cache value on reload Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:11 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 05/12] module_cache: add comment about weird resolving Cyrill Gorcunov via Tarantool-patches
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:11 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

We will use this field not only to count a number of
active calls to the module but to prevent the module
from disappear when it get loaded via "cmod" interface
(will be implemented in next patches).

Same time make it 64 bit long since there might be a
number of such references size_t didn't prevent from
numeric overflow.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/module_cache.c | 8 ++++----
 src/box/module_cache.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index b3bfb4963..9e5670c03 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -230,7 +230,7 @@ module_load(const char *package, const char *package_end)
 	memcpy(module->package, package, package_len);
 	module->package[package_len] = 0;
 	rlist_create(&module->funcs_list);
-	module->calls = 0;
+	module->refs = 0;
 
 	const char *tmpdir = getenv("TMPDIR");
 	if (tmpdir == NULL)
@@ -328,7 +328,7 @@ module_delete(struct module *module)
 static void
 module_gc(struct module *module)
 {
-	if (rlist_empty(&module->funcs_list) && module->calls == 0)
+	if (rlist_empty(&module->funcs_list) && module->refs == 0)
 		module_delete(module);
 }
 
@@ -442,9 +442,9 @@ module_sym_call(struct module_sym *mod_sym, struct port *args,
 	 */
 	struct module *module = mod_sym->module;
 	assert(module != NULL);
-	++module->calls;
+	++module->refs;
 	int rc = mod_sym->addr(&ctx, data, data + data_sz);
-	--module->calls;
+	--module->refs;
 	module_gc(module);
 	region_truncate(region, region_svp);
 
diff --git a/src/box/module_cache.h b/src/box/module_cache.h
index 0f5d2b64a..0d56aea92 100644
--- a/src/box/module_cache.h
+++ b/src/box/module_cache.h
@@ -41,9 +41,9 @@ struct module {
 	 */
 	struct rlist funcs_list;
 	/**
-	 * Count of active calls.
+	 * Count of active references to the module.
 	 */
-	size_t calls;
+	int64_t refs;
 	/**
 	 * Module's package name.
 	 */
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 05/12] module_cache: add comment about weird resolving
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (3 preceding siblings ...)
  2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 04/12] module_cache: rename calls to ref in module structure Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 06/12] module_cache: module_reload - drop redundant parameter Cyrill Gorcunov via Tarantool-patches
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

The module management is not obvious when reading
the code, mostly because of weird design in first
place which we can't change without breaking
backward compatibility.

Add a note about this into code itself otherwise
it is hard to understand why it works the way it
does.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/module_cache.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index 9e5670c03..60af290ae 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -418,6 +418,18 @@ int
 module_sym_call(struct module_sym *mod_sym, struct port *args,
 		struct port *ret)
 {
+	/*
+	 * The functions created with `box.schema.func`
+	 * help are not resolved immediately. Instead
+	 * they are deferred until first call. And when
+	 * call happens the we try to load a module and
+	 * resolve a symbol (which of course can fail if
+	 * there is no such module at all).
+	 *
+	 * While this is very weird (and frankly speaking
+	 * very bad design) we can't change it for backward
+	 * compatibility sake!
+	 */
 	if (mod_sym->addr == NULL) {
 		if (module_sym_load(mod_sym) != 0)
 			return -1;
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 06/12] module_cache: module_reload - drop redundant parameter
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (4 preceding siblings ...)
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 05/12] module_cache: add comment about weird resolving Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 07/12] module_cache: use references as a main usage counter Cyrill Gorcunov via Tarantool-patches
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

There is no need to return new module pointer, it is
only used in a caller code to find if there was no
such module at all.

Instead call diag_set by self and drop this useless
argument.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/call.c         | 10 ++--------
 src/box/module_cache.c |  9 +++------
 src/box/module_cache.h |  4 +---
 3 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/src/box/call.c b/src/box/call.c
index 9c291260e..2a1a2c5bb 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -128,14 +128,8 @@ box_module_reload(const char *name)
 				 user->def->name);
 		return -1;
 	}
-	struct module *module = NULL;
-	if (module_reload(name, name + strlen(name), &module) == 0) {
-		if (module != NULL)
-			return 0;
-		else
-			diag_set(ClientError, ER_NO_SUCH_MODULE, name);
-	}
-	return -1;
+
+	return module_reload(name, name + strlen(name));
 }
 
 int
diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index 60af290ae..4c64a6dc5 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -473,14 +473,12 @@ module_sym_call(struct module_sym *mod_sym, struct port *args,
 }
 
 int
-module_reload(const char *package, const char *package_end,
-	      struct module **module)
+module_reload(const char *package, const char *package_end)
 {
 	struct module *old = module_cache_find(package, package_end);
 	if (old == NULL) {
-		/* Module wasn't loaded - do nothing. */
-		*module = NULL;
-		return 0;
+		diag_set(ClientError, ER_NO_SUCH_MODULE, package);
+		return -1;
 	}
 
 	struct module *new = module_load(package, package_end);
@@ -514,7 +512,6 @@ module_reload(const char *package, const char *package_end,
 	}
 
 	module_gc(old);
-	*module = new;
 	return 0;
 
 restore:
diff --git a/src/box/module_cache.h b/src/box/module_cache.h
index 0d56aea92..fa62628c3 100644
--- a/src/box/module_cache.h
+++ b/src/box/module_cache.h
@@ -112,13 +112,11 @@ module_sym_call(struct module_sym *mod_sym, struct port *args,
  *
  * @param package shared library path start.
  * @param package_end shared library path end.
- * @param[out] module pointer to the reloaded module.
  *
  * @return 0 on succes, -1 otherwise, diag is set.
  */
 int
-module_reload(const char *package, const char *package_end,
-	      struct module **module);
+module_reload(const char *package, const char *package_end);
 
 /**
  * Initialize modules subsystem.
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 07/12] module_cache: use references as a main usage counter
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (5 preceding siblings ...)
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 06/12] module_cache: module_reload - drop redundant parameter Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 08/12] module_cache: make module to carry hash it belongs to Cyrill Gorcunov via Tarantool-patches
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

We use module:funcs_list as implicit reference counter,
lets get rid of it and use explicit counting with
module_ref/unref help.

An interesting moment is module reload: we can't drop
old instance from hash because hash now points to
a new module.

Part-of #4642

Suggested-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/module_cache.c | 137 ++++++++++++++++++++++++++---------------
 1 file changed, 87 insertions(+), 50 deletions(-)

diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index 4c64a6dc5..bb6ef39cd 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -129,6 +129,24 @@ module_cache_del(const char *name, const char *name_end)
 		mh_strnptr_del(mod_hash, e, NULL);
 }
 
+/**
+ * Mark module as out of cache.
+ */
+static void
+module_set_orphan(struct module *module)
+{
+	module->package[0] = '\0';
+}
+
+/**
+ * Test if module is out of cache.
+ */
+static bool
+module_is_orphan(struct module *module)
+{
+	return module->package[0] == '\0';
+}
+
 /**
  * Arguments for luaT_module_find used by lua_cpcall().
  */
@@ -206,6 +224,47 @@ module_find(const char *package, const char *package_end,
 	return 0;
 }
 
+/**
+ * Delete a module.
+ */
+static void
+module_delete(struct module *module)
+{
+	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
+	if (e != NULL)
+		--e->iparam;
+	dlclose(module->handle);
+	TRASH(module);
+	free(module);
+}
+
+/**
+ * Increase reference to a module.
+ */
+static void
+module_ref(struct module *module)
+{
+	assert(module->refs >= 0);
+	module->refs++;
+}
+
+/**
+ * Decrease module reference and delete it if last one.
+ */
+static void
+module_unref(struct module *module)
+{
+	assert(module->refs > 0);
+	if (module->refs-- == 1) {
+		if (!module_is_orphan(module)) {
+			size_t len = strlen(module->package);
+			module_cache_del(module->package,
+					 &module->package[len]);
+		}
+		module_delete(module);
+	}
+}
+
 /**
  * Load dynamic shared object, ie module library.
  *
@@ -301,6 +360,7 @@ module_load(const char *package, const char *package_end)
 	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
 	if (e != NULL)
 		++e->iparam;
+	module_ref(module);
 	return module;
 
 error:
@@ -308,30 +368,6 @@ module_load(const char *package, const char *package_end)
 	return NULL;
 }
 
-/**
- * Delete a module.
- */
-static void
-module_delete(struct module *module)
-{
-	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
-	if (e != NULL)
-		--e->iparam;
-	dlclose(module->handle);
-	TRASH(module);
-	free(module);
-}
-
-/**
- * Check if a module is unused and delete it then.
- */
-static void
-module_gc(struct module *module)
-{
-	if (rlist_empty(&module->funcs_list) && module->refs == 0)
-		module_delete(module);
-}
-
 /**
  * Import a function from a module.
  */
@@ -366,28 +402,17 @@ module_sym_load(struct module_sym *mod_sym)
 		if (module == NULL)
 			return -1;
 		if (module_cache_add(module) != 0) {
-			module_delete(module);
+			module_unref(module);
 			return -1;
 		}
 	} else {
+		module_ref(cached);
 		module = cached;
 	}
 
 	mod_sym->addr = module_sym(module, name.sym);
 	if (mod_sym->addr == NULL) {
-		if (cached == NULL) {
-			/*
-			 * In case if it was a first load we should
-			 * clean the cache immediately otherwise
-			 * the module continue being referenced even
-			 * if there will be no use of it.
-			 *
-			 * Note the module_sym set an error thus be
-			 * careful to not wipe it.
-			 */
-			module_cache_del(name.package, name.package_end);
-			module_delete(module);
-		}
+		module_unref(module);
 		return -1;
 	}
 
@@ -403,12 +428,11 @@ module_sym_unload(struct module_sym *mod_sym)
 		return;
 
 	rlist_del(&mod_sym->item);
-	if (rlist_empty(&mod_sym->module->funcs_list)) {
-		struct func_name name;
-		func_split_name(mod_sym->name, &name);
-		module_cache_del(name.package, name.package_end);
-	}
-	module_gc(mod_sym->module);
+	/*
+	 * Unref action might delete module
+	 * so call it after rlist_del.
+	 */
+	module_unref(mod_sym->module);
 
 	mod_sym->module = NULL;
 	mod_sym->addr = NULL;
@@ -454,10 +478,9 @@ module_sym_call(struct module_sym *mod_sym, struct port *args,
 	 */
 	struct module *module = mod_sym->module;
 	assert(module != NULL);
-	++module->refs;
+	module_ref(module);
 	int rc = mod_sym->addr(&ctx, data, data + data_sz);
-	--module->refs;
-	module_gc(module);
+	module_unref(module);
 	region_truncate(region, region_svp);
 
 	if (rc != 0) {
@@ -485,6 +508,9 @@ module_reload(const char *package, const char *package_end)
 	if (new == NULL)
 		return -1;
 
+	/* Extra ref until cache get updated */
+	module_ref(old);
+
 	struct module_sym *mod_sym, *tmp;
 	rlist_foreach_entry_safe(mod_sym, &old->funcs_list, item, tmp) {
 		struct func_name name;
@@ -499,6 +525,10 @@ module_reload(const char *package, const char *package_end)
 
 		mod_sym->module = new;
 		rlist_move(&new->funcs_list, &mod_sym->item);
+
+		/* Move reference to a new place */
+		module_ref(new);
+		module_unref(old);
 	}
 
 	if (module_cache_update(package, package_end, new) != 0) {
@@ -511,10 +541,15 @@ module_reload(const char *package, const char *package_end)
 		panic("module: can't update module cache (%s)", package);
 	}
 
-	module_gc(old);
+	module_set_orphan(old);
+	module_unref(old);
+
+	/* From explicit load above */
+	module_unref(new);
 	return 0;
 
 restore:
+	module_set_orphan(new);
 	/*
 	 * Some old-dso func can't be load from new module,
 	 * restore old functions.
@@ -533,11 +568,13 @@ module_reload(const char *package, const char *package_end)
 		}
 		mod_sym->module = old;
 		rlist_move(&old->funcs_list, &mod_sym->item);
+		module_ref(old);
+		module_unref(new);
 	} while (mod_sym != rlist_first_entry(&old->funcs_list,
 					      struct module_sym,
 					      item));
 	assert(rlist_empty(&new->funcs_list));
-	module_delete(new);
+	module_unref(new);
 	return -1;
 }
 
@@ -559,7 +596,7 @@ module_free(void)
 	while (mh_size(mod_hash) > 0) {
 		mh_int_t i = mh_first(mod_hash);
 		struct module *m = mh_strnptr_node(mod_hash, i)->val;
-		module_gc(m);
+		module_unref(m);
 	}
 	mh_strnptr_delete(mod_hash);
 	mod_hash = NULL;
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 08/12] module_cache: make module to carry hash it belongs to
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (6 preceding siblings ...)
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 07/12] module_cache: use references as a main usage counter Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 09/12] module_cache: use own hash for box.schema.func requests Cyrill Gorcunov via Tarantool-patches
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

We will need two hashes for backward compatibility sake,
thus make module structure to carry the hash it belongs.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/module_cache.c | 66 ++++++++++++++++++++++++------------------
 src/box/module_cache.h |  4 +++
 2 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index bb6ef39cd..d09947719 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -72,12 +72,13 @@ func_split_name(const char *str, struct func_name *name)
  * Look up a module in the modules cache.
  */
 static struct module *
-module_cache_find(const char *name, const char *name_end)
+module_cache_find(struct mh_strnptr_t *h, const char *name,
+		  const char *name_end)
 {
-	mh_int_t e = mh_strnptr_find_inp(mod_hash, name, name_end - name);
-	if (e == mh_end(mod_hash))
+	mh_int_t e = mh_strnptr_find_inp(h, name, name_end - name);
+	if (e == mh_end(h))
 		return NULL;
-	return mh_strnptr_node(mod_hash, e)->val;
+	return mh_strnptr_node(h, e)->val;
 }
 
 /**
@@ -86,6 +87,7 @@ module_cache_find(const char *name, const char *name_end)
 static int
 module_cache_add(struct module *module)
 {
+	struct mh_strnptr_t *h = module->hash;
 	size_t package_len = strlen(module->package);
 	const struct mh_strnptr_node_t nd = {
 		.str	= module->package,
@@ -94,7 +96,7 @@ module_cache_add(struct module *module)
 		.val	= module,
 	};
 
-	if (mh_strnptr_put(mod_hash, &nd, NULL, NULL) == mh_end(mod_hash)) {
+	if (mh_strnptr_put(h, &nd, NULL, NULL) == mh_end(h)) {
 		diag_set(OutOfMemory, sizeof(nd), "malloc",
 			 "module cache node");
 		return -1;
@@ -107,14 +109,17 @@ module_cache_add(struct module *module)
  * key based it is safe to just update the value.
  */
 static int
-module_cache_update(const char *name, const char *name_end,
-		    struct module *module)
+module_cache_update(struct module *module)
 {
-	mh_int_t e = mh_strnptr_find_inp(mod_hash, name, name_end - name);
-	if (e == mh_end(mod_hash))
+	struct mh_strnptr_t *h = module->hash;
+	const char *name = module->package;
+	size_t len = strlen(module->package);
+
+	mh_int_t e = mh_strnptr_find_inp(h, name, len);
+	if (e == mh_end(h))
 		return -1;
-	mh_strnptr_node(mod_hash, e)->str = module->package;
-	mh_strnptr_node(mod_hash, e)->val = module;
+	mh_strnptr_node(h, e)->str = module->package;
+	mh_strnptr_node(h, e)->val = module;
 	return 0;
 }
 
@@ -122,11 +127,15 @@ module_cache_update(const char *name, const char *name_end,
  * Delete a module from the modules cache.
  */
 static void
-module_cache_del(const char *name, const char *name_end)
+module_cache_del(struct module *module)
 {
-	mh_int_t e = mh_strnptr_find_inp(mod_hash, name, name_end - name);
-	if (e != mh_end(mod_hash))
-		mh_strnptr_del(mod_hash, e, NULL);
+	struct mh_strnptr_t *h = module->hash;
+	const char *name = module->package;
+	size_t len = strlen(module->package);
+
+	mh_int_t e = mh_strnptr_find_inp(h, name, len);
+	if (e != mh_end(h))
+		mh_strnptr_del(h, e, NULL);
 }
 
 /**
@@ -135,7 +144,7 @@ module_cache_del(const char *name, const char *name_end)
 static void
 module_set_orphan(struct module *module)
 {
-	module->package[0] = '\0';
+	module->hash = NULL;
 }
 
 /**
@@ -144,7 +153,7 @@ module_set_orphan(struct module *module)
 static bool
 module_is_orphan(struct module *module)
 {
-	return module->package[0] == '\0';
+	return module->hash == NULL;
 }
 
 /**
@@ -256,11 +265,8 @@ module_unref(struct module *module)
 {
 	assert(module->refs > 0);
 	if (module->refs-- == 1) {
-		if (!module_is_orphan(module)) {
-			size_t len = strlen(module->package);
-			module_cache_del(module->package,
-					 &module->package[len]);
-		}
+		if (!module_is_orphan(module))
+			module_cache_del(module);
 		module_delete(module);
 	}
 }
@@ -273,7 +279,8 @@ module_unref(struct module *module)
  * for cases of a function reload.
  */
 static struct module *
-module_load(const char *package, const char *package_end)
+module_load(struct mh_strnptr_t *h, const char *package,
+	    const char *package_end)
 {
 	char path[PATH_MAX];
 	if (module_find(package, package_end, path, sizeof(path)) != 0)
@@ -290,6 +297,7 @@ module_load(const char *package, const char *package_end)
 	module->package[package_len] = 0;
 	rlist_create(&module->funcs_list);
 	module->refs = 0;
+	module->hash = h;
 
 	const char *tmpdir = getenv("TMPDIR");
 	if (tmpdir == NULL)
@@ -396,9 +404,9 @@ module_sym_load(struct module_sym *mod_sym)
 	 * loading and take it from the cache.
 	 */
 	struct module *cached, *module;
-	cached = module_cache_find(name.package, name.package_end);
+	cached = module_cache_find(mod_hash, name.package, name.package_end);
 	if (cached == NULL) {
-		module = module_load(name.package, name.package_end);
+		module = module_load(mod_hash, name.package, name.package_end);
 		if (module == NULL)
 			return -1;
 		if (module_cache_add(module) != 0) {
@@ -498,13 +506,15 @@ module_sym_call(struct module_sym *mod_sym, struct port *args,
 int
 module_reload(const char *package, const char *package_end)
 {
-	struct module *old = module_cache_find(package, package_end);
+	struct module *old, *new;
+
+	old = module_cache_find(mod_hash, package, package_end);
 	if (old == NULL) {
 		diag_set(ClientError, ER_NO_SUCH_MODULE, package);
 		return -1;
 	}
 
-	struct module *new = module_load(package, package_end);
+	new = module_load(mod_hash, package, package_end);
 	if (new == NULL)
 		return -1;
 
@@ -531,7 +541,7 @@ module_reload(const char *package, const char *package_end)
 		module_unref(old);
 	}
 
-	if (module_cache_update(package, package_end, new) != 0) {
+	if (module_cache_update(new) != 0) {
 		/*
 		 * Module cache must be consistent at this moment,
 		 * we've looked up for the package recently. If
diff --git a/src/box/module_cache.h b/src/box/module_cache.h
index fa62628c3..874cc081c 100644
--- a/src/box/module_cache.h
+++ b/src/box/module_cache.h
@@ -36,6 +36,10 @@ struct module {
 	 * Module dlhandle.
 	 */
 	void *handle;
+	/**
+	 * Package hash module belongs to.
+	 */
+	struct mh_strnptr_t *hash;
 	/**
 	 * List of associated symbols (functions).
 	 */
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 09/12] module_cache: use own hash for box.schema.func requests
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (7 preceding siblings ...)
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 08/12] module_cache: make module to carry hash it belongs to Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 10/12] module_cache: provide module_load/unload calls Cyrill Gorcunov via Tarantool-patches
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

This is needed to distinguish requests from future `cmod` module
(which gonna be detecting modules renew automatically) and old
`box.schema.func` interface. To make it clear use `box_schema_hash`
for such modules.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/module_cache.c | 48 ++++++++++++++++++++++++++++--------------
 src/box/module_cache.h |  3 ++-
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index d09947719..f9ed6e7d8 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -23,7 +23,7 @@
 #include "module_cache.h"
 
 /** Modules name to descriptor hash. */
-static struct mh_strnptr_t *mod_hash = NULL;
+static struct mh_strnptr_t *box_schema_hash = NULL;
 
 /**
  * Parsed symbol and package names.
@@ -45,6 +45,16 @@ struct func_name {
 	const char *package_end;
 };
 
+/**
+ * Return module hash depending on where request comes
+ * from: if it is legacy `box.schema.func` interface or not.
+ */
+static inline struct mh_strnptr_t *
+hash_tbl(bool is_box_schema)
+{
+	return is_box_schema ? box_schema_hash : NULL;
+}
+
 /***
  * Split function name to symbol and package names.
  *
@@ -391,7 +401,7 @@ module_sym(struct module *module, const char *name)
 }
 
 int
-module_sym_load(struct module_sym *mod_sym)
+module_sym_load(struct module_sym *mod_sym, bool is_box_schema)
 {
 	assert(mod_sym->addr == NULL);
 
@@ -404,9 +414,10 @@ module_sym_load(struct module_sym *mod_sym)
 	 * loading and take it from the cache.
 	 */
 	struct module *cached, *module;
-	cached = module_cache_find(mod_hash, name.package, name.package_end);
+	struct mh_strnptr_t *h = hash_tbl(is_box_schema);
+	cached = module_cache_find(h, name.package, name.package_end);
 	if (cached == NULL) {
-		module = module_load(mod_hash, name.package, name.package_end);
+		module = module_load(h, name.package, name.package_end);
 		if (module == NULL)
 			return -1;
 		if (module_cache_add(module) != 0) {
@@ -463,7 +474,7 @@ module_sym_call(struct module_sym *mod_sym, struct port *args,
 	 * compatibility sake!
 	 */
 	if (mod_sym->addr == NULL) {
-		if (module_sym_load(mod_sym) != 0)
+		if (module_sym_load(mod_sym, true) != 0)
 			return -1;
 	}
 
@@ -508,13 +519,17 @@ module_reload(const char *package, const char *package_end)
 {
 	struct module *old, *new;
 
-	old = module_cache_find(mod_hash, package, package_end);
+	/*
+	 * Explicit module reloading is deprecated interface,
+	 * so always use box_schema_hash.
+	 */
+	old = module_cache_find(box_schema_hash, package, package_end);
 	if (old == NULL) {
 		diag_set(ClientError, ER_NO_SUCH_MODULE, package);
 		return -1;
 	}
 
-	new = module_load(mod_hash, package, package_end);
+	new = module_load(box_schema_hash, package, package_end);
 	if (new == NULL)
 		return -1;
 
@@ -591,10 +606,10 @@ module_reload(const char *package, const char *package_end)
 int
 module_init(void)
 {
-	mod_hash = mh_strnptr_new();
-	if (mod_hash == NULL) {
-		diag_set(OutOfMemory, sizeof(*mod_hash),
-			 "malloc", "modules hash table");
+	box_schema_hash = mh_strnptr_new();
+	if (box_schema_hash == NULL) {
+		diag_set(OutOfMemory, sizeof(*box_schema_hash),
+			 "malloc", "modules box_schema_hash");
 		return -1;
 	}
 	return 0;
@@ -603,11 +618,12 @@ module_init(void)
 void
 module_free(void)
 {
-	while (mh_size(mod_hash) > 0) {
-		mh_int_t i = mh_first(mod_hash);
-		struct module *m = mh_strnptr_node(mod_hash, i)->val;
+	struct mh_strnptr_t *h = box_schema_hash;
+	while (mh_size(h) > 0) {
+		mh_int_t i = mh_first(h);
+		struct module *m = mh_strnptr_node(h, i)->val;
 		module_unref(m);
 	}
-	mh_strnptr_delete(mod_hash);
-	mod_hash = NULL;
+	mh_strnptr_delete(box_schema_hash);
+	box_schema_hash = NULL;
 }
diff --git a/src/box/module_cache.h b/src/box/module_cache.h
index 874cc081c..875f2eb3c 100644
--- a/src/box/module_cache.h
+++ b/src/box/module_cache.h
@@ -80,11 +80,12 @@ struct module_sym {
  * Load a new module symbol.
  *
  * @param mod_sym symbol to load.
+ * @param is_box_schema flag if request comes from `box.schema.func`.
  *
  * @returns 0 on succse, -1 otherwise, diag is set.
  */
 int
-module_sym_load(struct module_sym *mod_sym);
+module_sym_load(struct module_sym *mod_sym, bool is_box_schema);
 
 /**
  * Unload a module's symbol.
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 10/12] module_cache: provide module_load/unload calls
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (8 preceding siblings ...)
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 09/12] module_cache: use own hash for box.schema.func requests Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 11/12] box/cmod: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 12/12] test: box/cfunc -- add cmod test Cyrill Gorcunov via Tarantool-patches
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

These calls provide an ability to load and unload modules
taking into account cache invalidation. In particular if
module file is changed on disk then all previous copies
become invalid and any attempt to resolve a function symbol
in such copies will fail. Still any existing instances will
be able to run until explicitly unloaded. This is done for
sake of new upcoming `cmod` module (will be implemented
in next patch).

Need to mention some moments here. Currently we already have
`box.schema.func` interface which is strange designed but can't
be dropped off without breaking backward compatibility. Very
inconvenient moment here is lazy symbols binding, iow we can
create a function but until we execute it the module won't be
read and parsed.

Moreover one can execute the function then overwrite the module
on disk and define an another function from the same module and
run it. This new function will be read from previously cached copy
and one have to run module reload procedure explicitly to update
the module.

Module update on its own is a very fragile procedure: if
there are running functions which are yielding then old
module won't be reload until execution is complete even
if explicit reload is called. And in case if function
return signature is changed (say occasionally) this will
lead to unpredicted results.

In summary: new module_load/unload API with cache
invalidation is incompatible with old `box.schema.func`
thus to keep backward compatibility (until we deprecate
old API) we keep modules in two separate caches: one
for module_load/unload and one for `box.schema.func`.
Later we will keep only new API and get rid of old
code.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/module_cache.c | 179 +++++++++++++++++++++++++++++++++++------
 src/box/module_cache.h |  27 +++++++
 2 files changed, 181 insertions(+), 25 deletions(-)

diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index f9ed6e7d8..aba55bbe3 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -19,11 +19,33 @@
 #include "error.h"
 #include "lua/utils.h"
 #include "libeio/eio.h"
+#include "trivia/util.h"
 
 #include "module_cache.h"
 
-/** Modules name to descriptor hash. */
+/**
+ * Modules names to descriptor hashes. The first one
+ * for modules created with old `box.schema.func`
+ * interface.
+ *
+ * Here is an important moment for backward compatibility.
+ * The `box.schema.func` operations always use cache and
+ * if a module is updated on a storage device or even
+ * no longer present, then lazy symbol resolving is done
+ * via previously loaded copy. To update modules one have
+ * to reload them manually.
+ *
+ * In turn new API implies to use module_load/unload explicit
+ * interface, and when module is re-loaded from cache then
+ * we make a cache validation to be sure the copy on storage
+ * is up to date.
+ *
+ * Due to all this we have to keep two hash tables. Probably
+ * we should deprecate explicit reload at all and require
+ * manual load/unload instead. But later.
+ */
 static struct mh_strnptr_t *box_schema_hash = NULL;
+static struct mh_strnptr_t *mod_hash = NULL;
 
 /**
  * Parsed symbol and package names.
@@ -52,7 +74,7 @@ struct func_name {
 static inline struct mh_strnptr_t *
 hash_tbl(bool is_box_schema)
 {
-	return is_box_schema ? box_schema_hash : NULL;
+	return is_box_schema ? box_schema_hash : mod_hash;
 }
 
 /***
@@ -289,13 +311,9 @@ module_unref(struct module *module)
  * for cases of a function reload.
  */
 static struct module *
-module_load(struct mh_strnptr_t *h, const char *package,
-	    const char *package_end)
+module_new(const char *path, struct mh_strnptr_t *h,
+	   const char *package, const char *package_end)
 {
-	char path[PATH_MAX];
-	if (module_find(package, package_end, path, sizeof(path)) != 0)
-		return NULL;
-
 	int package_len = package_end - package;
 	struct module *module = malloc(sizeof(*module) + package_len + 1);
 	if (module == NULL) {
@@ -334,8 +352,8 @@ module_load(struct mh_strnptr_t *h, const char *package,
 		goto error;
 	}
 
-	struct stat st;
-	if (stat(path, &st) < 0) {
+	struct stat *st = &module->st;
+	if (stat(path, st) < 0) {
 		diag_set(SystemError, "failed to stat() module %s", path);
 		goto error;
 	}
@@ -347,7 +365,7 @@ module_load(struct mh_strnptr_t *h, const char *package,
 	}
 
 	int dest_fd = open(load_name, O_WRONLY | O_CREAT | O_TRUNC,
-			   st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO));
+			   st->st_mode & (S_IRWXU | S_IRWXG | S_IRWXO));
 	if (dest_fd < 0) {
 		diag_set(SystemError, "failed to open file %s for writing ",
 			 load_name);
@@ -355,10 +373,10 @@ module_load(struct mh_strnptr_t *h, const char *package,
 		goto error;
 	}
 
-	off_t ret = eio_sendfile_sync(dest_fd, source_fd, 0, st.st_size);
+	off_t ret = eio_sendfile_sync(dest_fd, source_fd, 0, st->st_size);
 	close(source_fd);
 	close(dest_fd);
-	if (ret != st.st_size) {
+	if (ret != st->st_size) {
 		diag_set(SystemError, "failed to copy DSO %s to %s",
 			 path, load_name);
 		goto error;
@@ -405,6 +423,24 @@ module_sym_load(struct module_sym *mod_sym, bool is_box_schema)
 {
 	assert(mod_sym->addr == NULL);
 
+	/*
+	 * When module is created via new interface and
+	 * cached, it might be updated on disk afterward
+	 * so that symbols might not longer match the loaded
+	 * instance. Thus new resolving on same module requires
+	 * the whole module explicit reload.
+	 */
+	if (!is_box_schema) {
+		struct module *m = mod_sym->module;
+		assert(m != NULL);
+		if (module_is_orphan(m)) {
+			diag_set(ClientError, ER_UNSUPPORTED,
+				 "module updated on disk,",
+				 "needs whole reload");
+			return -1;
+		}
+	}
+
 	struct func_name name;
 	func_split_name(mod_sym->name, &name);
 
@@ -417,7 +453,12 @@ module_sym_load(struct module_sym *mod_sym, bool is_box_schema)
 	struct mh_strnptr_t *h = hash_tbl(is_box_schema);
 	cached = module_cache_find(h, name.package, name.package_end);
 	if (cached == NULL) {
-		module = module_load(h, name.package, name.package_end);
+		char path[PATH_MAX];
+		if (module_find(name.package, name.package_end,
+				path, sizeof(path)) != 0) {
+			return -1;
+		}
+		module = module_new(path, h, name.package, name.package_end);
 		if (module == NULL)
 			return -1;
 		if (module_cache_add(module) != 0) {
@@ -435,7 +476,9 @@ module_sym_load(struct module_sym *mod_sym, bool is_box_schema)
 		return -1;
 	}
 
-	mod_sym->module = module;
+	if (is_box_schema)
+		mod_sym->module = module;
+
 	rlist_add(&module->funcs_list, &mod_sym->item);
 	return 0;
 }
@@ -514,6 +557,72 @@ module_sym_call(struct module_sym *mod_sym, struct port *args,
 	return rc;
 }
 
+struct module *
+module_load(const char *package, const char *package_end)
+{
+	char path[PATH_MAX];
+	if (module_find(package, package_end, path, sizeof(path)) != 0)
+		return NULL;
+
+	struct module *cached, *module;
+	struct mh_strnptr_t *h = hash_tbl(false);
+	cached = module_cache_find(h, package, package_end);
+	if (cached == NULL) {
+		module = module_new(path, h, package, package_end);
+		if (module == NULL)
+			return NULL;
+		if (module_cache_add(module) != 0) {
+			module_unref(module);
+			return NULL;
+		}
+		return module;
+	}
+
+	struct stat st;
+	if (stat(path, &st) != 0) {
+		diag_set(SystemError, "module: stat() module %s", path);
+		return NULL;
+	}
+
+	/*
+	 * When module comes from cache make sure that
+	 * it is not changed on the storage device. The
+	 * test below still can miss update if cpu data
+	 * been manually moved backward and device/inode
+	 * persisted but this is a really rare situation.
+	 */
+	if (cached->st.st_dev == st.st_dev &&
+	    cached->st.st_ino == st.st_ino &&
+	    cached->st.st_size == st.st_size &&
+	    memcmp(&cached->st.st_mtim, &st.st_mtim,
+		   sizeof(st.st_mtim)) == 0) {
+		module_ref(cached);
+		return cached;
+	}
+
+	/*
+	 * Load a new module, update the cache and
+	 * zap old module: every attempt to resolve
+	 * symbols on old instance won't success.
+	 */
+	module = module_new(path, h, package, package_end);
+	if (module == NULL)
+		return NULL;
+	if (module_cache_update(module) != 0) {
+		module_unref(module);
+		return NULL;
+	}
+
+	module_set_orphan(cached);
+	return module;
+}
+
+void
+module_unload(struct module *module)
+{
+	module_unref(module);
+}
+
 int
 module_reload(const char *package, const char *package_end)
 {
@@ -529,7 +638,11 @@ module_reload(const char *package, const char *package_end)
 		return -1;
 	}
 
-	new = module_load(box_schema_hash, package, package_end);
+	char path[PATH_MAX];
+	if (module_find(package, package_end, path, sizeof(path)) != 0)
+		return -1;
+
+	new = module_new(path, box_schema_hash, package, package_end);
 	if (new == NULL)
 		return -1;
 
@@ -606,11 +719,21 @@ module_reload(const char *package, const char *package_end)
 int
 module_init(void)
 {
-	box_schema_hash = mh_strnptr_new();
-	if (box_schema_hash == NULL) {
-		diag_set(OutOfMemory, sizeof(*box_schema_hash),
-			 "malloc", "modules box_schema_hash");
-		return -1;
+	struct mh_strnptr_t **ht[] = {
+		&box_schema_hash,
+		&mod_hash,
+	};
+	for (size_t i = 0; i < lengthof(ht); i++) {
+		*ht[i] = mh_strnptr_new();
+		if (*ht[i] == NULL) {
+			diag_set(OutOfMemory, sizeof(*ht[i]),
+				 "malloc", "modules hash");
+			for (ssize_t j = i - 1; j >= 0; j--) {
+				mh_strnptr_delete(*ht[j]);
+				*ht[j] = NULL;
+			}
+			return -1;
+		}
 	}
 	return 0;
 }
@@ -618,12 +741,18 @@ module_init(void)
 void
 module_free(void)
 {
-	struct mh_strnptr_t *h = box_schema_hash;
-	while (mh_size(h) > 0) {
+	struct mh_strnptr_t **ht[] = {
+		&box_schema_hash,
+		&mod_hash,
+	};
+	for (size_t i = 0; i < lengthof(ht); i++) {
+		struct mh_strnptr_t *h = *ht[i];
+
 		mh_int_t i = mh_first(h);
 		struct module *m = mh_strnptr_node(h, i)->val;
 		module_unref(m);
+
+		mh_strnptr_delete(h);
+		*ht[i] = NULL;
 	}
-	mh_strnptr_delete(box_schema_hash);
-	box_schema_hash = NULL;
 }
diff --git a/src/box/module_cache.h b/src/box/module_cache.h
index 875f2eb3c..ba2cbc7dd 100644
--- a/src/box/module_cache.h
+++ b/src/box/module_cache.h
@@ -6,6 +6,10 @@
 
 #pragma once
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
 #include "small/rlist.h"
 
 #if defined(__cplusplus)
@@ -48,6 +52,10 @@ struct module {
 	 * Count of active references to the module.
 	 */
 	int64_t refs;
+	/**
+	 * Storage stat for identity check.
+	 */
+	struct stat st;
 	/**
 	 * Module's package name.
 	 */
@@ -112,6 +120,25 @@ int
 module_sym_call(struct module_sym *mod_sym, struct port *args,
 		struct port *ret);
 
+/**
+ * Load new module instance.
+ *
+ * @param package shared library path start.
+ * @param package_end shared library path end.
+ *
+ * @return 0 on succes, -1 otherwise, diag is set.
+ */
+struct module *
+module_load(const char *package, const char *package_end);
+
+/**
+ * Unload module instance.
+ *
+ * @param module instance to unload.
+ */
+void
+module_unload(struct module *module);
+
 /**
  * Reload a module and all associated symbols.
  *
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 11/12] box/cmod: implement cmod Lua module
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (9 preceding siblings ...)
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 10/12] module_cache: provide module_load/unload calls Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 12/12] test: box/cfunc -- add cmod test Cyrill Gorcunov via Tarantool-patches
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

Currently to run "C" function from some external module one
have to register it first in "_func" system space. This is
a problem if node is in read-only mode (replica).

Still people would like to have a way to run such functions
even in ro mode. For this sake we implement "cmod" lua module.

Fixes #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

@TarantoolBot document
Title: cmod module

Overview
========

`cmod` module provides a way to create, delete and execute
`C` procedures from shared libraries. Unlike `box.schema.func`
methods the functions created with `cmod` help are not persistent
and live purely in memory. Once a node get turned off they are
vanished. An initial purpose for them is to execute them on
nodes which are running in read-only mode.

Module functions
================

`require('cmod').load(path) -> obj | error`
-------------------------------------------

Loads a module from `path` and return an object instance
associate with the module, otherwise an error is thrown.

The `path` should not end up with shared library extension
(such as `.so`), only a file name shall be there.

Possible errors:

- IllegalParams: module path is either not supplied
  or not a string.
- SystemError: unable to open a module due to a system error.
- ClientError: a module does not exist.
- OutOfMemory: unable to allocate a module.

Example:

``` Lua
-- Without error handling
m = require('cmod').load('path/to/library)

-- With error handling
m, err = pcall(require('cmod').load, 'path/to/library')
if err ~= nil then
    print(err)
end
```

`module:unload() -> true | error`
---------------------------------

Unloads a module. Returns `true` on success, otherwise an error
is thrown. Once the module is unloaded one can't load new
functions from this module instance.

Possible errors:

- IllegalParams: a module is not supplied.
- IllegalParams: a module is already unloaded.

Example:

``` Lua
m = require('cmod').load('path/to/library')
--
-- do something with module
--
m:unload()
```

If there are functions from this module referenced somewhere
in other places of Lua code they still can be executed because
the module continue sitting in memory until the last reference
to it is closed.

If the module become a target to the Lua's garbage collector
then unload is called implicitly.

module:load(name) -> obj | error`
---------------------------------

Loads a new function with name `name` from the previously
loaded `module` and return a callable object instance
associated with the function. On failure an error is thrown.

Possible errors:
 - IllegalParams: function name is either not supplied
   or not a string.
 - IllegalParams: attempt to load a function but module
   has been unloaded already.
 - ClientError: no such function in the module.
 - ClientError: module has been updated on disk and not
   reloaded.
 - OutOfMemory: unable to allocate a function.

Example:

``` Lua
-- Load a module if not been loaded yet.
m = require('cmod').load('path/to/library')
-- Load a function with the `foo` name from the module `m`.
func = m:load('foo')
```

In case if there is no need for further loading of other
functions from the same module then the module might be
unloaded immediately.

``` Lua
m = require('cmod').load('path/to/library')
func = m:load('foo')
m:unload()
```

`function:unload() -> true | error`
-----------------------------------

Unloads a function. Returns `true` on success, otherwise
an error is thrown.

Possible errors:
 - IllegalParams: function name is either not supplied
   or not a string.
 - IllegalParams: the function does not exist.

Example:

``` Lua
m = require('cmod').load('path/to/library')
func = m:load('foo')
func:unload()
```

If the function become a target to the Lua's garbage collector
then unload is called implicitly.

Executing a loaded function
===========================

Once function is loaded it can be executed as an ordinary Lua call.
Lets consider the following example. We have a `C` function which
takes two numbers and returns their sum.

``` C
int
cfunc_sum(box_function_ctx_t *ctx, const char *args, const char *args_end)
{
	uint32_t arg_count = mp_decode_array(&args);
	if (arg_count != 2) {
		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
				     "invalid argument count");
	}
	uint64_t a = mp_decode_uint(&args);
	uint64_t b = mp_decode_uint(&args);

	char res[16];
	char *end = mp_encode_uint(res, a + b);
	box_return_mp(ctx, res, end);
	return 0;
}
```

The name of the function is `cfunc_sum` and the function is built into
`cfunc.so` shared library.

First we should load it as

``` Lua
m = require('cmod').load('cfunc')
cfunc_sum = m:load('cfunc_sum')
```

Once successfully loaded we can execute it. Note that unlike regular
Lua functions the context of `C` functions is different. They never
thrown an exception but return `true|nil, res` form where first value
set to `nil` in case of error condition and `res` carries an error
description.

Lets call the `cfunc_sum` with wrong number of arguments

``` Lua
local ok, res = cfunc_sum()
if not ok then
    print(res)
end
```

We will the `"invalid argument count"` message in output.
The error message has been set by the `box_error_set` in `C`
code above.

On success the first returned value set to `true` and `res` represent
function execution result.

``` Lua
local ok, res = cfunc_sum(1, 2)
assert(ok);
print(res)
```

We will see the number `3` in output.

The functions might return multple results. In this case the first
returned value is `true` and the rest are ones provided by function.

Module and function caches
==========================

Loading a module is relatively slow procedure because operating
system needs to read the library, resolve its symbols and etc.
Thus to speedup this procedure if the module is loaded for a first
time we put it into an internal cache. If module is sitting in
the cache already and new request to load comes in we simply
reuse a previous copy immediately. Same applies to functions:
while symbol lookup is a way faster than loading module from
disk it is not completely cheap, thus we cache functions as
well. Functions entries in cache are identified by a module
path and function name.

Still the following situation is possible: the module is loaded but
user does recompile it and overwrite on a storage device. Thus
cache content no longer matches the shared library on the disk.

To handle this situation we use that named cache invalidation procedure:
on every attempt to load the same module again we test low level file
attributes (such as storage device number, inode, size and modification
time) and if they are differ from ones kept by the cache then old module
marked as orphan, which means no new function lookup will be allowed.
All previously loaded function from this orphan module are fully
functional, until explicitly unloaded or collected by Lua's garbage
collector.

In case if there is a strong need to reload a module then better
to unload all functions and the module explicitly, load it from the
scratch and load all functions again. This will prevent from
unexpected errors.
---
 src/box/CMakeLists.txt |   1 +
 src/box/lua/cmod.c     | 600 +++++++++++++++++++++++++++++++++++++++++
 src/box/lua/cmod.h     |  24 ++
 src/box/lua/init.c     |   2 +
 4 files changed, 627 insertions(+)
 create mode 100644 src/box/lua/cmod.c
 create mode 100644 src/box/lua/cmod.h

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 339e2c8a9..feba5a037 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -195,6 +195,7 @@ add_library(box STATIC
     lua/init.c
     lua/call.c
     lua/cfg.cc
+    lua/cmod.c
     lua/console.c
     lua/serialize_lua.c
     lua/tuple.c
diff --git a/src/box/lua/cmod.c b/src/box/lua/cmod.c
new file mode 100644
index 000000000..91450ddff
--- /dev/null
+++ b/src/box/lua/cmod.c
@@ -0,0 +1,600 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
+ */
+
+#include <string.h>
+#include <lua.h>
+
+#include "assoc.h"
+#include "diag.h"
+
+#include "box/module_cache.h"
+#include "box/error.h"
+#include "box/port.h"
+#include "tt_static.h"
+
+#include "trivia/util.h"
+#include "lua/utils.h"
+
+/**
+ * Function descriptor.
+ */
+struct cmod_func {
+	/**
+	 * Symbol descriptor for the function in
+	 * an associated module.
+	 */
+	struct module_sym mod_sym;
+	/**
+	 * Length of @a name member.
+	 */
+	size_t len;
+	/**
+	 * Count of active references to the function.
+	 */
+	int64_t refs;
+	/**
+	 * Module path with function name separated
+	 * by a point, like "module.func".
+	 */
+	char name[0];
+};
+
+/** A type to find a module from an object. */
+static const char *cmod_module_uname = "cmod_module_uname";
+
+/** A type to find a function from an object. */
+static const char *cmod_func_uname = "cmod_func_uname";
+
+/** Get data associated with an object. */
+static void *
+get_udata(struct lua_State *L, const char *uname)
+{
+	void **pptr = luaL_testudata(L, 1, 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)
+{
+	void **pptr = luaL_testudata(L, 1, uname);
+	assert(pptr != NULL);
+	*pptr = ptr;
+}
+
+/** Setup a new data and associate it with an object. */
+static void
+new_udata(struct lua_State *L, const char *uname, void *ptr)
+{
+	*(void **)lua_newuserdata(L, sizeof(void *)) = ptr;
+	luaL_getmetatable(L, uname);
+	lua_setmetatable(L, -2);
+}
+
+/**
+ * Function name to cmod_func hash. The name includes
+ * module package path without file extension.
+ */
+static struct mh_strnptr_t *func_hash = NULL;
+
+/**
+ * Find function in cmod_func hash.
+ */
+struct cmod_func *
+func_cache_find(const char *name, size_t name_len)
+{
+	mh_int_t e = mh_strnptr_find_inp(func_hash, name, name_len);
+	if (e == mh_end(func_hash))
+		return NULL;
+	return mh_strnptr_node(func_hash, e)->val;
+}
+
+/**
+ * Delete a function instance from cmod_func hash.
+ */
+static void
+func_cache_del(struct cmod_func *cf)
+{
+	assert(cf->refs == 0);
+
+	mh_int_t e = mh_strnptr_find_inp(func_hash, cf->name, cf->len);
+	assert(e != mh_end(func_hash));
+	mh_strnptr_del(func_hash, e, NULL);
+}
+
+/**
+ * Add a function instance into cmod_func hash.
+ */
+static int
+func_cache_add(struct cmod_func *cf)
+{
+	const struct mh_strnptr_node_t nd = {
+		.str	= cf->name,
+		.len	= cf->len,
+		.hash	= mh_strn_hash(cf->name, cf->len),
+		.val	= cf,
+	};
+
+	mh_int_t e = mh_strnptr_put(func_hash, &nd, NULL, NULL);
+	if (e == mh_end(func_hash)) {
+		diag_set(OutOfMemory, sizeof(nd),
+			 "malloc", "cmod_func node");
+		return -1;
+	}
+	return 0;
+}
+
+/**
+ * Unload a symbol and free a function instance.
+ */
+static void
+func_delete(struct cmod_func *cf)
+{
+	assert(cf->refs == 0);
+	module_sym_unload(&cf->mod_sym);
+	TRASH(cf);
+	free(cf);
+}
+
+/**
+ * Increase reference to a function.
+ */
+static void
+func_ref(struct cmod_func *cf)
+{
+	assert(cf->refs >= 0);
+	cf->refs++;
+}
+
+/**
+ * Decrease a function reference and delete it if last one.
+ */
+static void
+func_unref(struct cmod_func *cf)
+{
+	assert(cf->refs > 0);
+	if (cf->refs-- == 1) {
+		func_cache_del(cf);
+		func_delete(cf);
+	}
+}
+
+/**
+ * Allocate a new function instance and resolve a symbol address.
+ *
+ * @param module module the function load from.
+ * @param name package path and a function name, ie "module.foo".
+ * @param len length of @a name.
+ *
+ * @returns function instance on success, NULL otherwise setting diag area.
+ */
+static struct cmod_func *
+func_new(struct module *m, const char *name, size_t len)
+{
+	size_t size = sizeof(struct cmod_func) + len + 1;
+	struct cmod_func *cf = malloc(size);
+	if (cf == NULL) {
+		diag_set(OutOfMemory, size, "malloc", "cf");
+		return NULL;
+	}
+
+	cf->mod_sym.addr	= NULL;
+	cf->mod_sym.module	= m;
+	cf->mod_sym.name	= cf->name;
+	cf->len			= len;
+	cf->refs		= 0;
+
+	memcpy(cf->name, name, len);
+	cf->name[len] = '\0';
+
+	if (module_sym_load(&cf->mod_sym, false) != 0) {
+		cf->mod_sym.module = NULL;
+		func_delete(cf);
+		return NULL;
+	}
+
+	func_ref(cf);
+	return cf;
+}
+
+/**
+ * Load a new function.
+ *
+ * This function takes a function name from the caller
+ * stack @a L and creates a new function object. If
+ * the function is already loaded we simply return
+ * a reference to existing one.
+ *
+ * Possible errors:
+ *
+ * - IllegalParams: function name is either not supplied
+ *   or not a string.
+ * - IllegalParams: function references limit exceeded.
+ * - OutOfMemory: unable to allocate a function.
+ * - ClientError: no such function in the module.
+ * - ClientError: module has been updated on disk and not
+ *   yet unloaded and loaded back.
+ *
+ * @returns function object on success or throwns an error.
+ */
+static int
+lcmod_func_load(struct lua_State *L)
+{
+	const char *method = "function = module:load";
+
+	if (lua_gettop(L) != 2 || !lua_isstring(L, 2)) {
+		const char *fmt =
+			"Expects %s(\'name\') but no name passed";
+		diag_set(IllegalParams, fmt, method);
+		return luaT_error(L);
+	}
+
+	struct module *m = get_udata(L, cmod_module_uname);
+	if (m == NULL) {
+		const char *fmt =
+			"Expects %s(\'name\') but not module object passed";
+		diag_set(IllegalParams, fmt, method);
+		return luaT_error(L);
+	}
+
+	const char *func_name = lua_tostring(L, 2);
+	const char *name = tt_sprintf("%s.%s", m->package, func_name);
+	size_t len = strlen(name);
+
+	/*
+	 * We try to reuse already allocated function in
+	 * case if someone is loading same function twise.
+	 * This will save memory and eliminates redundant
+	 * symbol address resolving.
+	 */
+	struct cmod_func *cf = func_cache_find(name, len);
+	if (cf == NULL) {
+		cf = func_new(m, name, len);
+		if (cf == NULL)
+			return luaT_error(L);
+		if (func_cache_add(cf) != 0) {
+			func_unref(cf);
+			return luaT_error(L);
+		}
+	} else {
+		func_ref(cf);
+	}
+
+	new_udata(L, cmod_func_uname, cf);
+	return 1;
+}
+
+/**
+ * Unload a function.
+ *
+ * This function takes a function object from
+ * the caller stack @a L and unloads it.
+ *
+ * Possible errors:
+ *
+ * - IllegalParams: function is not supplied.
+ * - IllegalParams: the function does not exist.
+ *
+ * @returns true on success or throwns an error.
+ */
+static int
+lcmod_func_unload(struct lua_State *L)
+{
+	if (lua_gettop(L) != 1) {
+		diag_set(IllegalParams, "Expects function:unload()");
+		return luaT_error(L);
+	}
+
+	struct cmod_func *cf = get_udata(L, cmod_func_uname);
+	if (cf == NULL) {
+		diag_set(IllegalParams, "The function is unloaded");
+		return luaT_error(L);
+	}
+
+	set_udata(L, cmod_func_uname, NULL);
+	func_unref(cf);
+
+	lua_pushboolean(L, true);
+	return 1;
+}
+
+/**
+ * Load a new module.
+ *
+ * This function takes a module patch from the caller
+ * stack @a L and creates a new module object.
+ *
+ * Possible errors:
+ *
+ * - IllegalParams: module path is either not supplied
+ *   or not a string.
+ * - SystemError: unable to open a module due to a system error.
+ * - ClientError: a module does not exist.
+ * - OutOfMemory: unable to allocate a module.
+ *
+ * @returns module object on success or throws an error.
+ */
+static int
+lcmod_module_load(struct lua_State *L)
+{
+	if (lua_gettop(L) != 1 || !lua_isstring(L, 1)) {
+		diag_set(IllegalParams, "Expects cmod.load(\'name\') "
+			 "but no name passed");
+		return luaT_error(L);
+	}
+
+	size_t name_len;
+	const char *name = lua_tolstring(L, 1, &name_len);
+
+	struct module *module = module_load(name, &name[name_len]);
+	if (module == NULL)
+		return luaT_error(L);
+
+	new_udata(L, cmod_module_uname, module);
+	return 1;
+}
+
+/**
+ * Unload a module.
+ *
+ * This function takes a module object from
+ * the caller stack @a L and unloads it.
+ *
+ * If there are some active functions left then
+ * module won't be freed internally until last function
+ * from this module is unloaded, this is guaranteed by
+ * module_cache engine.
+ *
+ * Possible errors:
+ *
+ * - IllegalParams: module is not supplied.
+ * - IllegalParams: module already unloaded.
+ *
+ * @returns true on success or throws an error.
+ */
+static int
+lcmod_module_unload(struct lua_State *L)
+{
+	if (lua_gettop(L) != 1) {
+		diag_set(IllegalParams, "Expects module:unload()");
+		return luaT_error(L);
+	}
+
+	struct module *m = get_udata(L, cmod_module_uname);
+	if (m == NULL) {
+		diag_set(IllegalParams, "The module is already unloaded");
+		return luaT_error(L);
+	}
+	set_udata(L, cmod_module_uname, NULL);
+	module_unload(m);
+	lua_pushboolean(L, true);
+	return 1;
+}
+
+/**
+ * Handle __index request for a module object.
+ */
+static int
+lcmod_module_index(struct lua_State *L)
+{
+	/*
+	 * Instead of showing userdata pointer
+	 * lets provide a serialized value.
+	 */
+	lua_getmetatable(L, 1);
+	lua_pushvalue(L, 2);
+	lua_rawget(L, -2);
+	if (!lua_isnil(L, -1))
+		return 1;
+
+	struct module *m = get_udata(L, cmod_module_uname);
+	if (m == NULL) {
+		lua_pushnil(L);
+		return 1;
+	}
+
+	const char *key = lua_tostring(L, 2);
+	if (key == NULL || lua_type(L, 2) != LUA_TSTRING) {
+		diag_set(IllegalParams,
+			 "Bad params, use __index(obj, <string>)");
+		return luaT_error(L);
+	}
+
+	if (strcmp(key, "path") == 0) {
+		lua_pushstring(L, m->package);
+		return 1;
+	}
+
+	return 0;
+}
+
+/**
+ * Module handle representation for REPL (console).
+ */
+static int
+lcmod_module_serialize(struct lua_State *L)
+{
+	struct module *m = get_udata(L, cmod_module_uname);
+	if (m == NULL) {
+		lua_pushnil(L);
+		return 1;
+	}
+
+	lua_createtable(L, 0, 0);
+	lua_pushstring(L, m->package);
+	lua_setfield(L, -2, "path");
+
+	return 1;
+}
+
+/**
+ * Collect a module handle.
+ */
+static int
+lcmod_module_gc(struct lua_State *L)
+{
+	struct module *m = get_udata(L, cmod_module_uname);
+	if (m != NULL) {
+		set_udata(L, cmod_module_uname, NULL);
+		module_unload(m);
+	}
+	return 0;
+}
+
+/**
+ * Function handle representation for REPL (console).
+ */
+static int
+lcmod_func_serialize(struct lua_State *L)
+{
+	struct cmod_func *cf = get_udata(L, cmod_func_uname);
+	if (cf == NULL) {
+		lua_pushnil(L);
+		return 1;
+	}
+
+	lua_createtable(L, 0, 1);
+	lua_pushstring(L, cf->name);
+	lua_setfield(L, -2, "name");
+
+	return 1;
+}
+
+/**
+ * Handle __index request for a function object.
+ */
+static int
+lcmod_func_index(struct lua_State *L)
+{
+	/*
+	 * Instead of showing userdata pointer
+	 * lets provide a serialized value.
+	 */
+	lua_getmetatable(L, 1);
+	lua_pushvalue(L, 2);
+	lua_rawget(L, -2);
+	if (!lua_isnil(L, -1))
+		return 1;
+
+	struct cmod_func *cf = get_udata(L, cmod_func_uname);
+	if (cf == NULL) {
+		lua_pushnil(L);
+		return 1;
+	}
+
+	const char *key = lua_tostring(L, 2);
+	if (key == NULL || lua_type(L, 2) != LUA_TSTRING) {
+		diag_set(IllegalParams,
+			 "Bad params, use __index(obj, <string>)");
+		return luaT_error(L);
+	}
+
+	if (strcmp(key, "name") == 0) {
+		lua_pushstring(L, cf->name);
+		return 1;
+	}
+
+	return 0;
+}
+
+/**
+ * Collect function handle if there is no active loads left.
+ */
+static int
+lcmod_func_gc(struct lua_State *L)
+{
+	struct cmod_func *cf = get_udata(L, cmod_func_uname);
+	if (cf != NULL) {
+		set_udata(L, cmod_func_uname, NULL);
+		func_unref(cf);
+	}
+	return 0;
+}
+
+/**
+ * Call a function by its name from the Lua code.
+ */
+static int
+lcmod_func_call(struct lua_State *L)
+{
+	struct cmod_func *cf = get_udata(L, cmod_func_uname);
+	if (cf == NULL) {
+		diag_set(IllegalParams, "The function is unloaded");
+		return luaT_error(L);
+	}
+
+	/*
+	 * FIXME: We should get rid of luaT_newthread but this
+	 * requires serious modifications. In particular
+	 * port_lua_do_dump uses tarantool_L reference and
+	 * coro_ref must be valid as well.
+	 */
+	lua_State *args_L = luaT_newthread(tarantool_L);
+	if (args_L == NULL)
+		return luaT_error(L);
+
+	int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
+	lua_xmove(L, args_L, lua_gettop(L) - 1);
+
+	struct port args;
+	port_lua_create(&args, args_L);
+	((struct port_lua *)&args)->ref = coro_ref;
+
+	struct port ret;
+	if (module_sym_call(&cf->mod_sym, &args, &ret) != 0) {
+		port_destroy(&args);
+		return luaT_error(L);
+	}
+
+	int top = lua_gettop(L);
+	lua_pushboolean(L, true);
+	port_dump_lua(&ret, L, true);
+	int cnt = lua_gettop(L) - top;
+
+	port_destroy(&ret);
+	port_destroy(&args);
+
+	return cnt;
+}
+
+/**
+ * Initialize cmod module.
+ */
+void
+box_lua_cmod_init(struct lua_State *L)
+{
+	func_hash = mh_strnptr_new();
+	if (func_hash == NULL) {
+		panic("Can't allocate cmod hash table");
+	}
+
+	static const struct luaL_Reg top_methods[] = {
+		{ "load",		lcmod_module_load	},
+		{ NULL, NULL },
+	};
+	luaL_register_module(L, "cmod", top_methods);
+	lua_pop(L, 1);
+
+	static const struct luaL_Reg module_methods[] = {
+		{ "load",		lcmod_func_load		},
+		{ "unload",		lcmod_module_unload	},
+		{ "__index",		lcmod_module_index	},
+		{ "__serialize",	lcmod_module_serialize	},
+		{ "__gc",		lcmod_module_gc		},
+		{ NULL, NULL },
+	};
+	luaL_register_type(L, cmod_module_uname, module_methods);
+
+	static const struct luaL_Reg func_methods[] = {
+		{ "unload",		lcmod_func_unload	},
+		{ "__index",		lcmod_func_index	},
+		{ "__serialize",	lcmod_func_serialize	},
+		{ "__call",		lcmod_func_call		},
+		{ "__gc",		lcmod_func_gc		},
+		{ NULL, NULL },
+	};
+	luaL_register_type(L, cmod_func_uname, func_methods);
+}
diff --git a/src/box/lua/cmod.h b/src/box/lua/cmod.h
new file mode 100644
index 000000000..f0ea2d34d
--- /dev/null
+++ b/src/box/lua/cmod.h
@@ -0,0 +1,24 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
+ */
+
+#pragma once
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+struct lua_State;
+
+/**
+ * Initialize cmod Lua module.
+ *
+ * @param L Lua state where to register the cmod module.
+ */
+void
+box_lua_cmod_init(struct lua_State *L);
+#if defined(__cplusplus)
+}
+#endif /* defined(__plusplus) */
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index fbcdfb20b..bad2b7ca9 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -60,6 +60,7 @@
 #include "box/lua/cfg.h"
 #include "box/lua/xlog.h"
 #include "box/lua/console.h"
+#include "box/lua/cmod.h"
 #include "box/lua/tuple.h"
 #include "box/lua/execute.h"
 #include "box/lua/key_def.h"
@@ -465,6 +466,7 @@ box_lua_init(struct lua_State *L)
 	box_lua_tuple_init(L);
 	box_lua_call_init(L);
 	box_lua_cfg_init(L);
+	box_lua_cmod_init(L);
 	box_lua_slab_init(L);
 	box_lua_index_init(L);
 	box_lua_space_init(L);
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v14 12/12] test: box/cfunc -- add cmod test
  2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
                   ` (10 preceding siblings ...)
  2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 11/12] box/cmod: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
@ 2021-02-02 22:12 ` Cyrill Gorcunov via Tarantool-patches
  11 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-02-02 22:12 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

Note that the test is disabled for a while since we
need to update test-run first like

 | diff --git a/pretest_clean.lua b/pretest_clean.lua
 | index 9b5ac9d..b0280c4 100644
 | --- a/pretest_clean.lua
 | +++ b/pretest_clean.lua
 | @@ -272,6 +272,7 @@ local function clean()
 |          package = true,
 |          pickle = true,
 |          popen = true,
 | +        cmod = true,
 |          pwd = true,
 |          socket = true,
 |          strict = true,

ie need to enable cmod module.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 test/box/CMakeLists.txt |   2 +
 test/box/cfunc1.c       |  58 ++++++++
 test/box/cfunc2.c       | 137 ++++++++++++++++++
 test/box/cmod.result    | 312 ++++++++++++++++++++++++++++++++++++++++
 test/box/cmod.test.lua  | 123 ++++++++++++++++
 test/box/suite.ini      |   2 +-
 6 files changed, 633 insertions(+), 1 deletion(-)
 create mode 100644 test/box/cfunc1.c
 create mode 100644 test/box/cfunc2.c
 create mode 100644 test/box/cmod.result
 create mode 100644 test/box/cmod.test.lua

diff --git a/test/box/CMakeLists.txt b/test/box/CMakeLists.txt
index 06bfbbe9d..2afbeadc3 100644
--- a/test/box/CMakeLists.txt
+++ b/test/box/CMakeLists.txt
@@ -3,3 +3,5 @@ build_module(function1 function1.c)
 build_module(reload1 reload1.c)
 build_module(reload2 reload2.c)
 build_module(tuple_bench tuple_bench.c)
+build_module(cfunc1 cfunc1.c)
+build_module(cfunc2 cfunc2.c)
diff --git a/test/box/cfunc1.c b/test/box/cfunc1.c
new file mode 100644
index 000000000..8f6d3990c
--- /dev/null
+++ b/test/box/cfunc1.c
@@ -0,0 +1,58 @@
+#include <stdio.h>
+#include <stdbool.h>
+#include <msgpuck.h>
+
+#include "module.h"
+
+/*
+ * Before the reload functions are just declared
+ * and simply exit with zero.
+ *
+ * After the module reload we should provide real
+ * functionality.
+ */
+
+int
+cfunc_nop(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	(void)ctx;
+	(void)args;
+	(void)args_end;
+	return 0;
+}
+
+int
+cfunc_fetch_evens(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	(void)ctx;
+	(void)args;
+	(void)args_end;
+	return 0;
+}
+
+int
+cfunc_multireturn(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	(void)ctx;
+	(void)args;
+	(void)args_end;
+	return 0;
+}
+
+int
+cfunc_args(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	(void)ctx;
+	(void)args;
+	(void)args_end;
+	return 0;
+}
+
+int
+cfunc_sum(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	(void)ctx;
+	(void)args;
+	(void)args_end;
+	return 0;
+}
diff --git a/test/box/cfunc2.c b/test/box/cfunc2.c
new file mode 100644
index 000000000..a4a871359
--- /dev/null
+++ b/test/box/cfunc2.c
@@ -0,0 +1,137 @@
+#include <stdio.h>
+#include <stdbool.h>
+#include <msgpuck.h>
+
+#include "module.h"
+
+/*
+ * Just make sure we've been called.
+ */
+int
+cfunc_nop(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	(void)ctx;
+	(void)args;
+	(void)args_end;
+	return 0;
+}
+
+/*
+ * Fetch first N even numbers (just to make sure the order of
+ * arguments is not screwed).
+ */
+int
+cfunc_fetch_evens(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	int arg_count = mp_decode_array(&args);
+	if (arg_count != 1) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+				     "invalid argument count");
+	}
+	int field_count = mp_decode_array(&args);
+	if (field_count < 1) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+				     "invalid array size");
+	}
+
+	/*
+	 * We expect even numbers sequence here. The idea is
+	 * to test invalid data an issue an error from inside
+	 * of C function.
+	 */
+	for (int i = 1; i <= field_count; i++) {
+		int val = mp_decode_uint(&args);
+		int needed = 2 * i;
+		if (val != needed) {
+			char res[128];
+			snprintf(res, sizeof(res), "%s %d != %d",
+				 "invalid argument", val, needed);
+			return box_error_set(__FILE__, __LINE__,
+					     ER_PROC_C, "%s", res);
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Return one element array twice.
+ */
+int
+cfunc_multireturn(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	char tuple_buf[512];
+	char *d = tuple_buf;
+	d = mp_encode_array(d, 1);
+	d = mp_encode_uint(d, 1);
+	assert(d <= tuple_buf + sizeof(tuple_buf));
+
+	box_tuple_format_t *fmt = box_tuple_format_default();
+	box_tuple_t *tuple_a = box_tuple_new(fmt, tuple_buf, d);
+	if (tuple_a == NULL)
+		return -1;
+	int rc = box_return_tuple(ctx, tuple_a);
+	if (rc == 0)
+		return box_return_tuple(ctx, tuple_a);
+	return rc;
+}
+
+/*
+ * Encode int + string pair back.
+ */
+int
+cfunc_args(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	uint32_t arg_count = mp_decode_array(&args);
+	if (arg_count != 2) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+				     "invalid argument count");
+	}
+
+	if (mp_typeof(*args) != MP_UINT) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+				     "tuple field must be uint");
+	}
+	uint32_t num = mp_decode_uint(&args);
+
+	if (mp_typeof(*args) != MP_STR) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+				     "tuple field must be string");
+	}
+	const char *str = args;
+	uint32_t len = mp_decode_strl(&str);
+
+	char tuple_buf[512];
+	char *d = tuple_buf;
+	d = mp_encode_array(d, 2);
+	d = mp_encode_uint(d, num);
+	d = mp_encode_str(d, str, len);
+	assert(d <= tuple_buf + sizeof(tuple_buf));
+
+	box_tuple_format_t *fmt = box_tuple_format_default();
+	box_tuple_t *tuple = box_tuple_new(fmt, tuple_buf, d);
+	if (tuple == NULL)
+		return -1;
+
+	return box_return_tuple(ctx, tuple);
+}
+
+/*
+ * Sum two integers.
+ */
+int
+cfunc_sum(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	uint32_t arg_count = mp_decode_array(&args);
+	if (arg_count != 2) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+				     "invalid argument count");
+	}
+	uint64_t a = mp_decode_uint(&args);
+	uint64_t b = mp_decode_uint(&args);
+
+	char res[16];
+	char *end = mp_encode_uint(res, a + b);
+	box_return_mp(ctx, res, end);
+	return 0;
+}
diff --git a/test/box/cmod.result b/test/box/cmod.result
new file mode 100644
index 000000000..625a22bd0
--- /dev/null
+++ b/test/box/cmod.result
@@ -0,0 +1,312 @@
+-- test-run result file version 2
+--
+-- gh-4642: New cmod module to be able to
+-- run C stored functions on read only nodes
+-- without requirement to register them with
+-- box.schema.func help.
+--
+build_path = os.getenv("BUILDDIR")
+ | ---
+ | ...
+package.cpath = build_path..'/test/box/?.so;'..build_path..'/test/box/?.dylib;'..package.cpath
+ | ---
+ | ...
+
+cmod = require('cmod')
+ | ---
+ | ...
+fio = require('fio')
+ | ---
+ | ...
+
+ext = (jit.os == "OSX" and "dylib" or "so")
+ | ---
+ | ...
+
+cfunc_path = fio.pathjoin(build_path, "test/box/cfunc.") .. ext
+ | ---
+ | ...
+cfunc1_path = fio.pathjoin(build_path, "test/box/cfunc1.") .. ext
+ | ---
+ | ...
+cfunc2_path = fio.pathjoin(build_path, "test/box/cfunc2.") .. ext
+ | ---
+ | ...
+
+_ = pcall(fio.unlink(cfunc_path))
+ | ---
+ | ...
+fio.symlink(cfunc1_path, cfunc_path)
+ | ---
+ | - true
+ | ...
+
+_, err = pcall(cmod.load, 'non-such-module')
+ | ---
+ | ...
+assert(err ~= nil)
+ | ---
+ | - true
+ | ...
+
+-- They all are sitting in cfunc.so. The "nop" module
+-- contains functions which are simply return nothing.
+old_module = cmod.load('cfunc')
+ | ---
+ | ...
+old_cfunc_nop = old_module:load('cfunc_nop')
+ | ---
+ | ...
+old_cfunc_fetch_evens = old_module:load('cfunc_fetch_evens')
+ | ---
+ | ...
+old_cfunc_multireturn = old_module:load('cfunc_multireturn')
+ | ---
+ | ...
+old_cfunc_args = old_module:load('cfunc_args')
+ | ---
+ | ...
+old_cfunc_sum = old_module:load('cfunc_sum')
+ | ---
+ | ...
+-- Test for error on nonexisting function.
+_, err = pcall(old_module.load, old_module, 'no-such-func')
+ | ---
+ | ...
+assert(err ~= nil)
+ | ---
+ | - true
+ | ...
+
+-- Make sure they all are callable.
+old_cfunc_nop()
+ | ---
+ | - true
+ | ...
+old_cfunc_fetch_evens()
+ | ---
+ | - true
+ | ...
+old_cfunc_multireturn()
+ | ---
+ | - true
+ | ...
+old_cfunc_args()
+ | ---
+ | - true
+ | ...
+old_cfunc_sum()
+ | ---
+ | - true
+ | ...
+
+-- Unload the module but keep old functions alive, so
+-- they keep reference to NOP module internally
+-- and still callable.
+old_module:unload()
+ | ---
+ | - true
+ | ...
+old_cfunc_nop()
+ | ---
+ | - true
+ | ...
+old_cfunc_fetch_evens()
+ | ---
+ | - true
+ | ...
+old_cfunc_multireturn()
+ | ---
+ | - true
+ | ...
+old_cfunc_args()
+ | ---
+ | - true
+ | ...
+old_cfunc_sum()
+ | ---
+ | - true
+ | ...
+
+-- The module is unloaded I should not be able
+-- to load new shared library.
+old_module:load('cfunc')
+ | ---
+ | - error: Expects function = module:load('name') but not module object passed
+ | ...
+-- Neither I should be able to unload module twise.
+old_module:unload()
+ | ---
+ | - error: The module is already unloaded
+ | ...
+
+-- Clean old functions.
+old_cfunc_nop:unload()
+ | ---
+ | - true
+ | ...
+old_cfunc_fetch_evens:unload()
+ | ---
+ | - true
+ | ...
+old_cfunc_multireturn:unload()
+ | ---
+ | - true
+ | ...
+old_cfunc_args:unload()
+ | ---
+ | - true
+ | ...
+old_cfunc_sum:unload()
+ | ---
+ | - true
+ | ...
+
+-- And reload old module again.
+old_module = cmod.load('cfunc')
+ | ---
+ | ...
+old_cfunc_nop = old_module:load('cfunc_nop')
+ | ---
+ | ...
+
+-- Overwrite module with new contents.
+_ = pcall(fio.unlink(cfunc_path))
+ | ---
+ | ...
+fio.symlink(cfunc2_path, cfunc_path)
+ | ---
+ | - true
+ | ...
+
+-- Now try to load a function from old module,
+-- we should get chache invalidation, old module
+-- get orphaned and lookup for functions from
+-- old module should be prohibited.
+new_module = cmod.load('cfunc')
+ | ---
+ | ...
+new_cfunc_nop = old_module:load('cfunc_nop')
+ | ---
+ | ...
+old_module:load('cfunc_fetch_evens')
+ | ---
+ | - error: module updated on disk, does not support needs whole reload
+ | ...
+-- Previously loaded functions from old module should
+-- sit in functions cache and simply increment internal
+-- references.
+old_cfunc_nop_cached = old_module:load('cfunc_nop')
+ | ---
+ | ...
+old_module:unload()
+ | ---
+ | - true
+ | ...
+
+-- New module is loaded, lets lookup for updated symbols.
+new_cfunc_nop = new_module:load('cfunc_nop')
+ | ---
+ | ...
+new_cfunc_fetch_evens = new_module:load('cfunc_fetch_evens')
+ | ---
+ | ...
+new_cfunc_multireturn = new_module:load('cfunc_multireturn')
+ | ---
+ | ...
+new_cfunc_args = new_module:load('cfunc_args')
+ | ---
+ | ...
+new_cfunc_sum = new_module:load('cfunc_sum')
+ | ---
+ | ...
+
+-- Call old functions.
+old_cfunc_nop()
+ | ---
+ | - true
+ | ...
+old_cfunc_nop_cached()
+ | ---
+ | - true
+ | ...
+
+-- And newly loaded ones.
+new_cfunc_nop()
+ | ---
+ | - true
+ | ...
+new_cfunc_multireturn()
+ | ---
+ | - true
+ | - [1]
+ | - [1]
+ | ...
+new_cfunc_fetch_evens({2,4,6})
+ | ---
+ | - true
+ | ...
+new_cfunc_fetch_evens({1,2,3})  -- error
+ | ---
+ | - error: invalid argument 1 != 2
+ | ...
+new_cfunc_args(1, "hello")
+ | ---
+ | - true
+ | - [1, 'hello']
+ | ...
+new_cfunc_sum(1) -- error
+ | ---
+ | - error: invalid argument count
+ | ...
+new_cfunc_sum(1,2)
+ | ---
+ | - true
+ | - 3
+ | ...
+
+-- Cleanup old module's functions.
+old_cfunc_nop:unload()
+ | ---
+ | - true
+ | ...
+old_cfunc_nop_cached:unload()
+ | ---
+ | - true
+ | ...
+old_cfunc_nop_cached:unload() -- error
+ | ---
+ | - error: The function is unloaded
+ | ...
+
+-- Cleanup new module data.
+new_cfunc_nop:unload()
+ | ---
+ | - true
+ | ...
+new_cfunc_multireturn:unload()
+ | ---
+ | - true
+ | ...
+new_cfunc_fetch_evens:unload()
+ | ---
+ | - true
+ | ...
+new_cfunc_args:unload()
+ | ---
+ | - true
+ | ...
+new_cfunc_sum:unload()
+ | ---
+ | - true
+ | ...
+new_module:unload()
+ | ---
+ | - true
+ | ...
+
+--
+-- Cleanup the generated symlink
+_ = pcall(fio.unlink(cfunc_path))
+ | ---
+ | ...
diff --git a/test/box/cmod.test.lua b/test/box/cmod.test.lua
new file mode 100644
index 000000000..67845a4ea
--- /dev/null
+++ b/test/box/cmod.test.lua
@@ -0,0 +1,123 @@
+--
+-- gh-4642: New cmod module to be able to
+-- run C stored functions on read only nodes
+-- without requirement to register them with
+-- box.schema.func help.
+--
+build_path = os.getenv("BUILDDIR")
+package.cpath = build_path..'/test/box/?.so;'..build_path..'/test/box/?.dylib;'..package.cpath
+
+cmod = require('cmod')
+fio = require('fio')
+
+ext = (jit.os == "OSX" and "dylib" or "so")
+
+cfunc_path = fio.pathjoin(build_path, "test/box/cfunc.") .. ext
+cfunc1_path = fio.pathjoin(build_path, "test/box/cfunc1.") .. ext
+cfunc2_path = fio.pathjoin(build_path, "test/box/cfunc2.") .. ext
+
+_ = pcall(fio.unlink(cfunc_path))
+fio.symlink(cfunc1_path, cfunc_path)
+
+_, err = pcall(cmod.load, 'non-such-module')
+assert(err ~= nil)
+
+-- They all are sitting in cfunc.so. The "nop" module
+-- contains functions which are simply return nothing.
+old_module = cmod.load('cfunc')
+old_cfunc_nop = old_module:load('cfunc_nop')
+old_cfunc_fetch_evens = old_module:load('cfunc_fetch_evens')
+old_cfunc_multireturn = old_module:load('cfunc_multireturn')
+old_cfunc_args = old_module:load('cfunc_args')
+old_cfunc_sum = old_module:load('cfunc_sum')
+-- Test for error on nonexisting function.
+_, err = pcall(old_module.load, old_module, 'no-such-func')
+assert(err ~= nil)
+
+-- Make sure they all are callable.
+old_cfunc_nop()
+old_cfunc_fetch_evens()
+old_cfunc_multireturn()
+old_cfunc_args()
+old_cfunc_sum()
+
+-- Unload the module but keep old functions alive, so
+-- they keep reference to NOP module internally
+-- and still callable.
+old_module:unload()
+old_cfunc_nop()
+old_cfunc_fetch_evens()
+old_cfunc_multireturn()
+old_cfunc_args()
+old_cfunc_sum()
+
+-- The module is unloaded I should not be able
+-- to load new shared library.
+old_module:load('cfunc')
+-- Neither I should be able to unload module twise.
+old_module:unload()
+
+-- Clean old functions.
+old_cfunc_nop:unload()
+old_cfunc_fetch_evens:unload()
+old_cfunc_multireturn:unload()
+old_cfunc_args:unload()
+old_cfunc_sum:unload()
+
+-- And reload old module again.
+old_module = cmod.load('cfunc')
+old_cfunc_nop = old_module:load('cfunc_nop')
+
+-- Overwrite module with new contents.
+_ = pcall(fio.unlink(cfunc_path))
+fio.symlink(cfunc2_path, cfunc_path)
+
+-- Now try to load a function from old module,
+-- we should get chache invalidation, old module
+-- get orphaned and lookup for functions from
+-- old module should be prohibited.
+new_module = cmod.load('cfunc')
+new_cfunc_nop = old_module:load('cfunc_nop')
+old_module:load('cfunc_fetch_evens')
+-- Previously loaded functions from old module should
+-- sit in functions cache and simply increment internal
+-- references.
+old_cfunc_nop_cached = old_module:load('cfunc_nop')
+old_module:unload()
+
+-- New module is loaded, lets lookup for updated symbols.
+new_cfunc_nop = new_module:load('cfunc_nop')
+new_cfunc_fetch_evens = new_module:load('cfunc_fetch_evens')
+new_cfunc_multireturn = new_module:load('cfunc_multireturn')
+new_cfunc_args = new_module:load('cfunc_args')
+new_cfunc_sum = new_module:load('cfunc_sum')
+
+-- Call old functions.
+old_cfunc_nop()
+old_cfunc_nop_cached()
+
+-- And newly loaded ones.
+new_cfunc_nop()
+new_cfunc_multireturn()
+new_cfunc_fetch_evens({2,4,6})
+new_cfunc_fetch_evens({1,2,3})  -- error
+new_cfunc_args(1, "hello")
+new_cfunc_sum(1) -- error
+new_cfunc_sum(1,2)
+
+-- Cleanup old module's functions.
+old_cfunc_nop:unload()
+old_cfunc_nop_cached:unload()
+old_cfunc_nop_cached:unload() -- error
+
+-- Cleanup new module data.
+new_cfunc_nop:unload()
+new_cfunc_multireturn:unload()
+new_cfunc_fetch_evens:unload()
+new_cfunc_args:unload()
+new_cfunc_sum:unload()
+new_module:unload()
+
+--
+-- Cleanup the generated symlink
+_ = pcall(fio.unlink(cfunc_path))
diff --git a/test/box/suite.ini b/test/box/suite.ini
index e700d0b9e..fc16c5951 100644
--- a/test/box/suite.ini
+++ b/test/box/suite.ini
@@ -2,7 +2,7 @@
 core = tarantool
 description = Database tests
 script = box.lua
-disabled = rtree_errinj.test.lua tuple_bench.test.lua
+disabled = rtree_errinj.test.lua tuple_bench.test.lua cmod.test.lua
 long_run = huge_field_map_long.test.lua
 config = engine.cfg
 release_disabled = errinj.test.lua errinj_index.test.lua rtree_errinj.test.lua upsert_errinj.test.lua iproto_stress.test.lua gh-4648-func-load-unload.test.lua
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-02-02 22:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 22:11 [Tarantool-patches] [PATCH v14 00/12] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 01/12] box/func: factor out c function entry structure Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 02/12] module_cache: move module handling into own subsystem Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 03/12] module_cache: direct update a cache value on reload Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:11 ` [Tarantool-patches] [PATCH v14 04/12] module_cache: rename calls to ref in module structure Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 05/12] module_cache: add comment about weird resolving Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 06/12] module_cache: module_reload - drop redundant parameter Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 07/12] module_cache: use references as a main usage counter Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 08/12] module_cache: make module to carry hash it belongs to Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 09/12] module_cache: use own hash for box.schema.func requests Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 10/12] module_cache: provide module_load/unload calls Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 11/12] box/cmod: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
2021-02-02 22:12 ` [Tarantool-patches] [PATCH v14 12/12] test: box/cfunc -- add cmod test Cyrill Gorcunov via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox