From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergos@tarantool.org Subject: [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids Date: Tue, 27 Jul 2021 23:24:12 +0200 [thread overview] Message-ID: <e8087c2764119c8530a275091196dac4d12b7cb3.1627420835.git.v.shpilevoy@tarantool.org> (raw) In-Reply-To: <cover.1627420835.git.v.shpilevoy@tarantool.org> box.info.uuid, box.info.cluster.uuid, and box.info.* replica UUIDs used tt_uuid_str() function. The function is not safe to use in preemptive context like Lua, where any attempt to push something onto the Lua stack might trigger GC, which in turn might invoke any other code. It is not safe because uses the static buffer, which is global and cyclic. Newer allocations can override the old data without any warning. Follow up #5632 Follow up #6050 Part of #6259 --- src/box/lua/info.c | 8 ++-- ...> gh-5632-6050-6259-gc-buf-reuse.test.lua} | 48 ++++++++++++++++--- 2 files changed, 46 insertions(+), 10 deletions(-) rename test/app-tap/{gh-5632-6050-gc-buf-reuse.test.lua => gh-5632-6050-6259-gc-buf-reuse.test.lua} (79%) diff --git a/src/box/lua/info.c b/src/box/lua/info.c index 1d8fe7938..d297ec6f6 100644 --- a/src/box/lua/info.c +++ b/src/box/lua/info.c @@ -176,7 +176,7 @@ lbox_pushreplica(lua_State *L, struct replica *replica) lua_settable(L, -3); lua_pushstring(L, "uuid"); - lua_pushstring(L, tt_uuid_str(&replica->uuid)); + luaL_pushuuidstr(L, &replica->uuid); lua_settable(L, -3); lua_pushstring(L, "lsn"); @@ -235,7 +235,7 @@ lbox_info_replication_anon_call(struct lua_State *L) if (!replica->anon) continue; - lua_pushstring(L, tt_uuid_str(&replica->uuid)); + luaL_pushuuidstr(L, &replica->uuid); lbox_pushreplica(L, replica); lua_settable(L, -3); @@ -290,7 +290,7 @@ lbox_info_id(struct lua_State *L) static int lbox_info_uuid(struct lua_State *L) { - lua_pushlstring(L, tt_uuid_str(&INSTANCE_UUID), UUID_STR_LEN); + luaL_pushuuidstr(L, &INSTANCE_UUID); return 1; } @@ -376,7 +376,7 @@ lbox_info_cluster(struct lua_State *L) { lua_createtable(L, 0, 2); lua_pushliteral(L, "uuid"); - lua_pushlstring(L, tt_uuid_str(&REPLICASET_UUID), UUID_STR_LEN); + luaL_pushuuidstr(L, &REPLICASET_UUID); lua_settable(L, -3); return 1; } diff --git a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua similarity index 79% rename from test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua rename to test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua index bf7590a14..f806ba6b7 100755 --- a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua +++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua @@ -1,10 +1,10 @@ #!/usr/bin/env tarantool -- --- gh-5632, gh-6050: Lua code should not use any global buffers or objects --- without proper ownership protection. Otherwise these items might be suddenly --- reused during Lua GC which happens almost at any moment. That might lead to --- data corruption. +-- gh-5632, gh-6050, gh-6259: Lua code should not use any global buffers or +-- objects without proper ownership protection. Otherwise these items might be +-- suddenly reused during Lua GC which happens almost at any moment. That might +-- lead to data corruption. -- local tap = require('tap') @@ -190,11 +190,47 @@ local function test_json(test) test:ok(is_success, 'json in gc') end -local test = tap.test('gh-5632-6050-gc-buf-reuse') -test:plan(4) +local function test_info_uuid(test) + test:plan(1) + + local gc_count = 100 + local iter_count = 1000 + local is_success = true + + local function uuid_to_str() + local uu = uuid.new() + local str1 = box.info.uuid + local str2 = box.info.cluster.uuid + local str3 = box.info.uuid + local str4 = box.info.cluster.uuid + if str1 ~= str3 or str2 ~= str4 then + is_success = false + assert(false) + end + end + + local function create_gc() + for _ = 1, gc_count do + ffi.gc(ffi.new('char[1]'), function() uuid_to_str() end) + end + end + + for _ = 1, iter_count do + create_gc() + uuid_to_str() + end + + test:ok(is_success, 'info uuid in gc') +end + +box.cfg{} + +local test = tap.test('gh-5632-6050-6259-gc-buf-reuse') +test:plan(5) test:test('uuid in __gc', test_uuid) test:test('uri in __gc', test_uri) test:test('msgpackffi in __gc', test_msgpackffi) test:test('json in __gc', test_json) +test:test('info uuid in __gc', test_info_uuid) os.exit(test:check() and 0 or 1) -- 2.24.3 (Apple Git-128)
next prev parent reply other threads:[~2021-07-27 21:25 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches 2021-07-27 21:24 ` [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr() Vladislav Shpilevoy via Tarantool-patches 2021-07-29 11:30 ` Sergey Ostanevich via Tarantool-patches 2021-07-27 21:24 ` Vladislav Shpilevoy via Tarantool-patches [this message] 2021-07-29 11:38 ` [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids Sergey Ostanevich via Tarantool-patches 2021-08-01 15:03 ` Vladislav Shpilevoy via Tarantool-patches 2021-08-01 17:01 ` Sergey Ostanevich via Tarantool-patches 2021-07-27 21:24 ` [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str Vladislav Shpilevoy via Tarantool-patches 2021-07-29 11:41 ` Sergey Ostanevich via Tarantool-patches 2021-08-01 15:03 ` Vladislav Shpilevoy via Tarantool-patches 2021-08-01 17:01 ` Sergey Ostanevich via Tarantool-patches 2021-07-27 21:24 ` [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string Vladislav Shpilevoy via Tarantool-patches 2021-07-29 11:52 ` Sergey Ostanevich via Tarantool-patches 2021-08-01 15:04 ` Vladislav Shpilevoy via Tarantool-patches 2021-08-01 17:06 ` Sergey Ostanevich via Tarantool-patches 2021-07-27 21:24 ` [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr() Vladislav Shpilevoy via Tarantool-patches 2021-07-29 12:28 ` Sergey Ostanevich via Tarantool-patches 2021-08-01 15:04 ` Vladislav Shpilevoy via Tarantool-patches 2021-07-27 21:39 ` [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Cyrill Gorcunov via Tarantool-patches 2021-08-02 19:45 ` Vladislav Shpilevoy via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=e8087c2764119c8530a275091196dac4d12b7cb3.1627420835.git.v.shpilevoy@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=gorcunov@gmail.com \ --cc=sergos@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox