[Tarantool-patches] [PATCH 0/2] Module is not unloaded on function drop

[Tarantool-patches] [PATCH 0/2] Module is not unloaded on function drop

[Vladislav Shpilevoy]

The patchset fixes a problem when a C function, added via
box.schema.func.create(), is dropped, but its dynamic library is not unloaded
from the process' memory.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4648-func-unload
Issue: https://github.com/tarantool/tarantool/issues/4648

Vladislav Shpilevoy (2):
  errinj: provide 'get' method in Lua
  func: fix not unloading of unused modules

 src/box/func.c                             |  16 +--
 src/box/func.h                             |   2 -
 src/box/lua/error.cc                       |  37 ++++--
 src/lib/core/errinj.h                      |   1 +
 test/box/errinj.result                     |  55 +++++++++
 test/box/errinj.test.lua                   |  17 +++
 test/box/function1.c                       |  15 +++
 test/box/gh-4648-func-load-unload.result   | 137 +++++++++++++++++++++
 test/box/gh-4648-func-load-unload.test.lua |  63 ++++++++++
 test/box/suite.ini                         |   2 +-
 10 files changed, 326 insertions(+), 19 deletions(-)
 create mode 100644 test/box/gh-4648-func-load-unload.result
 create mode 100644 test/box/gh-4648-func-load-unload.test.lua

-- 
2.21.0 (Apple Git-122.2)

[Tarantool-patches] [PATCH 1/2] errinj: provide 'get' method in Lua

[Vladislav Shpilevoy]

Error injections are used to simulate an error. They are
represented as a flag, or a number, and are used in Lua tests. But
they don't have any feedback. That makes impossible to use the
injections to check that something has happened. Something very
needed to be checked, and impossible to check in a different way.

More certainly, the patch is motivated by a necessity to count
loaded dynamic libraries to ensure, that they are loaded and
unloaded when expected. This is impossible to do in a platform
independent way. But an error injection as a debug-only counter
would solve the problem.

Needed for #4648
---
 src/box/lua/error.cc     | 37 ++++++++++++++++++++-------
 test/box/errinj.result   | 55 ++++++++++++++++++++++++++++++++++++++++
 test/box/errinj.test.lua | 17 +++++++++++++
 3 files changed, 100 insertions(+), 9 deletions(-)

diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
index 230d51dec..fc53a40f4 100644
--- a/src/box/lua/error.cc
+++ b/src/box/lua/error.cc
@@ -184,26 +184,44 @@ lbox_errinj_set(struct lua_State *L)
 	return 1;
 }
 
