diff --git a/src/box/func.c b/src/box/func.c index d9481b714dcacb21019dc01c970cd1281f765ed0..431577127fe0b92413d952cac0198e3b67ffe718 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 @@ module_load(const char *package, const char *package_end) 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 18b83faac34cb709203b01e714d17f0a19be0cf9..581e468cba69d86ea3ed1511fb85b96012e62695 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 3072a00ea62d230f59a45ebb50f96b216fcbadc1..672da21199b8592cbeea9472b9e66072e6fd1a04 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 b0d983e2b5b962aad1fc4d06a5e5f99fe4950729..87062d6a8f251fb226c559d571efa7367fab3ab6 100644 --- a/test/box/function1.c +++ b/test/box/function1.c @@ -229,3 +229,19 @@ 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 a cancellation. Purpose of this function - + * test module unloading prevention while at least one of + * its functions is being executed. + */ + 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 new file mode 100644 index 0000000000000000000000000000000000000000..e695dd365d75dfc04e835df28db14f35f55dcd14 --- /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:cancel() + | --- + | ... +test_run:wait_cond(function() return f1:status() == 'dead' end) + | --- + | - true + | ... +check_module_count_diff(0) + | --- + | ... + +f2:cancel() + | --- + | ... +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 0000000000000000000000000000000000000000..10b9de87ac15373b910a55010c1e3e18a4d231a8 --- /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:cancel() +test_run:wait_cond(function() return f1:status() == 'dead' end) +check_module_count_diff(0) + +f2:cancel() +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 6e8508188996ccf26c2bb3cc64633fc3148d1d42..9aa30ede686442aaab602a21779a0e3faa6b86b2 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