[Tarantool-patches] [PATCH v15 00/11] box: implement cmod Lua module

[Tarantool-patches] [PATCH v15 00/11] box: implement cmod Lua module

[Cyrill Gorcunov via Tarantool-patches]

The series implements a bare minimum. 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
v15:
 - report module state cached/orphan
 - update test cases
 - do not prevent functions lookup in orphan modules
 - there was an idea to use box.shema.func cache as on
   top of cmod's one, but this doesn't work because in case
   if module doesnt exist in any caches we would put it into
   into cmod's one as well but there wont be a module on
   cmod level which would clean it up later (which makes
   code a way more comple if we choose to track state
   of modules).

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

Cyrill Gorcunov (11):
  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
  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      | 610 ++++++++++++++++++++++++++++++++
 src/box/lua/cmod.h      |  24 ++
 src/box/lua/init.c      |   2 +
 src/box/module_cache.c  | 756 ++++++++++++++++++++++++++++++++++++++++
 src/box/module_cache.h  | 178 ++++++++++
 test/box/CMakeLists.txt |   2 +
 test/box/cfunc1.c       |  58 +++
 test/box/cfunc2.c       | 137 ++++++++
 test/box/cmod.result    | 336 ++++++++++++++++++
 test/box/cmod.test.lua  | 130 +++++++
 test/box/suite.ini      |   2 +-
 16 files changed, 2248 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

[Tarantool-patches] [PATCH v15 01/11] box/func: factor out c function entry structure

[Cyrill Gorcunov via Tarantool-patches]

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

[Tarantool-patches] [PATCH v15 02/11] module_cache: move module handling into own subsystem

[Cyrill Gorcunov via Tarantool-patches]

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..4d89baf9b
--- /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->addr == 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

[Tarantool-patches] [PATCH v15 03/11] module_cache: direct update a cache value on reload

[Cyrill Gorcunov via Tarantool-patches]

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 4d89baf9b..90d18fa73 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

[Tarantool-patches] [PATCH v15 04/11] module_cache: rename calls to ref in module structure

[Cyrill Gorcunov via Tarantool-patches]

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 90d18fa73..e8a510d26 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

[Tarantool-patches] [PATCH v15 05/11] module_cache: add comment about weird resolving

[Cyrill Gorcunov via Tarantool-patches]

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 e8a510d26..f606ec327 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

[Tarantool-patches] [PATCH v15 06/11] module_cache: module_reload - drop redundant parameter

[Cyrill Gorcunov via Tarantool-patches]

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 f606ec327..3fc8261e2 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

[Tarantool-patches] [PATCH v15 07/11] module_cache: use references as a main usage counter

[Cyrill Gorcunov via Tarantool-patches]

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 3fc8261e2..44f34b375 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

[Tarantool-patches] [PATCH v15 08/11] module_cache: make module to carry hash it belongs to

[Cyrill Gorcunov via Tarantool-patches]

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 44f34b375..786e49cba 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

[Tarantool-patches] [PATCH v15 09/11] module_cache: use own hash for box.schema.func requests

[Cyrill Gorcunov via Tarantool-patches]

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 786e49cba..22b906fd7 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

[Tarantool-patches] [PATCH v15 10/11] box/cmod: implement cmod Lua module

[Cyrill Gorcunov via Tarantool-patches]

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.

The cmod interface implies explicit module loading and unloading
before resolving symbols. For this sake we introduce module_load
and module_unload calls.

module_loads tries to reuse modules cache in case if shared library
has been loaded already. This is needed to speedup module loading
when some complex application with a number of different Lua scripts
use the same shared library.

Internally module_load test for shared library file attributes
(device, inode, mtime, size) to make sure the library has not
been overwritten, otherwise the cache entry get evicted and
new instance loaded instaead. Note that all previous instances
of loaded module are not changed and continue working as is.
This is on of the main differences from box.schema.func interface
which never expires cache entries until explicitly "reloaded".
Presumably we deprecate old inteface completely in time. For
now to keep backward compatibility we track modules in two different
caches -- one for box.schema.func and one for cmod interface, they
do not interfere.

