[Tarantool-patches] [PATCH v20 5/7] box/schema.func: switch to new module api

Cyrill Gorcunov gorcunov at gmail.com
Fri Apr 2 15:34:18 MSK 2021


Since we have low level module api now we can switch
the box.schema.func code to use it. In particular we
define schema_module structure as a wrapper over the
modules api -- it carries a pointer to general module
structure.

Because of modules reload functionality (which was actually
a big mistake to introduce in a first place, because it is too
fragile and in case of unintended misuse may simply crash the
application in a best case) the schema modules carry own cache
of modules instances. Thus to make overall code somehow close
to modules api we do:

1) every schema_module variable should be named as `mod`. this
   allows to distinguish this kind of modules from general
  `module` variables;
2) cache operations are renamed to cache_find/put/update/del;
3) C functions are switched to use module_func low level api;
4) because low level modules api carry own references we can
   take no explicit reference when calling a function.

Part-of #4642

Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
---
 src/box/func.c                             | 527 +++++++++------------
 src/box/func.h                             |  15 +-
 src/box/func_def.h                         |  14 -
 test/box/gh-4648-func-load-unload.result   |   7 +-
 test/box/gh-4648-func-load-unload.test.lua |   7 +-
 5 files changed, 228 insertions(+), 342 deletions(-)

diff --git a/src/box/func.c b/src/box/func.c
index 08918e6db..54d0cbc68 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -30,19 +30,13 @@
  */
 #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.
@@ -56,6 +50,19 @@ struct func_name {
 	const char *package_end;
 };
 
+/**
+ * Schema module (box.schema) instance.
+ */
+struct schema_module {
+	/** Low level module instance. */
+	struct module *module;
+	/** List of imported functions. */
+	struct rlist funcs;
+	/** Reference counter. */
+	int64_t refs;
+};
+
+
 struct func_c {
 	/** Function object base class. */
 	struct func base;
@@ -64,16 +71,21 @@ struct func_c {
 	 */
 	struct rlist item;
 	/**
-	 * For C functions, the body of the function.
+	 * C function to call.
 	 */
-	box_function_f func;
+	struct module_func mf;
 	/**
-	 * Each stored function keeps a handle to the
-	 * dynamic library for the C callback.
+	 * A schema module the function belongs to.
 	 */
-	struct module *module;
+	struct schema_module *mod;
 };
 
+static void
+schema_module_ref(struct schema_module *mod);
+
+static void
+schema_module_unref(struct schema_module *mod);
+
 /***
  * Split function name to symbol and package names.
  * For example, str = foo.bar.baz => sym = baz, package = foo.bar
@@ -95,82 +107,9 @@ func_split_name(const char *str, struct func_name *name)
 	}
 }
 
-/**
- * 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;
-}
-
+/** Schema modules hash. */
 static struct mh_strnptr_t *modules = NULL;
 
-static void
-module_gc(struct module *module);
-
 int
 schema_module_init(void)
 {
@@ -188,10 +127,8 @@ schema_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);
+		struct schema_module *mod = mh_strnptr_node(modules, i)->val;
+		schema_module_unref(mod);
 	}
 	mh_strnptr_delete(modules);
 }
@@ -199,25 +136,30 @@ schema_module_free(void)
 /**
  * Look up a module in the modules cache.
  */
-static struct module *
-module_cache_find(const char *name, const char *name_end)
+static struct schema_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;
+	return mh_strnptr_node(modules, i)->val;
 }
 
 /**
- * Save module to the module cache.
+ * Save a module to the modules cache.
  */
-static inline int
-module_cache_put(struct module *module)
+static int
+cache_put(struct schema_module *mod)
 {
-	size_t package_len = strlen(module->package);
-	uint32_t name_hash = mh_strn_hash(module->package, package_len);
+	const char *str = mod->module->package;
+	size_t len = mod->module->package_len;
+
 	const struct mh_strnptr_node_t strnode = {
-		module->package, package_len, name_hash, module};
+		.str	= str,
+		.len	= len,
+		.hash	= mh_strn_hash(str, len),
+		.val	= mod,
+	};
 
 	if (mh_strnptr_put(modules, &strnode, NULL, NULL) == mh_end(modules)) {
 		diag_set(OutOfMemory, sizeof(strnode), "malloc", "modules");
@@ -227,190 +169,214 @@ module_cache_put(struct module *module)
 }
 
 /**
- * Delete a module from the module cache
+ * Update a module in the modules cache.
  */
 static void