-static inline int
-lbox_errinj_cb(struct errinj *e, void *cb_ctx)
+static int
+lbox_errinj_push_value(struct lua_State *L, const struct errinj *e)
 {
-	struct lua_State *L = (struct lua_State*)cb_ctx;
-	lua_pushstring(L, e->name);
-	lua_newtable(L);
-	lua_pushstring(L, "state");
 	switch (e->type) {
 	case ERRINJ_BOOL:
 		lua_pushboolean(L, e->bparam);
-		break;
+		return 1;
 	case ERRINJ_INT:
 		luaL_pushint64(L, e->iparam);
-		break;
+		return 1;
 	case ERRINJ_DOUBLE:
 		lua_pushnumber(L, e->dparam);
-		break;
+		return 1;
 	default:
 		unreachable();
+		return 0;
 	}
+}
+
+static int
+lbox_errinj_get(struct lua_State *L)
+{
+	char *name = (char*)luaL_checkstring(L, 1);
+	struct errinj *e = errinj_by_name(name);
+	if (e != NULL)
+		return lbox_errinj_push_value(L, e);
+	lua_pushfstring(L, "error: can't find error injection '%s'", name);
+	return 1;
+}
+
+static inline int
+lbox_errinj_cb(struct errinj *e, void *cb_ctx)
+{
+	struct lua_State *L = (struct lua_State*)cb_ctx;
+	lua_pushstring(L, e->name);
+	lua_newtable(L);
+	lua_pushstring(L, "state");
+	lbox_errinj_push_value(L, e);
 	lua_settable(L, -3);
 	lua_settable(L, -3);
 	return 0;
@@ -259,6 +277,7 @@ box_lua_error_init(struct lua_State *L) {
 	static const struct luaL_Reg errinjlib[] = {
 		{"info", lbox_errinj_info},
 		{"set", lbox_errinj_set},
+		{"get", lbox_errinj_get},
 		{NULL, NULL}
 	};
 	/* box.error.injection is not set by register_module */
diff --git a/test/box/errinj.result b/test/box/errinj.result
index a148346e8..40683730c 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -1724,3 +1724,58 @@ box.schema.user.drop('testg')
 box.space.testg:drop()
 ---
 ...
+--
+-- Errinj:get().
+--
+box.error.injection.get('bad name')
+---
+- 'error: can''t find error injection ''bad name'''
+...
+box.error.injection.set('ERRINJ_WAL_IO', true)
+---
+- ok
+...
+box.error.injection.get('ERRINJ_WAL_IO')
+---
+- true
+...
+box.error.injection.set('ERRINJ_WAL_IO', false)
+---
+- ok
+...
+box.error.injection.get('ERRINJ_WAL_IO')
+---
+- false
+...
+box.error.injection.set('ERRINJ_TUPLE_FORMAT_COUNT', 20)
+---
+- ok
+...
+box.error.injection.get('ERRINJ_TUPLE_FORMAT_COUNT')
+---
+- 20
+...
+box.error.injection.set('ERRINJ_TUPLE_FORMAT_COUNT', -1)
+---
+- ok
+...
+box.error.injection.get('ERRINJ_TUPLE_FORMAT_COUNT')
+---
+- -1
+...
+box.error.injection.set('ERRINJ_RELAY_TIMEOUT', 0.5)
+---
+- ok
+...
+box.error.injection.get('ERRINJ_RELAY_TIMEOUT')
+---
+- 0.5
+...
+box.error.injection.set('ERRINJ_RELAY_TIMEOUT', 0)
+---
+- ok
+...
+box.error.injection.get('ERRINJ_RELAY_TIMEOUT')
+---
+- 0
+...
diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
index 31dd9665b..03c088677 100644
--- a/test/box/errinj.test.lua
+++ b/test/box/errinj.test.lua
@@ -603,3 +603,20 @@ box.session.su('admin')
 box.error.injection.set('ERRINJ_WAL_IO', false)
 box.schema.user.drop('testg')
 box.space.testg:drop()
+
+--
+-- Errinj:get().
+--
+box.error.injection.get('bad name')
+box.error.injection.set('ERRINJ_WAL_IO', true)
+box.error.injection.get('ERRINJ_WAL_IO')
+box.error.injection.set('ERRINJ_WAL_IO', false)
+box.error.injection.get('ERRINJ_WAL_IO')
+box.error.injection.set('ERRINJ_TUPLE_FORMAT_COUNT', 20)
+box.error.injection.get('ERRINJ_TUPLE_FORMAT_COUNT')
+box.error.injection.set('ERRINJ_TUPLE_FORMAT_COUNT', -1)
+box.error.injection.get('ERRINJ_TUPLE_FORMAT_COUNT')
+box.error.injection.set('ERRINJ_RELAY_TIMEOUT', 0.5)
+box.error.injection.get('ERRINJ_RELAY_TIMEOUT')
+box.error.injection.set('ERRINJ_RELAY_TIMEOUT', 0)
+box.error.injection.get('ERRINJ_RELAY_TIMEOUT')
-- 
2.21.0 (Apple Git-122.2)

[Tarantool-patches] [PATCH 2/2] func: fix not unloading of unused modules

[Vladislav Shpilevoy]

C functions are loaded from .so/.dylib dynamic libraries. A
library is loaded when any function from there is called first
time. And was supposed to be unloaded, when all its functions are
dropped from the schema (box.schema.func.drop()), and none of them
is still in a call. But the unloading part was broken.

In fact, box.schema.func.drop() never unloaded anything. Moreover,
when functions from the module were added again without a restart,
it led to a second mmap of the same module. And so on, the same
library could be loaded any number of times.

The problem was in a useless flag in struct module preventing its
unloading even when it is totally unused. It is dropped.

Closes #4648
---
 src/box/func.c                             |  16 +--
 src/box/func.h                             |   2 -
 src/lib/core/errinj.h                      |   1 +
 test/box/function1.c                       |  15 +++
 test/box/gh-4648-func-load-unload.result   | 137 +++++++++++++++++++++
 test/box/gh-4648-func-load-unload.test.lua |  63 ++++++++++
 test/box/suite.ini                         |   2 +-
 7 files changed, 226 insertions(+), 10 deletions(-)
 create mode 100644 test/box/gh-4648-func-load-unload.result
 create mode 100644 test/box/gh-4648-func-load-unload.test.lua

diff --git a/src/box/func.c b/src/box/func.c
index d9481b714..431577127 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -35,6 +35,7 @@
 #include "lua/utils.h"
 #include "lua/call.h"
 #include "error.h"
+#include "errinj.h"
 #include "diag.h"
 #include "port.h"
 #include "schema.h"
@@ -260,7 +261,6 @@ module_load(const char *package, const char *package_end)
 	module->package[package_len] = 0;
 	rlist_create(&module->funcs);
 	module->calls = 0;
-	module->is_unloading = false;
 	char dir_name[] = "/tmp/tntXXXXXX";
 	if (mkdtemp(dir_name) == NULL) {
 		diag_set(SystemError, "failed to create unique dir name");
@@ -283,7 +283,9 @@ module_load(const char *package, const char *package_end)
 			  package, dlerror());
 		goto error;
 	}
-
+	struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT);
+	if (e != NULL)
+		++e->iparam;
 	return module;
 error:
 	free(module);