@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, new instance is loaded and become a valid cache
entry. Module state could be seen in module variable output

```Lua
m = require('cmod').load('cfunc')
m
```
which will output
```
tarantool> m
---
- path: cfunc
  state: cached
```

The `state` is either `cached` if module is present in cache
and valid or `orphan` if entry in cache has been updated.

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     | 610 +++++++++++++++++++++++++++++++++++++++++
 src/box/lua/cmod.h     |  24 ++
 src/box/lua/init.c     |   2 +
 src/box/module_cache.c | 211 +++++++++++---
 src/box/module_cache.h |  36 +++
 6 files changed, 842 insertions(+), 42 deletions(-)
 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..60fd2e812
--- /dev/null
+++ b/src/box/lua/cmod.c
@@ -0,0 +1,610 @@
+/*
+ * 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) {
+		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;
+}
+
+static const char *
+module_state_str(struct module *m)
+{
+	return module_is_orphan(m) ? "orphan" : "cached";
+}
+
+/**
+ * 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;
+	} else if (strcmp(key, "state") == 0) {
+		lua_pushstring(L, module_state_str(m));
+		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, 2);
+	lua_pushstring(L, m->package);
+	lua_setfield(L, -2, "path");
+	lua_pushstring(L, module_state_str(m));
+	lua_setfield(L, -2, "state");
+
+	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);
diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index 22b906fd7..e96cbd1f8 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;
 }
 
 /***
@@ -160,7 +182,7 @@ module_set_orphan(struct module *module)
 /**
  * Test if module is out of cache.
  */
-static bool
+bool
 module_is_orphan(struct module *module)
 {
 	return module->hash == NULL;
@@ -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;
@@ -403,30 +421,52 @@ module_sym(struct module *module, const char *name)
 int
 module_sym_load(struct module_sym *mod_sym, bool is_box_schema)
 {
+	struct module *cached, *module;
 	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;
-	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);
-		if (module == NULL)
-			return -1;
-		if (module_cache_add(module) != 0) {
-			module_unref(module);
-			return -1;
+	if (is_box_schema) {
+		/*
+		 * Deprecated interface -- request comes
+		 * from box.schema.func.
+		 *
+		 * In case if module has been loaded already by
+		 * some previous call we can eliminate redundant
+		 * loading and take it from the cache.
+		 */
+		struct mh_strnptr_t *h = hash_tbl(is_box_schema);
+		cached = module_cache_find(h, name.package, name.package_end);
+		if (cached == NULL) {
+			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) {
+				module_unref(module);
+				return -1;
+			}
+		} else {
+			module_ref(cached);
+			module = cached;
 		}
+		mod_sym->module = module;
 	} else {
-		module_ref(cached);
-		module = cached;
+		/*
+		 * New approach is always load module
+		 * explicitly and pass it inside symbol,
+		 * the refernce to the module already has
+		 * to be incremented.
+		 */
+		assert(mod_sym->module->refs > 0);
+		module_ref(mod_sym->module);
+		module = mod_sym->module;
 	}
 
 	mod_sym->addr = module_sym(module, name.sym);
@@ -435,7 +475,6 @@ module_sym_load(struct module_sym *mod_sym, bool is_box_schema)
 		return -1;
 	}
 
-	mod_sym->module = module;
 	rlist_add(&module->funcs_list, &mod_sym->item);
 	return 0;
 }
@@ -514,6 +553,74 @@ 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 update is needed one can simply "touch file.so"
+	 * to invalidate the cache entry.
+	 */
+	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 orphan an old module instance.
+	 */
+	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 +636,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 +717,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 +739,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..17a2e27bb 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.
 	 */
@@ -76,6 +84,15 @@ struct module_sym {
 	char *name;
 };
 
