From c1f307125337c704762865163518b4a4539077d7 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Tue, 8 Sep 2026 13:53:59 +0300 Subject: [PATCH 1/4] ci: bump luatest to 1.4.5 --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 171933f..7dbbe86 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -113,7 +113,7 @@ jobs: tt version - name: Install luatest - run: tt rocks install luatest 1.4.0 + run: tt rocks install luatest 1.4.5 - name: Generate make files run: PATH="$PATH:$(pwd)/.rocks/bin" cmake -S . -B build From 12dff0458d15a69ffc4cc5108fc3361a78bf9dae Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Tue, 8 Sep 2026 11:21:36 +0300 Subject: [PATCH 2/4] Remove graceful OOM handling Reasons to drop it: - Tarantool panics on any malloc failure so there's no need to gracefully handle OOM here. - We don't test the code handling OOM (there's no easy way to test it). - We store a reference to the OOM error in a global variable, which means we must make it thread-local to fix #9. If we dropped the OOM handling code altogether, we wouldn't need to bother. Needed for #9 --- cv/cv.c | 218 +++++++++++++++++--------------------------------------- 1 file changed, 66 insertions(+), 152 deletions(-) diff --git a/cv/cv.c b/cv/cv.c index 0adad17..d5a9b04 100644 --- a/cv/cv.c +++ b/cv/cv.c @@ -19,6 +19,39 @@ #include #include +static inline void * +cv_calloc(size_t nmemb, size_t size) +{ + void *ret = calloc(nmemb, size); + if (ret == NULL) { + say_crit("cv: calloc failed"); + abort(); + } + return ret; +} + +static inline void * +cv_realloc(void *ptr, size_t size) +{ + void *ret = realloc(ptr, size); + if (ret == NULL) { + say_crit("cv: realloc failed"); + abort(); + } + return ret; +} + +static inline char * +cv_strdup(const char *s) +{ + char *ret = strdup(s); + if (ret == NULL) { + say_crit("cv: strdup failed"); + abort(); + } + return ret; +} + /* ========================================================= * Type codes * ========================================================= */ @@ -223,12 +256,6 @@ struct cv_node { } as; }; -/* ========================================================= - * Static OOM error - * ========================================================= */ - -static int cv_oom_error_ref = LUA_NOREF; - /* ========================================================= * Runtime type handles (set by cv._init) * ========================================================= */ @@ -423,9 +450,7 @@ cv_props_free(lua_State *L, static struct cv_node * cv_node_alloc(void) { - struct cv_node *n = calloc(1, sizeof(*n)); - if (n == NULL) - return NULL; + struct cv_node *n = cv_calloc(1, sizeof(*n)); n->default_ref = LUA_NOREF; n->name_ref = LUA_NOREF; n->constraint_ref = LUA_NOREF; @@ -560,7 +585,7 @@ cv_type_by_name(const char *name) * Parse enum list * ========================================================= */ -static bool +static void cv_parse_enum(lua_State *L, int tbl_idx, struct cv_enum *out, enum cv_type ntype, @@ -568,11 +593,9 @@ cv_parse_enum(lua_State *L, int tbl_idx, { int n = (int)lua_objlen(L, tbl_idx); if (n == 0) - return true; + return; - out->refs = calloc(n, sizeof(int)); - if (out->refs == NULL) - return false; + out->refs = cv_calloc(n, sizeof(int)); for (int i = 0; i < n; i++) out->refs[i] = LUA_NOREF; out->count = n; @@ -586,7 +609,6 @@ cv_parse_enum(lua_State *L, int tbl_idx, out->refs[i - 1] = luaL_ref(L, LUA_REGISTRYINDEX); } - return true; } /* ========================================================= @@ -595,19 +617,17 @@ cv_parse_enum(lua_State *L, int tbl_idx, static struct cv_node * cv_compile_node(lua_State *L, int def_idx, - const char *path, int errors_idx, - bool *oom); + const char *path, int errors_idx); /* ========================================================= * Parse map properties table. * properties table is at stack index props_idx. * ========================================================= */ -static bool +static void cv_parse_properties(lua_State *L, int props_idx, struct cv_node *n, - const char *path, int errors_idx, - bool *oom) + const char *path, int errors_idx) { /* count keys */ int nprops = 0; @@ -618,14 +638,9 @@ cv_parse_properties(lua_State *L, int props_idx, } if (nprops == 0) - return true; + return; - n->as.map.props = calloc(nprops, - sizeof(struct cv_property)); - if (n->as.map.props == NULL) { - *oom = true; - return false; - } + n->as.map.props = cv_calloc(nprops, sizeof(struct cv_property)); n->as.map.nprops = nprops; int i = 0; @@ -638,12 +653,7 @@ cv_parse_properties(lua_State *L, int props_idx, if (lua_type(L, -2) == LUA_TSTRING) { const char *s = lua_tostring(L, -2); pkey.is_int = false; - pkey.sval = strdup(s); - if (pkey.sval == NULL) { - lua_pop(L, 1); - *oom = true; - return false; - } + pkey.sval = cv_strdup(s); snprintf(child_path, sizeof(child_path), "%s.%s", path, s); @@ -679,18 +689,13 @@ cv_parse_properties(lua_State *L, int props_idx, struct cv_node *child = cv_compile_node(L, val_idx, child_path, - errors_idx, oom); - if (*oom) { - lua_pop(L, 1); - return false; - } + errors_idx); n->as.map.props[i].node = child; i++; lua_pop(L, 1); /* pop value */ } /* adjust nprops in case some keys were skipped */ n->as.map.nprops = i; - return true; } /* ========================================================= @@ -699,11 +704,8 @@ cv_parse_properties(lua_State *L, int props_idx, static struct cv_node * cv_compile_node(lua_State *L, int def_idx, - const char *path, int errors_idx, - bool *oom) + const char *path, int errors_idx) { - *oom = false; - const char *type_str = NULL; bool nullable_from_type = false; @@ -883,10 +885,6 @@ cv_compile_node(lua_State *L, int def_idx, } struct cv_node *n = cv_node_alloc(); - if (n == NULL) { - *oom = true; - return NULL; - } n->type = (enum cv_type)type_code; n->optional = optional; /* '?' suffix makes value nullable too: @@ -1022,15 +1020,10 @@ cv_compile_node(lua_State *L, int def_idx, lua_getfield(L, def_idx, "enum"); if (lua_type(L, -1) == LUA_TTABLE) { int tbl = lua_gettop(L); - if (!cv_parse_enum(L, tbl, - &n->as.string.enums, - CV_TYPE_STRING, - path, errors_idx)) { - *oom = true; - lua_pop(L, 1); - cv_node_free(L, n); - return NULL; - } + cv_parse_enum(L, tbl, + &n->as.string.enums, + CV_TYPE_STRING, + path, errors_idx); } lua_pop(L, 1); break; @@ -1066,15 +1059,10 @@ cv_compile_node(lua_State *L, int def_idx, lua_getfield(L, def_idx, "enum"); if (lua_type(L, -1) == LUA_TTABLE) { int tbl = lua_gettop(L); - if (!cv_parse_enum(L, tbl, - &n->as.number.enums, - n->type, - path, errors_idx)) { - *oom = true; - lua_pop(L, 1); - cv_node_free(L, n); - return NULL; - } + cv_parse_enum(L, tbl, + &n->as.number.enums, + n->type, + path, errors_idx); } lua_pop(L, 1); break; @@ -1095,14 +1083,8 @@ cv_compile_node(lua_State *L, int def_idx, } if (rcount > 0) { n->as.map.rename.entries = - calloc(rcount, + cv_calloc(rcount, sizeof(struct cv_rename_entry)); - if (n->as.map.rename.entries == NULL) { - lua_pop(L, 1); - *oom = true; - cv_node_free(L, n); - return NULL; - } n->as.map.rename.count = rcount; int ri = 0; lua_pushnil(L); @@ -1117,15 +1099,8 @@ cv_compile_node(lua_State *L, int def_idx, lua_tointeger(L, -2); } else { e->from.is_int = false; - e->from.sval = strdup( + e->from.sval = cv_strdup( lua_tostring(L, -2)); - if (e->from.sval == NULL) { - lua_pop(L, 2); - lua_pop(L, 1); - *oom = true; - cv_node_free(L, n); - return NULL; - } } /* to = value at -1 */ if (lua_type(L, -1) == @@ -1135,15 +1110,8 @@ cv_compile_node(lua_State *L, int def_idx, lua_tointeger(L, -1); } else { e->to.is_int = false; - e->to.sval = strdup( + e->to.sval = cv_strdup( lua_tostring(L, -1)); - if (e->to.sval == NULL) { - lua_pop(L, 2); - lua_pop(L, 1); - *oom = true; - cv_node_free(L, n); - return NULL; - } } ri++; lua_pop(L, 1); @@ -1170,12 +1138,8 @@ cv_compile_node(lua_State *L, int def_idx, n->as.map.items = cv_compile_node(L, items_idx, child_path, - errors_idx, oom); + errors_idx); lua_pop(L, 1); - if (*oom) { - cv_node_free(L, n); - return NULL; - } break; /* skip properties */ } lua_pop(L, 1); @@ -1186,13 +1150,9 @@ cv_compile_node(lua_State *L, int def_idx, (lua_type(L, -1) == LUA_TTABLE); if (has_props) { int props_idx = lua_gettop(L); - if (!cv_parse_properties( - L, props_idx, n, - path, errors_idx, oom)) { - lua_pop(L, 1); - cv_node_free(L, n); - return NULL; - } + cv_parse_properties( + L, props_idx, n, + path, errors_idx); } lua_pop(L, 1); @@ -1323,15 +1283,10 @@ cv_compile_node(lua_State *L, int def_idx, struct cv_property *pp = &n->as.map.props[pi]; /* grow aliases array by 1 */ - struct cv_key *na = realloc( + struct cv_key *na = cv_realloc( pp->aliases, (pp->naliases + 1) * sizeof(struct cv_key)); - if (na == NULL) { - *oom = true; - cv_node_free(L, n); - return NULL; - } pp->aliases = na; struct cv_key *ak = &pp->aliases[pp->naliases]; @@ -1340,13 +1295,8 @@ cv_compile_node(lua_State *L, int def_idx, ak->ival = re->from.ival; } else { ak->is_int = false; - ak->sval = strdup( + ak->sval = cv_strdup( re->from.sval); - if (ak->sval == NULL) { - *oom = true; - cv_node_free(L, n); - return NULL; - } } pp->naliases++; } @@ -1392,12 +1342,8 @@ cv_compile_node(lua_State *L, int def_idx, n->as.array.items = cv_compile_node(L, items_idx, child_path, - errors_idx, oom); + errors_idx); lua_pop(L, 1); - if (*oom) { - cv_node_free(L, n); - return NULL; - } } else { lua_pop(L, 1); } @@ -1434,14 +1380,8 @@ cv_compile_node(lua_State *L, int def_idx, break; } n->as.oneof.variants = - calloc(vcount, + cv_calloc(vcount, sizeof(struct cv_node *)); - if (n->as.oneof.variants == NULL) { - lua_pop(L, 1); - *oom = true; - cv_node_free(L, n); - return NULL; - } n->as.oneof.nvariants = vcount; int vi = 0; lua_pushnil(L); @@ -1456,12 +1396,7 @@ cv_compile_node(lua_State *L, int def_idx, cv_compile_node( L, val_idx, child_path, - errors_idx, oom); - if (*oom) { - lua_pop(L, 2); - cv_node_free(L, n); - return NULL; - } + errors_idx); n->as.oneof.variants[vi] = v; vi++; lua_pop(L, 1); @@ -3258,16 +3193,8 @@ cv_compile(lua_State *L) lua_newtable(L); int errors_idx = lua_gettop(L); - bool oom = false; struct cv_node *n = - cv_compile_node(L, 1, "$", errors_idx, &oom); - - if (oom) { - lua_pushnil(L); - lua_rawgeti(L, LUA_REGISTRYINDEX, - cv_oom_error_ref); - return 2; - } + cv_compile_node(L, 1, "$", errors_idx); int nerrors = (int)lua_objlen(L, errors_idx); @@ -3403,19 +3330,6 @@ cv_is_schema(lua_State *L) LUA_API int luaopen_cv_cvalidator(lua_State *L) { - /* pre-allocate OOM error object */ - lua_newtable(L); - lua_newtable(L); - lua_pushstring(L, "$"); - lua_setfield(L, -2, "path"); - lua_pushstring(L, "MEMORY_ERROR"); - lua_setfield(L, -2, "type"); - lua_pushstring(L, "Not enough memory"); - lua_setfield(L, -2, "message"); - lua_rawseti(L, -2, 1); - cv_oom_error_ref = - luaL_ref(L, LUA_REGISTRYINDEX); - /* metatable for schema nodes */ luaL_newmetatable(L, CV_NODE_MT); From 2695235747cf796561a8c12f43e863ef2886edb9 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Tue, 8 Sep 2026 13:18:31 +0300 Subject: [PATCH 3/4] Do not assume that cv__init may be called more than once cv__init is called once when the module is loaded so there's no need to release previously taken Lua references. Moreover, releasing the old references makes it difficult to reproduce #9: the problem is that luaL_ref called right after luaL_unref takes the same slot in the reference table so with the current initialization procedure the bug may only occur if application threads race to load the c-validator module in parallel. Needed for #9 --- cv/cv.c | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/cv/cv.c b/cv/cv.c index d5a9b04..af50319 100644 --- a/cv/cv.c +++ b/cv/cv.c @@ -3240,11 +3240,6 @@ cv__init(lua_State *L) cv_ctid_uint64 = luaL_ctypeid(L, "uint64_t"); /* uuid_is */ - if (cv_ref_uuid_is != LUA_NOREF) { - luaL_unref(L, LUA_REGISTRYINDEX, - cv_ref_uuid_is); - cv_ref_uuid_is = LUA_NOREF; - } lua_getfield(L, 1, "uuid_is"); if (lua_isfunction(L, -1)) cv_ref_uuid_is = @@ -3253,11 +3248,6 @@ cv__init(lua_State *L) lua_pop(L, 1); /* tuple_is */ - if (cv_ref_tuple_is != LUA_NOREF) { - luaL_unref(L, LUA_REGISTRYINDEX, - cv_ref_tuple_is); - cv_ref_tuple_is = LUA_NOREF; - } lua_getfield(L, 1, "tuple_is"); if (lua_isfunction(L, -1)) cv_ref_tuple_is = @@ -3266,11 +3256,6 @@ cv__init(lua_State *L) lua_pop(L, 1); /* ffi_typestr */ - if (cv_ref_ffi_typestr != LUA_NOREF) { - luaL_unref(L, LUA_REGISTRYINDEX, - cv_ref_ffi_typestr); - cv_ref_ffi_typestr = LUA_NOREF; - } lua_getfield(L, 1, "ffi_typestr"); if (lua_isfunction(L, -1)) cv_ref_ffi_typestr = @@ -3279,11 +3264,6 @@ cv__init(lua_State *L) lua_pop(L, 1); /* box_null — cdata representing NULL */ - if (cv_ref_box_null != LUA_NOREF) { - luaL_unref(L, LUA_REGISTRYINDEX, - cv_ref_box_null); - cv_ref_box_null = LUA_NOREF; - } lua_getfield(L, 1, "box_null"); if (!lua_isnil(L, -1)) cv_ref_box_null = @@ -3292,11 +3272,6 @@ cv__init(lua_State *L) lua_pop(L, 1); /* deepcopy function — required */ - if (cv_ref_deepcopy != LUA_NOREF) { - luaL_unref(L, LUA_REGISTRYINDEX, - cv_ref_deepcopy); - cv_ref_deepcopy = LUA_NOREF; - } lua_getfield(L, 1, "deepcopy"); if (!lua_isfunction(L, -1)) { lua_pop(L, 1); From 78f96a08bd2828eea2052addf67212f564e6eda2 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Tue, 8 Sep 2026 14:00:46 +0300 Subject: [PATCH 4/4] Make module safe to use in application threads This commit marks all global variables used in the c-validator module thread-local. This should be enough to make it usable in Tarantool application threads. Closes #9 --- cv/cv.c | 14 ++++---- test/threads_test.lua | 76 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 test/threads_test.lua diff --git a/cv/cv.c b/cv/cv.c index af50319..8aa6362 100644 --- a/cv/cv.c +++ b/cv/cv.c @@ -261,15 +261,15 @@ struct cv_node { * ========================================================= */ /* CTypeIDs for int64_t / uint64_t — set at _init time */ -static uint32_t cv_ctid_int64 = 0; -static uint32_t cv_ctid_uint64 = 0; +static __thread uint32_t cv_ctid_int64 = 0; +static __thread uint32_t cv_ctid_uint64 = 0; /* Lua refs for type checkers / helpers */ -static int cv_ref_uuid_is = LUA_NOREF; -static int cv_ref_tuple_is = LUA_NOREF; -static int cv_ref_ffi_typestr = LUA_NOREF; -static int cv_ref_box_null = LUA_NOREF; -static int cv_ref_deepcopy = LUA_NOREF; +static __thread int cv_ref_uuid_is = LUA_NOREF; +static __thread int cv_ref_tuple_is = LUA_NOREF; +static __thread int cv_ref_ffi_typestr = LUA_NOREF; +static __thread int cv_ref_box_null = LUA_NOREF; +static __thread int cv_ref_deepcopy = LUA_NOREF; /* * Helper: is cdata at idx an int64_t? diff --git a/test/threads_test.lua b/test/threads_test.lua new file mode 100644 index 0000000..1049c45 --- /dev/null +++ b/test/threads_test.lua @@ -0,0 +1,76 @@ +#!/usr/bin/env tarantool + +local t = require('luatest') +local server = require('luatest.server') +local g = t.group('cv') + +g.before_all(function(cg) + cg.server = server:new({ + box_cfg = {app_threads = 1}, + net_box_credentials = {user = 'admin'} + }) + cg.server:start() +end) + +g.after_all(function(cg) + cg.server:drop() +end) + +g.test_threads = function(cg) + local function check() + local cv = require('cv') + local uuid = require('uuid') + local result, problems = cv.check({ + int64 = -123LL, + uint64 = 42LL, + tuple = box.tuple.new({123}), + uuid = uuid.NULL, + null = box.NULL, + nested = {}, + }, { + type = 'table', + properties = { + int64 = {'integer'}, + uint64 = {'unsigned'}, + tuple = 'tuple', + uuid = 'uuid', + null = 'null', + nested = { + type = 'table', + properties = { + foo = {'boolean', default = true}, + }, + }, + }, + }) + t.assert_equals(problems, {}) + t.assert_equals(result, { + int64 = -123LL, + uint64 = 42LL, + tuple = box.tuple.new({123}), + uuid = uuid.NULL, + null = box.NULL, + nested = {foo = true}, + }) + result, problems = cv.check(box.tuple.new({123}), 'uuid') + t.assert_is(result, nil) + t.assert_equals(problems, { + { + type = 'TYPE_ERROR', + message = 'Wrong type, expected uuid, got cdata', + path = '$', + details = { + value = box.tuple.new({123}), + actual_type = 'cdata', + cdata_type = 'ctype', + expected_type = 'uuid', + }, + }, + }) + end + cg.server:exec(check, {}, {_thread_id = 0}) + cg.server:exec(check, {}, {_thread_id = 1}) + cg.server:exec(check, {}, {_thread_id = 0}) + cg.server:exec(check, {}, {_thread_id = 1}) +end +-- vim: ts=4 sts=4 sw=4 et