@@ -293,6 +295,9 @@ error:
 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);
@@ -304,10 +309,8 @@ module_delete(struct module *module)
 static void
 module_gc(struct module *module)
 {
-	if (!module->is_unloading || !rlist_empty(&module->funcs) ||
-	     module->calls != 0)
-		return;
-	module_delete(module);
+	if (rlist_empty(&module->funcs) && module->calls == 0)
+		module_delete(module);
 }
 
 /*
@@ -351,7 +354,6 @@ module_reload(const char *package, const char *package_end, struct module **modu
 	module_cache_del(package, package_end);
 	if (module_cache_put(new_module) != 0)
 		goto restore;
-	old_module->is_unloading = true;
 	module_gc(old_module);
 	*module = new_module;
 	return 0;
diff --git a/src/box/func.h b/src/box/func.h
index 18b83faac..581e468cb 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -54,8 +54,6 @@ struct module {
 	struct rlist funcs;
 	/** Count of active calls. */
 	size_t calls;
-	/** True if module is being unloaded. */
-	bool is_unloading;
 	/** Module's package name. */
 	char package[0];
 };
diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
index 3072a00ea..672da2119 100644
--- a/src/lib/core/errinj.h
+++ b/src/lib/core/errinj.h
@@ -134,6 +134,7 @@ struct errinj {
 	_(ERRINJ_SQL_NAME_NORMALIZATION, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_COIO_SENDFILE_CHUNK, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_SWIM_FD_ONLY, ERRINJ_BOOL, {.bparam = false}) \
+	_(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT, {.iparam = 0}) \
 
 ENUM0(errinj_id, ERRINJ_LIST);
 extern struct errinj errinjs[];
diff --git a/test/box/function1.c b/test/box/function1.c
index b0d983e2b..fd434c280 100644
--- a/test/box/function1.c
+++ b/test/box/function1.c
@@ -229,3 +229,18 @@ test_yield(box_function_ctx_t *ctx, const char *args, const char *args_end)
 	printf("ok - yield\n");
 	return 0;
 }