+/**
+ * Test if module is orphan and cache carries
+ * up to date version instead.
+ *
+ * @retval true if module is orphan, false otherwise.
+ */
+bool
+module_is_orphan(struct module *module);
+
 /**
  * Load a new module symbol.
  *
@@ -112,6 +129,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

[Tarantool-patches] [PATCH v15 11/11] test: box/cfunc -- add cmod test

[Cyrill Gorcunov via Tarantool-patches]

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    | 336 ++++++++++++++++++++++++++++++++++++++++
 test/box/cmod.test.lua  | 130 ++++++++++++++++
 test/box/suite.ini      |   2 +-
 6 files changed, 664 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..72d2f1d7d
--- /dev/null
+++ b/test/box/cmod.result
@@ -0,0 +1,336 @@
+-- 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')
+ | ---
+ | ...
+assert(old_module.state == "cached")
+ | ---
+ | - true
+ | ...
+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 but all
+-- old functions should be callable.
+new_module = cmod.load('cfunc')
+ | ---
+ | ...
+assert(old_module.state == "orphan")
+ | ---
+ | - true
+ | ...
+
+-- Still all functions from old module should
+-- be callable.
+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')
+ | ---
+ | ...
+
+-- 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_fetch_evens()
+ | ---
+ | - true
+ | ...
+old_cfunc_multireturn()
+ | ---
+ | - true
+ | ...
+old_cfunc_args()
+ | ---
+ | - true
+ | ...
+old_cfunc_sum()
+ | ---
+ | - true
+ | ...
+
+-- And newly loaded ones.
+new_cfunc_nop()
+ | ---
+ | - true
+ | ...
+new_cfunc_multireturn()
+ | ---
+ | - true
+ | ...
+new_cfunc_fetch_evens({2,4,6})
+ | ---
+ | - true
+ | ...
+new_cfunc_fetch_evens({1,2,3})  -- error
+ | ---
+ | - true
+ | ...
+new_cfunc_args(1, "hello")
+ | ---
+ | - true
+ | ...
+new_cfunc_sum(1) -- error
+ | ---
+ | - true
+ | ...
+new_cfunc_sum(1,2)
+ | ---
+ | - true
+ | ...
+
+-- Cleanup old module's 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
+ | ...
+
+-- 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..4d8291651
--- /dev/null
+++ b/test/box/cmod.test.lua
@@ -0,0 +1,130 @@
+--
+-- 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')
+assert(old_module.state == "cached")
+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 but all
+-- old functions should be callable.
+new_module = cmod.load('cfunc')
+assert(old_module.state == "orphan")
+
+-- Still all functions from old module should
+-- be callable.
+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')
+
+-- 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_fetch_evens()
+old_cfunc_multireturn()
+old_cfunc_args()
+old_cfunc_sum()
+
+-- 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_fetch_evens:unload()
+old_cfunc_multireturn:unload()
+old_cfunc_args:unload()
+old_cfunc_sum:unload()
+
+-- 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

Re: [Tarantool-patches] [PATCH v15 00/11] box: implement cmod Lua module

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the patchset!

> v15:
>  - report module state cached/orphan
>  - update test cases
>  - do not prevent functions lookup in orphan modules
>  - there was an idea to use box.shema.func cache as on
>    top of cmod's one, but this doesn't work because in case
>    if module doesnt exist in any caches we would put it into
>    into cmod's one as well but there wont be a module on
>    cmod level which would clean it up later (which makes
>    code a way more comple if we choose to track state
>    of modules).

It wouldn't be related to cmod. Did you really try? The cache
would work on the level of module_cache. And entries there would
be reference counted. If a module is used only by box.schema.func,
it has ref counter 1. If it not used anymore, it gets counter 0
and is deleted. If it also used by cmod, its counter is not 0, and
it is not deleted. What is the problem?

> branch gorcunov/gh-4642-func-ro-15
> issue https://github.com/tarantool/tarantool/issues/4642
> 
> Cyrill Gorcunov (11):
>   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
>   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      | 610 ++++++++++++++++++++++++++++++++
>  src/box/lua/cmod.h      |  24 ++
>  src/box/lua/init.c      |   2 +
>  src/box/module_cache.c  | 756 ++++++++++++++++++++++++++++++++++++++++
>  src/box/module_cache.h  | 178 ++++++++++
>  test/box/CMakeLists.txt |   2 +
>  test/box/cfunc1.c       |  58 +++
>  test/box/cfunc2.c       | 137 ++++++++
>  test/box/cmod.result    | 336 ++++++++++++++++++
>  test/box/cmod.test.lua  | 130 +++++++
>  test/box/suite.ini      |   2 +-
>  16 files changed, 2248 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
> 

Re: [Tarantool-patches] [PATCH v15 02/11] module_cache: move module handling into own subsystem

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the patch!

> diff --git a/src/box/module_cache.c b/src/box/module_cache.c
> new file mode 100644
> index 000000000..4d89baf9b
> --- /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"

Error.h is included twice again. Here and a few lines below.

> +#include "errinj.h"
> +#include "fiber.h"
> +#include "port.h"
> +
> +#include "error.h"

Re: [Tarantool-patches] [PATCH v15 07/11] module_cache: use references as a main usage counter

[Vladislav Shpilevoy via Tarantool-patches]

Thanks for the patch!

See 2 comments below.

> +
> +/**
> + * 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]);

1. You can also compare pointer in the hash. If it matches -
delete it. Then you wouldn't need the orphan concept at all.

> +		}
> +		module_delete(module);
> +	}
> +}
> @@ -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);

2. Where is the explicit ref?

Re: [Tarantool-patches] [PATCH v15 08/11] module_cache: make module to carry hash it belongs to

[Vladislav Shpilevoy via Tarantool-patches]

Thanks for the patch!

On 05.02.2021 19:54, Cyrill Gorcunov via Tarantool-patches wrote:
> We will need two hashes for backward compatibility sake,
> thus make module structure to carry the hash it belongs.

You don't need this patch if you implement the second hash on top of
the first one like I advised.

> 
> Part-of #4642
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH v15 09/11] module_cache: use own hash for box.schema.func requests

[Vladislav Shpilevoy via Tarantool-patches]

Thanks for the patch!

On 05.02.2021 19:54, Cyrill Gorcunov via Tarantool-patches wrote:
> 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.

After this patch you made module_cache depend on box.schema.func.
Which should not happen. Module_cache is at the lower level. It
should not depend on anything in box except basic things like ClientError,
port.

Besides, you made cmod depend on box.schema.func implicitly. Depend on
the storage. Which was an issue of the first versions of the patchset.
We should not go there again. Both this patch and the previous one should
not really exist. If you have a hash specific for box.schema.func, it
should be there. In box/func.c for example.

AFAIS, this issue should disappear when you will have a second hash on top
of the first one. Then module_sym on first load will get a module from
module_cache. You will put it into the box.schema.func hash, increase
reference counter. And all next loads will see module_sym.module != NULL
even if it was deleted from the lower hash. So it won't lead to load of
a new module.

But even if we wouldn't make the second hash on top, but on the side, the
hack in this commit also could be fixed. You would need to extract
module_sym first load into a new function. The regular sym load would accept
a module object. And also you would need to expose methods to load/unload an
entire module, not just module_sym. Then you could have a separate hash in
another file. However as we discussed verbally, better go for the two-level
hash approach.

I suggest to try this really-really hard. It won't help if you will send
a new patchset almost every day. It will only slow things down.

P.S. I see you even did separate the module load/unload into new
functions, like I advised above. This should be a sign that the module_cache
literally asks us not to make 2 independent hash tables in it.

> diff --git a/src/box/module_cache.c b/src/box/module_cache.c
> index 786e49cba..22b906fd7 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)

The amount of hacks like 'is_box_schema' flag, 'hash_tbl' function,
'module hash pointer' in the module object, and even usage of word 'schema'
in a subsystem which has nothing to do with the storage, should be a
red flag that something is wrong.

Re: [Tarantool-patches] [PATCH v15 10/11] box/cmod: implement cmod Lua module

[Vladislav Shpilevoy via Tarantool-patches]

Thanks for the patch!

See 10 comments below.

Overall no major issues, much better now.

> 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.

1. We decided not to throw an errors in this case.

>  - OutOfMemory: unable to allocate a function.
> 
> 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.

2. In the previous review I reminded you that now we throw errors
again, according to the updated Lua guidelines.

> 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.

3. Will what? "See"?

> The error message has been set by the `box_error_set` in `C`
> code above.
> 
> 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, new instance is loaded and become a valid cache
> entry. Module state could be seen in module variable output
> 
> ```Lua
> m = require('cmod').load('cfunc')
> m
> ```
> which will output
> ```
> tarantool> m
> ---
> - path: cfunc
>   state: cached
> ```

4. Lets not expose any states or other attributes until it is requested
explicitly by someone.

> The `state` is either `cached` if module is present in cache
> and valid or `orphan` if entry in cache has been updated.
> 
> diff --git a/src/box/lua/cmod.c b/src/box/lua/cmod.c
> new file mode 100644
> index 000000000..60fd2e812
> --- /dev/null
> +++ b/src/box/lua/cmod.c
> +
> +/**
> + * Unload a symbol and free a function instance.
> + */
> +static void
> +func_delete(struct cmod_func *cf)