-module_cache_del(const char *name, const char *name_end)
+cache_update(struct schema_module *mod)
 {
-	mh_int_t i = mh_strnptr_find_inp(modules, name, name_end - name);
+	const char *str = mod->module->package;
+	size_t len = mod->module->package_len;
+
+	mh_int_t i = mh_strnptr_find_inp(modules, str, len);
 	if (i == mh_end(modules))
-		return;
-	mh_strnptr_del(modules, i, NULL);
+		panic("func: failed to update cache: %s", str);
+
+	mh_strnptr_node(modules, i)->str = str;
+	mh_strnptr_node(modules, i)->val = mod;
 }
 
-/*
- * 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.
+/**
+ * Delete a module from the module cache.
  */
-static struct module *
-module_load(const char *package, const char *package_end)
+static void
+cache_del(struct schema_module *mod)
 {
-	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;
+	const char *str = mod->module->package;
+	size_t len = mod->module->package_len;
+
+	mh_int_t i = mh_strnptr_find_inp(modules, str, len);
+	if (i != mh_end(modules)) {
+		struct schema_module *v;
+		v = mh_strnptr_node(modules, i)->val;
+		/*
+		 * The module may be already reloaded so
+		 * the cache carries a new entry instead.
+		 */
+		if (v == mod)
+			mh_strnptr_del(modules, i, NULL);
 	}
+}
 
-	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;
-	}
+/** Delete a module. */
+static void
+schema_module_delete(struct schema_module *mod)
+{
+	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
+	if (e != NULL)
+		--e->iparam;
+	module_unload(mod->module);
+	TRASH(mod);
+	free(mod);
+}
 
-	struct stat st;
-	if (stat(path, &st) < 0) {
-		diag_set(SystemError, "failed to stat() module %s", path);
-		goto error;
-	}
+/** Increment reference to a module. */
+static void
+schema_module_ref(struct schema_module *mod)
+{
+	assert(mod->refs >= 0);
+	++mod->refs;
+}
 
-	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;
+/** Decrement reference to a module and delete it if last one. */
+static void
+schema_module_unref(struct schema_module *mod)
+{
+	assert(mod->refs > 0);
+	if (--mod->refs == 0) {
+		cache_del(mod);
+		schema_module_delete(mod);
 	}
+}
 
-	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;
+static struct schema_module *
+__schema_module_load(const char *name, size_t len, bool force)
+{
+	struct schema_module *mod = malloc(sizeof(*mod));
+	if (mod != NULL) {
+		mod->module = NULL;
+		mod->refs = 0;
+		rlist_create(&mod->funcs);
+	} else {
+		diag_set(OutOfMemory, sizeof(*mod),
+			 "malloc", "struct schema_module");
+		return NULL;
 	}
 
-	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;
+	if (force)
+		mod->module = module_load_force(name, len);
+	else
+		mod->module = module_load(name, len);
+
+	if (!mod->module) {
+		free(mod);
+		return NULL;
 	}
+
 	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
 	if (e != NULL)
 		++e->iparam;
-	return module;
-error:
-	free(module);
-	return NULL;
+
+	schema_module_ref(mod);
+	return mod;
 }
 
-static void
-module_delete(struct module *module)
+/**
+ * Load a new module.
+ */
+static struct schema_module *
+schema_module_load(const char *name, const char *name_end)
 {
-	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
-	if (e != NULL)
-		--e->iparam;
-	dlclose(module->handle);
-	TRASH(module);
-	free(module);
+	return __schema_module_load(name, name_end - name, false);
 }
 
-/*
- * Check if a dso is unused and can be closed.
+/**
+ * Force load a new module.
  */
+static struct schema_module *
+schema_module_load_force(const char *name, const char *name_end)
+{
+	return __schema_module_load(name, name_end - name, true);
+}
+
+static struct func_vtab func_c_vtab;
+
+/** Create a new C function. */
 static void
-module_gc(struct module *module)
+func_c_create(struct func_c *func_c)
 {
-	if (rlist_empty(&module->funcs) && module->calls == 0)
-		module_delete(module);
+	func_c->mod = NULL;
+	func_c->base.vtab = &func_c_vtab;
+	rlist_create(&func_c->item);
+	module_func_create(&func_c->mf);
 }
 