+
+int
+test_sleep(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	(void) ctx;
+	(void) args;
+	(void) args_end;
+	/*
+	 * Sleep until an explicit wakeup. Purpose of this
+	 * function - test module unloading prevention while at
+	 * least one of its functions is being executed.
+	 */
+	fiber_sleep(100000);
+	return 0;
+}
diff --git a/test/box/gh-4648-func-load-unload.result b/test/box/gh-4648-func-load-unload.result
new file mode 100644
index 000000000..8c4d23938
--- /dev/null
+++ b/test/box/gh-4648-func-load-unload.result
@@ -0,0 +1,137 @@
+-- test-run result file version 2
+fiber = require('fiber')
+ | ---
+ | ...
+test_run = require('test_run').new()
+ | ---
+ | ...
+build_path = os.getenv("BUILDDIR")
+ | ---
+ | ...
+package.cpath = build_path..'/test/box/?.so;'..build_path..'/test/box/?.dylib;'..package.cpath
+ | ---
+ | ...
+errinj = box.error.injection
+ | ---
+ | ...
+
+--
+-- gh-4648: box.schema.func.drop() didn't unload a .so/.dylib
+-- module. Even if it was unused already. Moreover, recreation of
+-- functions from the same module led to its multiple mmaping.
+--
+
+current_module_count = errinj.get("ERRINJ_DYN_MODULE_COUNT")
+ | ---
+ | ...
+function check_module_count_diff(diff)                          \
+    local module_count = errinj.get("ERRINJ_DYN_MODULE_COUNT")  \
+    current_module_count = current_module_count + diff          \
+    if current_module_count ~= module_count then                \
+        return current_module_count, module_count               \
+    end                                                         \
+end
+ | ---
+ | ...
+
+-- Module is not loaded until any of its functions is called first
+-- time.
+box.schema.func.create('function1', {language = 'C'})
+ | ---
+ | ...
+check_module_count_diff(0)
+ | ---
+ | ...
+box.schema.func.drop('function1')
+ | ---
+ | ...
+check_module_count_diff(0)
+ | ---
+ | ...
+
+-- Module is unloaded when its function is dropped, and there are
+-- no not finished invocations of the function.
+box.schema.func.create('function1', {language = 'C'})
+ | ---
+ | ...
+check_module_count_diff(0)
+ | ---
+ | ...
+box.func.function1:call()
+ | ---
+ | ...
+check_module_count_diff(1)
+ | ---
+ | ...
+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.
+box.schema.func.create('function1', {language = 'C'})
+ | ---
+ | ...
+box.schema.func.create('function1.test_sleep', {language = 'C'})
+ | ---
+ | ...
+check_module_count_diff(0)
+ | ---
+ | ...
+
+function long_call() box.func['function1.test_sleep']:call() end
+ | ---
+ | ...
+f1 = fiber.create(long_call)
+ | ---
+ | ...
+f2 = fiber.create(long_call)
+ | ---
+ | ...
+test_run:wait_cond(function()                                   \
+    return f1:status() == 'suspended' and                       \
+        f2:status() == 'suspended'                              \
+end)
+ | ---
+ | - true
+ | ...
+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)
+ | ---
+ | ...
+
+f1:wakeup()
+ | ---
+ | ...
+test_run:wait_cond(function() return f1:status() == 'dead' end)
+ | ---
+ | - true
+ | ...
+check_module_count_diff(0)
+ | ---
+ | ...
+
+f2:wakeup()
+ | ---
+ | ...
+test_run:wait_cond(function() return f2:status() == 'dead' end)
+ | ---
+ | - true
+ | ...
+check_module_count_diff(-1)
+ | ---
+ | ...
diff --git a/test/box/gh-4648-func-load-unload.test.lua b/test/box/gh-4648-func-load-unload.test.lua
new file mode 100644
index 000000000..d7e445f65
--- /dev/null
+++ b/test/box/gh-4648-func-load-unload.test.lua
@@ -0,0 +1,63 @@
+fiber = require('fiber')
+test_run = require('test_run').new()
+build_path = os.getenv("BUILDDIR")
+package.cpath = build_path..'/test/box/?.so;'..build_path..'/test/box/?.dylib;'..package.cpath
+errinj = box.error.injection
+
+--
+-- gh-4648: box.schema.func.drop() didn't unload a .so/.dylib
+-- module. Even if it was unused already. Moreover, recreation of
+-- functions from the same module led to its multiple mmaping.
+--
+
+current_module_count = errinj.get("ERRINJ_DYN_MODULE_COUNT")
+function check_module_count_diff(diff)                          \
+    local module_count = errinj.get("ERRINJ_DYN_MODULE_COUNT")  \
+    current_module_count = current_module_count + diff          \
+    if current_module_count ~= module_count then                \
+        return current_module_count, module_count               \
+    end                                                         \
+end
+
+-- Module is not loaded until any of its functions is called first
+-- time.
+box.schema.func.create('function1', {language = 'C'})
+check_module_count_diff(0)
+box.schema.func.drop('function1')
+check_module_count_diff(0)
+
+-- Module is unloaded when its function is dropped, and there are
+-- no not finished invocations of the function.
+box.schema.func.create('function1', {language = 'C'})
+check_module_count_diff(0)
+box.func.function1:call()
+check_module_count_diff(1)
+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.
+box.schema.func.create('function1', {language = 'C'})
+box.schema.func.create('function1.test_sleep', {language = 'C'})
+check_module_count_diff(0)
+
+function long_call() box.func['function1.test_sleep']:call() end
+f1 = fiber.create(long_call)
+f2 = fiber.create(long_call)
+test_run:wait_cond(function()                                   \
+    return f1:status() == 'suspended' and                       \
+        f2:status() == 'suspended'                              \
+end)
+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)
+
+f1:wakeup()
+test_run:wait_cond(function() return f1:status() == 'dead' end)
+check_module_count_diff(0)
+
+f2:wakeup()
+test_run:wait_cond(function() return f2:status() == 'dead' end)
+check_module_count_diff(-1)
diff --git a/test/box/suite.ini b/test/box/suite.ini
index 6e8508188..9aa30ede6 100644
--- a/test/box/suite.ini
+++ b/test/box/suite.ini
@@ -3,7 +3,7 @@ core = tarantool
 description = Database tests
 script = box.lua
 disabled = rtree_errinj.test.lua tuple_bench.test.lua
-release_disabled = errinj.test.lua errinj_index.test.lua rtree_errinj.test.lua upsert_errinj.test.lua iproto_stress.test.lua
+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
 lua_libs = lua/fifo.lua lua/utils.lua lua/bitset.lua lua/index_random_test.lua lua/push.lua lua/identifier.lua
 use_unix_sockets = True
 use_unix_sockets_iproto = True
-- 
2.21.0 (Apple Git-122.2)

Re: [Tarantool-patches] [PATCH 1/2] errinj: provide 'get' method in Lua

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/11/27 11:15]:
> Error injections are used to simulate an error. They are
> represented as a flag, or a number, and are used in Lua tests. But
> they don't have any feedback. That makes impossible to use the
> injections to check that something has happened. Something very
> needed to be checked, and impossible to check in a different way.
> 
> More certainly, the patch is motivated by a necessity to count
> loaded dynamic libraries to ensure, that they are loaded and
> unloaded when expected. This is impossible to do in a platform
> independent way. But an error injection as a debug-only counter
> would solve the problem.

lgtm


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 2/2] func: fix not unloading of unused modules

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/11/27 11:15]:
> C functions are loaded from .so/.dylib dynamic libraries. A
> library is loaded when any function from there is called first
> time. And was supposed to be unloaded, when all its functions are
> dropped from the schema (box.schema.func.drop()), and none of them
> is still in a call. But the unloading part was broken.

Wow. This is damn genius!

It never occurred to me that error injections could be used to
test some obscure code paths.

The impact of this on testing is going to be huge.
>  module_gc(struct module *module)
>  {
> -	if (!module->is_unloading || !rlist_empty(&module->funcs) ||
> -	     module->calls != 0)
> -		return;
> -	module_delete(module);
> +	if (rlist_empty(&module->funcs) && module->calls == 0)
> +		module_delete(module);
>  }