5. Methods of a struct should have the struct name as a prefix.
So it should be cmod_func_delete(). The same for other
cmod_func methods. For Lua the prefix 'lcmod_func' is good.

> +{
> +	assert(cf->refs == 0);
> +	module_sym_unload(&cf->mod_sym);
> +	TRASH(cf);
> +	free(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.

6. It was decided not to throw an error in this case.

> + *
> + * @returns function object on success or throwns an error.
> + */
> +static int
> +lcmod_func_load(struct lua_State *L)
> 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);

7. Please, add an empty line after the declaration.

> +#if defined(__cplusplus)
> +}
> +#endif /* defined(__plusplus) */
> diff --git a/src/box/module_cache.c b/src/box/module_cache.c
> index 22b906fd7..e96cbd1f8 100644
> --- a/src/box/module_cache.c
> +++ b/src/box/module_cache.c

8. Most of changes in this file should not happen, as I
described in the comments to the previous patches.

You will probably need to add a few more commits for this.
One to add a second hash in box/func; next to make module
loading always check stat in module_cache.

> @@ -514,6 +553,74 @@ 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 update is needed one can simply "touch file.so"
> +	 * to invalidate the cache entry.
> +	 */
> +	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;

9. Build fails on Mac:

/Users/gerold/Work/Repositories/tarantool/src/box/module_cache.c:596:25: error: no member named 'st_mtim' in 'struct stat'
            memcmp(&cached->st.st_mtim, &st.st_mtim,
                    ~~~~~~~~~~ ^
/Users/gerold/Work/Repositories/tarantool/src/box/module_cache.c:596:38: error: no member named 'st_mtim' in 'struct stat'
            memcmp(&cached->st.st_mtim, &st.st_mtim,
                                         ~~ ^
/Users/gerold/Work/Repositories/tarantool/src/box/module_cache.c:597:16: error: no member named 'st_mtim' in 'struct stat'
                   sizeof(st.st_mtim)) == 0) {
                          ~~ ^
3 errors generated.

This is because here last modification time is stored as a
timespec struct, and has name 'st_mtimespec'.

> +	}
> +
> +	/*
> +	 * Load a new module, update the cache
> +	 * and orphan an old module instance.
> +	 */
> +	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)
>  {
> diff --git a/src/box/module_cache.h b/src/box/module_cache.h
> index 875f2eb3c..17a2e27bb 100644
> --- a/src/box/module_cache.h
> +++ b/src/box/module_cache.h
> @@ -48,6 +52,10 @@ struct module {
>  	 * Count of active references to the module.
>  	 */
>  	int64_t refs;
> +	/**
> +	 * Storage stat for identity check.
> +	 */
> +	struct stat st;

10. The structure if huge. And you use just a few fields. Better
store only the need fields.

>  	/**
>  	 * Module's package name.
>  	 */

Re: [Tarantool-patches] [PATCH v15 11/11] test: box/cfunc -- add cmod test

[Vladislav Shpilevoy via Tarantool-patches]

On 05.02.2021 19:54, Cyrill Gorcunov via Tarantool-patches wrote:
> 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    | 336 ++++++++++++++++++++++++++++++++++++++++
>  test/box/cmod.test.lua  | 130 ++++++++++++++++
>  test/box/suite.ini      |   2 +-
>  6 files changed, 664 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..72d2f1d7d
> --- /dev/null
> +++ b/test/box/cmod.result
> @@ -0,0 +1,336 @@
> +-- 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')
> + | ---
> + | ...
> +assert(old_module.state == "cached")
> + | ---
> + | - true
> + | ...
> +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 but all
> +-- old functions should be callable.
> +new_module = cmod.load('cfunc')
> + | ---
> + | ...
> +assert(old_module.state == "orphan")
> + | ---
> + | - true
> + | ...
> +
> +-- Still all functions from old module should
> +-- be callable.
> +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')
> + | ---
> + | ...
> +
> +-- 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_fetch_evens()
> + | ---
> + | - true
> + | ...
> +old_cfunc_multireturn()
> + | ---
> + | - true
> + | ...
> +old_cfunc_args()
> + | ---
> + | - true
> + | ...
> +old_cfunc_sum()
> + | ---
> + | - true
> + | ...
> +
> +-- And newly loaded ones.
> +new_cfunc_nop()
> + | ---
> + | - true
> + | ...
> +new_cfunc_multireturn()
> + | ---
> + | - true
> + | ...
> +new_cfunc_fetch_evens({2,4,6})
> + | ---
> + | - true
> + | ...
> +new_cfunc_fetch_evens({1,2,3})  -- error

*. Why error? The same a few lines below.

> + | ---
> + | - true
> + | ...
> +new_cfunc_args(1, "hello")
> + | ---
> + | - true
> + | ...
> +new_cfunc_sum(1) -- error
> + | ---
> + | - true
> + | ...
> +new_cfunc_sum(1,2)
> + | ---
> + | - true
> + | ...
> +
> +-- Cleanup old module's 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
> + | ...
> +
> +-- 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..4d8291651
> --- /dev/null
> +++ b/test/box/cmod.test.lua
> @@ -0,0 +1,130 @@
> +--
> +-- 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')
> +assert(old_module.state == "cached")
> +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 but all
> +-- old functions should be callable.
> +new_module = cmod.load('cfunc')
> +assert(old_module.state == "orphan")
> +
> +-- Still all functions from old module should
> +-- be callable.
> +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')
> +
> +-- 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_fetch_evens()
> +old_cfunc_multireturn()
> +old_cfunc_args()
> +old_cfunc_sum()
> +
> +-- 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_fetch_evens:unload()
> +old_cfunc_multireturn:unload()
> +old_cfunc_args:unload()
> +old_cfunc_sum:unload()
> +
> +-- 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
> 

Re: [Tarantool-patches] [PATCH v15 11/11] test: box/cfunc -- add cmod test

[Vladislav Shpilevoy via Tarantool-patches]

Sorry, I sent the previous email accidentally. Here is the
correct version.

> +new_cfunc_fetch_evens({1,2,3})  -- error

Why error? The same a few lines below.

Re: [Tarantool-patches] [PATCH v15 07/11] module_cache: use references as a main usage counter

[Cyrill Gorcunov via Tarantool-patches]

On Sun, Feb 07, 2021 at 06:20:34PM +0100, Vladislav Shpilevoy wrote:
> Thanks for the patch!
> 
> See 2 comments below.
> 
> > +
> > +/**
> > + * 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]);
> 
> 1. You can also compare pointer in the hash. If it matches -
> delete it. Then you wouldn't need the orphan concept at all.

This gonna be a way more longer than zapping slash. But if you
prefer this way, sure thing.

> >  
> > -	module_gc(old);
> > +	module_set_orphan(old);
> > +	module_unref(old);
> > +
> > +	/* From explicit load above */
> > +	module_unref(new);
> 
> 2. Where is the explicit ref?

A couple of lines above

	struct module *new = module_load(package, package_end);
	if (new == NULL)
		return -1;

The thing is that box.schema.func doesn't use module_load at all
which takes first reference, instead it uses symols as an origin
for counters, thus we need to decrease the conter here because
when box.schema.func close last symbol nobody gonna call module_unload
and module will hang in memory forever. Probably I should write
more verbose comment here.

	Cyrill