[Tarantool-patches] [PATCH v10 1/4] box/func: factor out c function entry structure
Cyrill Gorcunov
gorcunov at gmail.com
Thu Nov 5 18:18:05 MSK 2020
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.
While been factoring out Vlad reported that we've an issue:
if a module has been loaded for a first time but doesn't have
symbol requested it will remain in memory keeping a reference
to a shared library for nothing.
Fixes #5475
Part-of #4642
Reported-by: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
---
src/box/func.c | 172 +++++++++++++++++++++++++------------------------
src/box/func.h | 42 ++++++++++++
2 files changed, 131 insertions(+), 83 deletions(-)
diff --git a/src/box/func.c b/src/box/func.c
index 8087c953f..36bfca33f 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,70 @@ 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 may continue being referenced even
+ * if noone gonna use 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 +438,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 +461,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 +471,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,79 +538,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 *module = module_cache_find(name.package,
- name.package_end);
- if (module == NULL) {
- /* Try to find loaded module in the cache */
- module = module_load(name.package, name.package_end);
- if (module == NULL)
- return -1;
- if (module_cache_put(module)) {
- module_delete(module);
- return -1;
- }
- }
-
- func->func = module_sym(module, name.sym);
- if (func->func == NULL)
- 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;
}
@@ -571,10 +577,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.26.2
More information about the Tarantool-patches
mailing list