This is LGTM. Georgy wrote the original patches so you may want
to solicit his review as well.
> +f1:wakeup()
> + | ---
> + | ...
> +test_run:wait_cond(function() return f1:status() == 'dead' end)
> + | ---
> + | - true
> + | ...
> +check_module_count_diff(0)
> + | ---
> + | ...
> +
> +f2:wakeup()

This is a bit hackish way to cancel a sleep. I thought we had
errinj_sleep injection which would reliably cancel a sleep when
you clear it? If there is no such function I don't think 
it is a blocker for this patch to get in as is.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 2/2] func: fix not unloading of unused modules

[Vladislav Shpilevoy]

Hi! Thanks for the review!

On 27/11/2019 09:41, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/11/27 11:15]:
>> C functions are loaded from .so/.dylib dynamic libraries. A
>> library is loaded when any function from there is called first
>> time. And was supposed to be unloaded, when all its functions are
>> dropped from the schema (box.schema.func.drop()), and none of them
>> is still in a call. But the unloading part was broken.
> 
> Wow. This is damn genius!
> 
> It never occurred to me that error injections could be used to
> test some obscure code paths.
> 
> The impact of this on testing is going to be huge.
>>  module_gc(struct module *module)
>>  {
>> -	if (!module->is_unloading || !rlist_empty(&module->funcs) ||
>> -	     module->calls != 0)
>> -		return;
>> -	module_delete(module);
>> +	if (rlist_empty(&module->funcs) && module->calls == 0)
>> +		module_delete(module);
>>  }
> 
> This is LGTM. Georgy wrote the original patches so you may want
> to solicit his review as well.
>> +f1:wakeup()
>> + | ---
>> + | ...
>> +test_run:wait_cond(function() return f1:status() == 'dead' end)
>> + | ---
>> + | - true
>> + | ...
>> +check_module_count_diff(0)
>> + | ---
>> + | ...
>> +
>> +f2:wakeup()
> 
> This is a bit hackish way to cancel a sleep. I thought we had
> errinj_sleep injection which would reliably cancel a sleep when
> you clear it? If there is no such function I don't think 
> it is a blocker for this patch to get in as is.
> 

The sleeping function is in a C module, and can use only public
API from module.h. So we can't use error injections in it. But
the sleep can be dropped in a different way. I pushed the diff
below on the branch:

====================================================================

diff --git a/test/box/function1.c b/test/box/function1.c
index fd434c280..87062d6a8 100644
--- a/test/box/function1.c
+++ b/test/box/function1.c
@@ -237,10 +237,11 @@ test_sleep(box_function_ctx_t *ctx, const char *args, const char *args_end)
 	(void) args;
 	(void) args_end;
 	/*
-	 * Sleep until an explicit wakeup. Purpose of this
-	 * function - test module unloading prevention while at
-	 * least one of its functions is being executed.
+	 * Sleep until a cancellation. Purpose of this function -
+	 * test module unloading prevention while at least one of
+	 * its functions is being executed.
 	 */
-	fiber_sleep(100000);
+	while (!fiber_is_cancelled())
+		fiber_sleep(0);
 	return 0;
 }
diff --git a/test/box/gh-4648-func-load-unload.result b/test/box/gh-4648-func-load-unload.result
index 8c4d23938..e695dd365 100644
--- a/test/box/gh-4648-func-load-unload.result
+++ b/test/box/gh-4648-func-load-unload.result
@@ -114,7 +114,7 @@ check_module_count_diff(0)
  | ---
  | ...
 
-f1:wakeup()
+f1:cancel()
  | ---
  | ...
 test_run:wait_cond(function() return f1:status() == 'dead' end)
@@ -125,7 +125,7 @@ check_module_count_diff(0)
  | ---
  | ...
 
-f2:wakeup()
+f2:cancel()
  | ---
  | ...
 test_run:wait_cond(function() return f2:status() == 'dead' end)
