From 088d45eb181578f882b4e30b3d197523cbdb28a6 Mon Sep 17 00:00:00 2001
From: Ilya Verbin <iverbin@tarantool.org>
Date: Wed, 5 Jul 2023 20:26:31 +0300
Subject: [PATCH] box: reference collation by key_def parts
It was possible to delete a collation, which is in use by a key_def.
Part of #4544
NO_DOC=bugfix
NO_CHANGELOG=next commit
(cherry picked from commit 07beb340a9eb6a9f38699836cfce5002e807466e)
---
src/box/index_def.c | 13 ++--
src/box/key_def.c | 9 +++
src/box/lua/key_def.c | 15 ++++-
.../gh_4544_collation_drop_test.lua | 62 +++++++++++++++++++
4 files changed, 88 insertions(+), 11 deletions(-)
create mode 100644 test/engine-luatest/gh_4544_collation_drop_test.lua
diff --git a/src/box/index_def.c b/src/box/index_def.c
index 5dbc5d8e98..458ebcaecd 100644
--- a/src/box/index_def.c
+++ b/src/box/index_def.c
@@ -204,22 +204,17 @@ index_stat_dup(const struct index_stat *src)
return dup;
}
-/** Free a key definition. */
void
index_def_delete(struct index_def *index_def)
{
index_opts_destroy(&index_def->opts);
free(index_def->name);
- if (index_def->key_def) {
- TRASH(index_def->key_def);
- free(index_def->key_def);
- }
+ if (index_def->key_def)
+ key_def_delete(index_def->key_def);
- if (index_def->cmp_def) {
- TRASH(index_def->cmp_def);
- free(index_def->cmp_def);
- }
+ if (index_def->cmp_def)
+ key_def_delete(index_def->cmp_def);
TRASH(index_def);
free(index_def);
diff --git a/src/box/key_def.c b/src/box/key_def.c
index 0de7edc438..69386321b7 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -121,6 +121,9 @@ key_def_copy_impl(struct key_def *res, const struct key_def *src, size_t sz)
size_t path_offset = src->multikey_path - (char *)src;
res->multikey_path = (char *)res + path_offset;
}
+ for (uint32_t i = 0; i < res->part_count; i++)
+ if (res->parts[i].coll != NULL)
+ coll_ref(res->parts[i].coll);
return res;
}
@@ -144,6 +147,10 @@ key_def_dup(const struct key_def *src)
void
key_def_delete(struct key_def *def)
{
+ for (uint32_t i = 0; i < def->part_count; i++)
+ if (def->parts[i].coll != NULL)
+ coll_unref(def->parts[i].coll);
+ TRASH(def);
free(def);
}
@@ -247,6 +254,8 @@ key_def_set_part(struct key_def *def, uint32_t part_no, uint32_t fieldno,
def->parts[part_no].fieldno = fieldno;
def->parts[part_no].type = type;
def->parts[part_no].coll = coll;
+ if (coll != NULL)
+ coll_ref(def->parts[part_no].coll);
def->parts[part_no].coll_id = coll_id;
def->parts[part_no].sort_order = sort_order;
def->parts[part_no].offset_slot_cache = offset_slot;
diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index e5fc370e49..375d18f227 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -68,9 +68,20 @@ luaT_push_key_def(struct lua_State *L, const struct key_def *key_def)
}
if (part->coll_id != COLL_NONE) {
+ const char *name;
struct coll_id *coll_id = coll_by_id(part->coll_id);
- assert(coll_id != NULL);
- lua_pushstring(L, coll_id->name);
+ /*
+ * It is possible that the original collation with
+ * `id == part->coll_id` was replaced by the new one
+ * with the same id, but a different fingerprint.
+ * However, it their fingerprints match, then coll_new()
+ * reuses `struct coll` and this condition is met:
+ */
+ if (coll_id != NULL && coll_id->coll == part->coll)
+ name = coll_id->name;
+ else
+ name = "<deleted>";
+ lua_pushstring(L, name);
lua_setfield(L, -2, "collation");
}
lua_rawseti(L, -2, i + 1);
diff --git a/test/engine-luatest/gh_4544_collation_drop_test.lua b/test/engine-luatest/gh_4544_collation_drop_test.lua
new file mode 100644
index 0000000000..4f4f532405
--- /dev/null
+++ b/test/engine-luatest/gh_4544_collation_drop_test.lua
@@ -0,0 +1,62 @@
+local t = require('luatest')
+
+local function before_all(cg)
+ local server = require('luatest.server')
+ cg.server = server:new()
+ cg.server:start()
+end
+
+local function after_all(cg)
+ cg.server:drop()
+end
+
+local g1 = t.group('gh-4544-1')
+g1.before_all(before_all)
+g1.after_all(after_all)
+
+-- Check that key_def compare doesn't crash after collation drop.
+g1.test_keydef_crash = function(cg)
+ cg.server:exec(function()
+ local key_def = require('key_def')
+ local coll_name = 'unicode_af_s1'
+ local kd = key_def.new({{field = 1, type = 'string',
+ collation = coll_name}})
+ box.internal.collation.drop(coll_name)
+ kd:compare({'a'}, {'b'})
+ t.assert_equals(kd:totable()[1].collation, '<deleted>')
+ end)
+end
+
+-- Replace old collation, which is used by key_def, by the new one with the same
+-- fingerprint.
+g1.test_keydef_replace_coll_same = function(cg)
+ cg.server:exec(function()
+ local key_def = require('key_def')
+ local old_name = 'my old coll'
+ local new_name = 'my new coll'
+ box.internal.collation.create(old_name, 'ICU', '', {})
+ local kd = key_def.new({{field = 1, type = 'string',
+ collation = old_name}})
+ box.internal.collation.drop(old_name)
+ box.internal.collation.create(new_name, 'ICU', '', {})
+ t.assert_equals(kd:totable()[1].collation, new_name)
+ box.internal.collation.drop(new_name)
+ end)
+end
+
+-- Replace old collation, which is used by key_def, by the new one with
+-- different fingerprint.
+g1.test_keydef_replace_coll_different = function(cg)
+ cg.server:exec(function()
+ local key_def = require('key_def')
+ local old_name = 'my old coll'
+ local new_name = 'my new coll'
+ box.internal.collation.create(old_name, 'ICU', '', {})
+ local kd = key_def.new({{field = 1, type = 'string',
+ collation = old_name}})
+ box.internal.collation.drop(old_name)
+ box.internal.collation.create(new_name, 'ICU', 'ru-RU', {})
+ t.assert_equals(kd:totable()[1].collation, '<deleted>')
+ box.internal.collation.drop(new_name)
+ end)
+end
--
GitLab