-/*
- * Import a function from the module.
- */
-static box_function_f
-module_sym(struct module *module, const char *name)
+static int
+schema_func_c_load(struct schema_module *mod, const char *func_name,
+		   struct func_c *func)
 {
-	box_function_f f = (box_function_f)dlsym(module->handle, name);
-	if (f == NULL) {
-		diag_set(ClientError, ER_LOAD_FUNCTION, name, dlerror());
-		return NULL;
+	assert(module_func_is_empty(&func->mf));
+
+	if (module_func_load(mod->module, func_name, &func->mf) != 0)
+		return -1;
+
+	func->mod = mod;
+	rlist_move(&mod->funcs, &func->item);
+	schema_module_ref(mod);
+
+	return 0;
+}
+
+static void
+schema_func_c_unload(struct func_c *func)
+{
+	if (!module_func_is_empty(&func->mf)) {
+		rlist_del(&func->item);
+		schema_module_unref(func->mod);
+		module_func_unload(&func->mf);
+		func_c_create(func);
 	}
-	return f;
 }
 
 int
 schema_module_reload(const char *package, const char *package_end)
 {
-	struct module *old_module = module_cache_find(package, package_end);
-	if (old_module == NULL) {
+	struct schema_module *old = cache_find(package, package_end);
+	if (old == NULL) {
 		/* Module wasn't loaded - do nothing. */
 		diag_set(ClientError, ER_NO_SUCH_MODULE, package);
 		return -1;
 	}
 
-	struct module *new_module = module_load(package, package_end);
-	if (new_module == NULL)
+	struct schema_module *new = schema_module_load_force(package, package_end);
+	if (new == NULL)
 		return -1;
 
+	/*
+	 * Keep an extra reference to the old module
+	 * so it won't be freed until reload is complete,
+	 * otherwise we might free old module then fail
+	 * on some function loading and in result won't
+	 * be able to restore old symbols.
+	 */
+	schema_module_ref(old);
 	struct func_c *func, *tmp_func;
-	rlist_foreach_entry_safe(func, &old_module->funcs, item, tmp_func) {
-		/* Move immediately for restore sake. */
-		rlist_move(&new_module->funcs, &func->item);
+	rlist_foreach_entry_safe(func, &old->funcs, item, tmp_func) {
 		struct func_name name;
 		func_split_name(func->base.def->name, &name);
-		func->func = module_sym(new_module, name.sym);
-		if (func->func == NULL)
+		schema_func_c_unload(func);
+		if (schema_func_c_load(new, name.sym, func) != 0) {
+			/*
+			 * WARN: A hack accessing new->funcs directly
+			 * to start restore from this failing function.
+			 */
+			rlist_move(&new->funcs, &func->item);
 			goto restore;
-		func->module = new_module;
+		}
 	}
-	module_cache_del(package, package_end);
-	if (module_cache_put(new_module) != 0)
-		goto restore;
-	module_gc(old_module);
+	cache_update(new);
+	schema_module_unref(old);
+	schema_module_unref(new);
 	return 0;
 restore:
 	/*
 	 * Some old functions are not found int the new module,
 	 * thus restore all migrated functions back to original.
 	 */
-	rlist_foreach_entry_safe(func, &new_module->funcs, item, tmp_func) {
+	rlist_foreach_entry_safe(func, &new->funcs, item, tmp_func) {
 		struct func_name name;
 		func_split_name(func->base.def->name, &name);
-		func->func = module_sym(old_module, name.sym);
-		if (func->func == NULL) {
+		schema_func_c_unload(func);
+		if (schema_func_c_load(old, name.sym, func) != 0) {
 			/*
 			 * Something strange was happen, an early loaden
 			 * function was not found in an old dso.
@@ -418,10 +384,9 @@ schema_module_reload(const char *package, const char *package_end)
 			panic("Can't restore module function, "
 			      "server state is inconsistent");
 		}
-		func->module = old_module;
-		rlist_move(&old_module->funcs, &func->item);
 	}
-	module_delete(new_module);
+	schema_module_unref(old);
+	schema_module_unref(new);
 	return -1;
 }
 
@@ -469,8 +434,6 @@ func_new(struct func_def *def)
 	return func;
 }
 
-static struct func_vtab func_c_vtab;
-
 static struct func *
 func_c_new(MAYBE_UNUSED struct func_def *def)
 {
@@ -481,35 +444,17 @@ func_c_new(MAYBE_UNUSED struct func_def *def)
 		diag_set(OutOfMemory, sizeof(*func), "malloc", "func");
 		return NULL;
 	}
-	func->base.vtab = &func_c_vtab;
-	func->func = NULL;
-	func->module = NULL;
+	func_c_create(func);
 	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);
+	schema_func_c_unload(func);
 	TRASH(base);
 	free(func);
 }
@@ -521,45 +466,32 @@ func_c_destroy(struct func *base)
 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);
+	struct schema_module *cached, *mod;
+	cached = cache_find(name.package, name.package_end);
 	if (cached == NULL) {
-		module = module_load(name.package, name.package_end);
-		if (module == NULL)
+		mod = schema_module_load(name.package, name.package_end);
+		if (mod == NULL)
 			return -1;
-		if (module_cache_put(module)) {
-			module_delete(module);
+		if (cache_put(mod)) {
+			schema_module_unref(mod);
 			return -1;
 		}
 	} else {
-		module = cached;
+		mod = cached;
+		schema_module_ref(mod);
 	}
 
-	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 rc = schema_func_c_load(mod, name.sym, func);
+	/*
+	 * There is no explicit module loading in this
+	 * interface so each function carries a reference
+	 * by their own.
+	 */
+	schema_module_unref(mod);
+	return rc;
 }
 
 int
@@ -568,38 +500,17 @@ 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 (module_func_is_empty(&func->mf)) {
 		if (func_c_load(func) != 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->module;
-	assert(module != NULL);
-	++module->calls;
-	int rc = func->func(&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;
+	/*
+	 * Note that we don't take a reference to the
+	 * module, it is handled by low level instance,
+	 * thus while been inside the call the associated
+	 * schema_module can be unreferenced and freed.
+	 */
+	return module_func_call(&func->mf, args, ret);
 }
 
 static struct func_vtab func_c_vtab = {
diff --git a/src/box/func.h b/src/box/func.h
index 5a49e34f4..987ca70db 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -37,6 +37,7 @@
 #include "small/rlist.h"
 #include "func_def.h"
 #include "user_def.h"
+#include "module_cache.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -44,20 +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];
-};
-
 /** Virtual method table for func object. */
 struct func_vtab {
 	/** Call function with given arguments. */
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/test/box/gh-4648-func-load-unload.result b/test/box/gh-4648-func-load-unload.result
index e695dd365..91b78d083 100644
--- a/test/box/gh-4648-func-load-unload.result
+++ b/test/box/gh-4648-func-load-unload.result
@@ -71,7 +71,8 @@ check_module_count_diff(-1)
  | ...
 
 -- A not finished invocation of a function from a module prevents
--- its unload. Until the call is finished.
+-- low level module intance unload while schema level module is
+-- free to unload immediately when dropped.
 box.schema.func.create('function1', {language = 'C'})
  | ---
  | ...
@@ -110,7 +111,7 @@ box.schema.func.drop('function1')
 box.schema.func.drop('function1.test_sleep')
  | ---
  | ...
-check_module_count_diff(0)
+check_module_count_diff(-1)
  | ---
  | ...
 
@@ -132,6 +133,6 @@ test_run:wait_cond(function() return f2:status() == 'dead' end)
  | ---
  | - true
  | ...
-check_module_count_diff(-1)
+check_module_count_diff(0)
  | ---
  | ...
diff --git a/test/box/gh-4648-func-load-unload.test.lua b/test/box/gh-4648-func-load-unload.test.lua
index 10b9de87a..4c4f30af7 100644
--- a/test/box/gh-4648-func-load-unload.test.lua
+++ b/test/box/gh-4648-func-load-unload.test.lua
@@ -36,7 +36,8 @@ box.schema.func.drop('function1')
 check_module_count_diff(-1)
 
 -- A not finished invocation of a function from a module prevents
--- its unload. Until the call is finished.
+-- low level module intance unload while schema level module is
+-- free to unload immediately when dropped.
 box.schema.func.create('function1', {language = 'C'})
 box.schema.func.create('function1.test_sleep', {language = 'C'})
 check_module_count_diff(0)
@@ -52,7 +53,7 @@ box.func.function1:call()
 check_module_count_diff(1)
 box.schema.func.drop('function1')
 box.schema.func.drop('function1.test_sleep')
-check_module_count_diff(0)
+check_module_count_diff(-1)
 
 f1:cancel()
 test_run:wait_cond(function() return f1:status() == 'dead' end)
@@ -60,4 +61,4 @@ check_module_count_diff(0)
 
 f2:cancel()
 test_run:wait_cond(function() return f2:status() == 'dead' end)
-check_module_count_diff(-1)
+check_module_count_diff(0)
-- 
2.30.2



More information about the Tarantool-patches mailing list