diff --git a/test/box/gh-4648-func-load-unload.test.lua b/test/box/gh-4648-func-load-unload.test.lua
index d7e445f65..10b9de87a 100644
--- a/test/box/gh-4648-func-load-unload.test.lua
+++ b/test/box/gh-4648-func-load-unload.test.lua
@@ -54,10 +54,10 @@ box.schema.func.drop('function1')
 box.schema.func.drop('function1.test_sleep')
 check_module_count_diff(0)
 
-f1:wakeup()
+f1:cancel()
 test_run:wait_cond(function() return f1:status() == 'dead' end)
 check_module_count_diff(0)
 
-f2:wakeup()
+f2:cancel()
 test_run:wait_cond(function() return f2:status() == 'dead' end)
 check_module_count_diff(-1)

Re: [Tarantool-patches] [PATCH 0/2] Module is not unloaded on function drop

[Kirill Yukhin]

Hello,

On 27 ноя 00:29, Vladislav Shpilevoy wrote:
> The patchset fixes a problem when a C function, added via
> box.schema.func.create(), is dropped, but its dynamic library is not unloaded
> from the process' memory.
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4648-func-unload
> Issue: https://github.com/tarantool/tarantool/issues/4648
> 
> Vladislav Shpilevoy (2):
>   errinj: provide 'get' method in Lua
>   func: fix not unloading of unused modules

I've checked your patchset into 1.10, 2.2 and master.

--
Regards, Kirill Yukhin

Re: [Tarantool-patches] [PATCH 0/2] Module is not unloaded on function drop

[Alexander Turenko]

On Tue, Dec 10, 2019 at 05:22:04PM +0300, Kirill Yukhin wrote:
> Hello,
> 
> On 27 ноя 00:29, Vladislav Shpilevoy wrote:
> > The patchset fixes a problem when a C function, added via
> > box.schema.func.create(), is dropped, but its dynamic library is not unloaded
> > from the process' memory.
> > 
> > Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4648-func-unload
> > Issue: https://github.com/tarantool/tarantool/issues/4648
> > 
> > Vladislav Shpilevoy (2):
> >   errinj: provide 'get' method in Lua
> >   func: fix not unloading of unused modules
> 
> I've checked your patchset into 1.10, 2.2 and master.

It seems you don't pushed it to 1.10.

When you'll do, please, apply also
eddc8cc03bc8a00c4f2edfc9782910b88b0fdd33 ('test: update
box/errinj.test.lua result file').

Re: [Tarantool-patches] [PATCH 0/2] Module is not unloaded on function drop

[Vladislav Shpilevoy]

Hi!

On 10/12/2019 15:22, Kirill Yukhin wrote:
> Hello,
> 
> On 27 ноя 00:29, Vladislav Shpilevoy wrote:
>> The patchset fixes a problem when a C function, added via
>> box.schema.func.create(), is dropped, but its dynamic library is not unloaded
>> from the process' memory.
>>
>> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4648-func-unload
>> Issue: https://github.com/tarantool/tarantool/issues/4648
>>
>> Vladislav Shpilevoy (2):
>>   errinj: provide 'get' method in Lua
>>   func: fix not unloading of unused modules
> 
> I've checked your patchset into 1.10, 2.2 and master.

The patchset was not pushed to 1.10. Because you wouldn't be
able to backport it, because 1.10 misses some functionality
uses in the test. I ported it. See the branch:

    gerold103/gh-4648-func-unload-1.10

Also I've taken into account the broken box/errinj.test.lua.
On the branch it is fixed.

> 
> --
> Regards, Kirill Yukhin
> 

Re: [Tarantool-patches] [PATCH 0/2] Module is not unloaded on function drop

[Kirill Yukhin]

Hello,

On 27 ноя 00:29, Vladislav Shpilevoy wrote:
> The patchset fixes a problem when a C function, added via
> box.schema.func.create(), is dropped, but its dynamic library is not unloaded
> from the process' memory.
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4648-func-unload
> Issue: https://github.com/tarantool/tarantool/issues/4648

I've checked your patchset into 1.10, 2.2 and master.

--
Regards, Kirill Yukhin