From ffc9cee400cd3f067b25f1797027eebacaa57c65 Mon Sep 17 00:00:00 2001
From: Aleksandr Lyapunov <alyapunov@tarantool.org>
Date: Mon, 22 Nov 2021 17:31:16 +0300
Subject: [PATCH] box: add pin/unping infrastructure for func cache
There are cases when we need to be sure that a function is not
deleted and/or removed from func cache. For example constraints:
they must hold a pointer to struct func while it's very hard to
determine whether there'a constraint that points to given func.
Implement func pin/unpin for this purpose. You can pin a func to
declare that the func must not be deleted. To have to unpin it
when the func is not needed anymore.
NO_DOC=refactoring
NO_CHANGELOG=refactoring
---
src/box/alter.cc | 11 ++
src/box/func.c | 2 +
src/box/func.h | 2 +
src/box/func_cache.c | 47 ++++++++-
src/box/func_cache.h | 60 +++++++++++
src/box/sql/func.c | 1 +
test/unit/CMakeLists.txt | 2 +
test/unit/func_cache.c | 203 ++++++++++++++++++++++++++++++++++++
test/unit/func_cache.result | 53 ++++++++++
9 files changed, 379 insertions(+), 2 deletions(-)
create mode 100644 test/unit/func_cache.c
create mode 100644 test/unit/func_cache.result
diff --git a/src/box/alter.cc b/src/box/alter.cc
index bfd70dc2b3..774d5a9a25 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3635,6 +3635,17 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
"function has references");
return -1;
}
+ /* Check whether old_func is used somewhere. */
+ enum func_holder_type pinned_type;
+ if (func_is_pinned(old_func, &pinned_type)) {
+ const char *type_str =
+ func_cache_holder_type_strs[pinned_type];
+ diag_set(ClientError, ER_DROP_FUNCTION,
+ (unsigned int)old_func->def->uid,
+ tt_sprintf("function is referenced by %s",
+ type_str));
+ return -1;
+ }
struct trigger *on_commit =
txn_alter_trigger_new(on_drop_func_commit, old_func);
struct trigger *on_rollback =
diff --git a/src/box/func.c b/src/box/func.c
index e7a368b2f2..a43add36c8 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -399,6 +399,7 @@ func_new(struct func_def *def)
if (func == NULL)
return NULL;
func->def = def;
+ rlist_create(&func->func_cache_pin_list);
/** Nobody has access to the function but the owner. */
memset(func->access, 0, sizeof(func->access));
/*
@@ -500,6 +501,7 @@ static struct func_vtab func_c_vtab = {
void
func_delete(struct func *func)
{
+ assert(rlist_empty(&func->func_cache_pin_list));
struct func_def *def = func->def;
credentials_destroy(&func->owner_credentials);
func->vtab->destroy(func);
diff --git a/src/box/func.h b/src/box/func.h
index a8a6932689..b0e5dafe60 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -60,6 +60,8 @@ struct func {
struct func_def *def;
/** Virtual method table. */
const struct func_vtab *vtab;
+ /** List of func holders. This member is a property of func cache. */
+ struct rlist func_cache_pin_list;
/**
* Authentication id of the owner of the function,
* used for set-user-id functions.
diff --git a/src/box/func_cache.c b/src/box/func_cache.c
index 38f09e0cc1..6657bfe9fd 100644
--- a/src/box/func_cache.c
+++ b/src/box/func_cache.c
@@ -13,6 +13,10 @@ static struct mh_i32ptr_t *funcs;
/** Name -> func dictionary. */
static struct mh_strnptr_t *funcs_by_name;
+const char *func_cache_holder_type_strs[FUNC_HOLDER_MAX] = {
+ "constraint",
+};
+
void
func_cache_init(void)
{
@@ -55,8 +59,8 @@ func_cache_delete(uint32_t fid)
mh_int_t k = mh_i32ptr_find(funcs, fid, NULL);
if (k == mh_end(funcs))
return;
- struct func *func = (struct func *)
- mh_i32ptr_node(funcs, k)->val;
+ struct func *func = (struct func *)mh_i32ptr_node(funcs, k)->val;
+ assert(rlist_empty(&func->func_cache_pin_list));
mh_i32ptr_del(funcs, k, NULL);
k = mh_strnptr_find_str(funcs_by_name, func->def->name,
strlen(func->def->name));
@@ -81,3 +85,42 @@ func_by_name(const char *name, uint32_t name_len)
return NULL;
return (struct func *)mh_strnptr_node(funcs_by_name, func)->val;
}
+
+void
+func_pin(struct func *func, struct func_cache_holder *holder,
+ enum func_holder_type type)
+{
+ assert(func_by_id(func->def->fid) != NULL);
+ holder->func = func;
+ holder->type = type;
+ rlist_add_tail(&func->func_cache_pin_list, &holder->link);
+}
+
+void
+func_unpin(struct func_cache_holder *holder)
+{
+ assert(func_by_id(holder->func->def->fid) != NULL);
+#ifndef NDEBUG
+ /* Paranoid check that the func is pinned by holder. */
+ bool is_in_list = false;
+ struct rlist *tmp;
+ rlist_foreach(tmp, &holder->func->func_cache_pin_list)
+ is_in_list = is_in_list || tmp == &holder->link;
+ assert(is_in_list);
+#endif
+ rlist_del(&holder->link);
+ holder->func = NULL;
+}
+
+bool
+func_is_pinned(struct func *func, enum func_holder_type *type)
+{
+ assert(func_by_id(func->def->fid) != NULL);
+ if (rlist_empty(&func->func_cache_pin_list))
+ return false;
+ struct func_cache_holder *h =
+ rlist_first_entry(&func->func_cache_pin_list,
+ struct func_cache_holder, link);
+ *type = h->type;
+ return true;
+}
diff --git a/src/box/func_cache.h b/src/box/func_cache.h
index 72fcca09ac..b022c10e54 100644
--- a/src/box/func_cache.h
+++ b/src/box/func_cache.h
@@ -14,6 +14,36 @@ extern "C" {
struct func;
+/**
+ * Type of a holder that can pin a func. @sa struct func_cache_holder.
+ */
+enum func_holder_type {
+ FUNC_HOLDER_CONSTRAINT,
+ FUNC_HOLDER_MAX,
+};
+
+/**
+ * Lowercase name of each type.
+ */
+extern const char *func_cache_holder_type_strs[FUNC_HOLDER_MAX];
+
+/**
+ * Definition of a holder that pinned some func. Pinning of a func is
+ * a mechanism that is designed for preventing of deletion of some func from
+ * func cache by storing links to holders that prevented that.
+ */
+struct func_cache_holder {
+ /** Holders of the same func are linked into ring list by this link. */
+ struct rlist link;
+ /** Actual pointer to func. */
+ struct func *func;
+ /**
+ * Type of holder, mostly for better error generation, but also can be
+ * used for proper container_of application.
+ */
+ enum func_holder_type type;
+};
+
/**
* Initialize function cache storage.
*/
@@ -35,6 +65,9 @@ func_cache_insert(struct func *func);
/**
* Delete a function object from the function cache.
+ * The function must not have any keepers (assert, @sa func_cache_is_kept),
+ * so if there is no assurance that there are no pins, @sa func_cache_is_pinned
+ * must be called before.
* If the function is not found by five ID - do nothing.
* @param fid ID of function object.
*/
@@ -56,6 +89,33 @@ func_by_id(uint32_t fid);
struct func *
func_by_name(const char *name, uint32_t name_len);
+/**
+ * Register that there is a @a holder of type @a type that is dependent
+ * on function @a func.
+ * The function must be in cache (asserted).
+ * If a function has holders, it must not be deleted (asserted).
+ */
+void
+func_pin(struct func *func, struct func_cache_holder *holder,
+ enum func_holder_type type);
+
+/**
+ * Notify that a @a holder does not depend anymore on function.
+ * The function must be in cache (asserted).
+ * If a function has no holders, it can be deleted.
+ */
+void
+func_unpin(struct func_cache_holder *holder);
+
+/**
+ * Check whether the function @a func has holders or not.
+ * If it has, @a type argument is set to the first holder's type.
+ * The function must be in cache (asserted).
+ * If a function has holders, it must not be deleted (asserted).
+ */
+bool
+func_is_pinned(struct func *func, enum func_holder_type *type);
+
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined(__cplusplus) */
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index df1fcd9795..2d701d2824 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2149,6 +2149,7 @@ sql_built_in_functions_cache_init(void)
panic("Out of memory on creating SQL built-in");
func->base.def = def;
+ rlist_create(&func->base.func_cache_pin_list);
func->base.vtab = &func_sql_builtin_vtab;
credentials_create_empty(&func->base.owner_credentials);
memset(func->base.access, 0, sizeof(func->base.access));
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index 16e1c4d0e1..1f0eee4f71 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -92,6 +92,8 @@ add_executable(fiber.test fiber.cc core_test_utils.c)
target_link_libraries(fiber.test core unit)
add_executable(fiber_stack.test fiber_stack.c core_test_utils.c)
target_link_libraries(fiber_stack.test core unit)
+add_executable(func_cache.test func_cache.c box_test_utils.c)
+target_link_libraries(func_cache.test box unit)
if (NOT ENABLE_GCOV)
# This test is known to be broken with GCOV
diff --git a/test/unit/func_cache.c b/test/unit/func_cache.c
new file mode 100644
index 0000000000..3b9892cf18
--- /dev/null
+++ b/test/unit/func_cache.c
@@ -0,0 +1,203 @@
+#include "func_cache.h"
+#include "func.h"
+#include "trivia/util.h"
+#include "unit.h"
+
+int status;
+
+struct func *
+test_func_new(uint32_t id, const char *name)
+{
+ uint32_t name_len = strlen(name);
+ struct func_def *def = xmalloc(offsetof(struct func_def,
+ name[name_len + 1]));
+ def->fid = id;
+ strcpy(def->name, name);
+ def->name_len = strlen(name);
+ struct func *f = xmalloc(sizeof(*f));
+ f->def = def;
+ rlist_create(&f->func_cache_pin_list);
+ return f;
+}
+
+static void
+test_func_delete(struct func *f)
+{
+ free(f->def);
+ free(f);
+}
+
+/**
+ * Test that pin/is_pinned/unpin works fine with one func and one holder.
+ */
+static void
+func_cache_pin_test_one_holder(void)
+{
+ header();
+ plan(7);
+
+ func_cache_init();
+ struct func *f1 = test_func_new(1, "func1");
+ enum func_holder_type type;
+ struct func_cache_holder h1;
+
+ func_cache_insert(f1);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_pin(f1, &h1, 1);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1, "ok");
+ func_unpin(&h1);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_pin(f1, &h1, 1);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1, "ok");
+ func_unpin(&h1);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_cache_delete(f1->def->fid);
+
+ test_func_delete(f1);
+ func_cache_destroy();
+
+ footer();
+ status |= check_plan();
+}
+
+/**
+ * Test several holders that pin/unpins one func in FIFO order.
+ */
+static void
+func_cache_pin_test_fifo(void)
+{
+ header();
+ plan(8);
+
+ func_cache_init();
+ struct func *f1 = test_func_new(1, "func1");
+ enum func_holder_type type;
+ struct func_cache_holder h1, h2;
+
+ func_cache_insert(f1);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_pin(f1, &h1, 1);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1, "ok");
+ func_pin(f1, &h2, 2);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1 || type == 2, "ok");
+ func_unpin(&h1);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 2, "ok");
+ func_unpin(&h2);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_cache_delete(f1->def->fid);
+
+ test_func_delete(f1);
+ func_cache_destroy();
+
+ footer();
+ status |= check_plan();
+}
+
+/**
+ * Test several holders that pin/unpins one func in LIFO order.
+ */
+static void
+func_cache_pin_test_lifo(void)
+{
+ header();
+ plan(8);
+
+ func_cache_init();
+ struct func *f1 = test_func_new(1, "func1");
+ enum func_holder_type type;
+ struct func_cache_holder h1, h2;
+
+ func_cache_insert(f1);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_pin(f1, &h1, 1);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1, "ok");
+ func_pin(f1, &h2, 2);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1 || type == 2, "ok");
+ func_unpin(&h2);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1, "ok");
+ func_unpin(&h1);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_cache_delete(f1->def->fid);
+
+ test_func_delete(f1);
+ func_cache_destroy();
+
+ footer();
+ status |= check_plan();
+}
+
+/**
+ * Test several holders with several funcs.
+ */
+static void
+func_cache_pin_test_several(void)
+{
+ header();
+ plan(18);
+
+ func_cache_init();
+ struct func *f1 = test_func_new(1, "func1");
+ struct func *f2 = test_func_new(2, "func2");
+ enum func_holder_type type;
+ struct func_cache_holder h1, h2, h3;
+
+ func_cache_insert(f1);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_pin(f1, &h1, 1);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1, "ok");
+
+ func_cache_insert(f2);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(!func_is_pinned(f2, &type), "ok");
+
+ func_pin(f1, &h2, 2);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(!func_is_pinned(f2, &type), "ok");
+
+ func_pin(f2, &h3, 3);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 1 || type == 2, "ok");
+ ok(func_is_pinned(f2, &type), "ok");
+ ok(type == 3, "ok");
+
+ func_unpin(&h1);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(type == 2, "ok");
+ ok(func_is_pinned(f2, &type), "ok");
+ ok(type == 3, "ok");
+
+ func_unpin(&h3);
+ ok(func_is_pinned(f1, &type), "ok");
+ ok(!func_is_pinned(f2, &type), "ok");
+ func_cache_delete(f2->def->fid);
+
+ func_unpin(&h2);
+ ok(!func_is_pinned(f1, &type), "ok");
+ func_cache_delete(f1->def->fid);
+
+ test_func_delete(f1);
+ test_func_delete(f2);
+ func_cache_destroy();
+
+ footer();
+ status |= check_plan();
+}
+
+int
+main(void)
+{
+ func_cache_pin_test_one_holder();
+ func_cache_pin_test_fifo();
+ func_cache_pin_test_lifo();
+ func_cache_pin_test_several();
+ return status;
+}
diff --git a/test/unit/func_cache.result b/test/unit/func_cache.result
new file mode 100644
index 0000000000..f038c5578d
--- /dev/null
+++ b/test/unit/func_cache.result
@@ -0,0 +1,53 @@
+ *** func_cache_pin_test_one_holder ***
+1..7
+ok 1 - ok
+ok 2 - ok
+ok 3 - ok
+ok 4 - ok
+ok 5 - ok
+ok 6 - ok
+ok 7 - ok
+ *** func_cache_pin_test_one_holder: done ***
+ *** func_cache_pin_test_fifo ***
+1..8
+ok 1 - ok
+ok 2 - ok
+ok 3 - ok
+ok 4 - ok
+ok 5 - ok
+ok 6 - ok
+ok 7 - ok
+ok 8 - ok
+ *** func_cache_pin_test_fifo: done ***
+ *** func_cache_pin_test_lifo ***
+1..8
+ok 1 - ok
+ok 2 - ok
+ok 3 - ok
+ok 4 - ok
+ok 5 - ok
+ok 6 - ok
+ok 7 - ok
+ok 8 - ok
+ *** func_cache_pin_test_lifo: done ***
+ *** func_cache_pin_test_several ***
+1..18
+ok 1 - ok
+ok 2 - ok
+ok 3 - ok
+ok 4 - ok
+ok 5 - ok
+ok 6 - ok
+ok 7 - ok
+ok 8 - ok
+ok 9 - ok
+ok 10 - ok
+ok 11 - ok
+ok 12 - ok
+ok 13 - ok
+ok 14 - ok
+ok 15 - ok
+ok 16 - ok
+ok 17 - ok
+ok 18 - ok
+ *** func_cache_pin_test_several: done ***
--
GitLab