[Tarantool-patches] [PATCH 09/15] uuid: drop tt_uuid_str() from Lua

[Vladislav Shpilevoy]

The function converts struct tt_uuid * to a string. The string is
allocated on the static buffer, which can't be used in Lua due to
unpredictable GC behaviour. It can start working any moment even
if tt_uuid_str() has returned, but its result wasn't passed to
ffi.string() yet. Then the buffer might be overwritten.

Lua uuid now uses tt_uuid_to_string() which does the same but
takes the buffer pointer. The buffer is stored in an ffi stash,
because it is x4 times faster than ffi.new('char[37]') (where 37
is length of a UUID string + terminating 0) (2.4 ns vs 0.8 ns).

After this patch UUID is supposed to be fully compatible with Lua
GC handlers.

Part of #5632

(cherry picked from commit acf8745ed8fef47e6d1f1c31708c7c9d6324d2f3)
---
 extra/exports                              |  1 +
 src/lua/uuid.lua                           | 13 ++++--
 test/app-tap/gh-5632-gc-buf-reuse.test.lua | 49 ++++++++++++++++++++++
 test/app/uuid.result                       |  2 +-
 4 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100755 test/app-tap/gh-5632-gc-buf-reuse.test.lua

diff --git a/extra/exports b/extra/exports
index 91094206d..b27b22a7c 100644
--- a/extra/exports
+++ b/extra/exports
@@ -51,6 +51,7 @@ tt_uuid_is_equal
 tt_uuid_is_nil
 tt_uuid_bswap
 tt_uuid_from_string
+tt_uuid_to_string
 log_level
 log_format
 uri_parse
diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua
index 46b35075a..3efb7f66b 100644
--- a/src/lua/uuid.lua
+++ b/src/lua/uuid.lua
@@ -26,8 +26,6 @@ bool
 tt_uuid_is_nil(const struct tt_uuid *uu);
 bool
 tt_uuid_is_equal(const struct tt_uuid *lhs, const struct tt_uuid *rhs);
-char *
-tt_uuid_str(const struct tt_uuid *uu);
 extern const struct tt_uuid uuid_nil;
 ]]
 
@@ -38,11 +36,20 @@ local uuid_stash = buffer.ffi_stash_new(uuid_t)
 local uuid_stash_take = uuid_stash.take
 local uuid_stash_put = uuid_stash.put
 
+local uuid_str_stash =
+    buffer.ffi_stash_new(string.format('char[%s]', UUID_STR_LEN + 1))
+local uuid_str_stash_take = uuid_str_stash.take
+local uuid_str_stash_put = uuid_str_stash.put
+
 local uuid_tostring = function(uu)
     if not ffi.istype(uuid_t, uu) then
         return error('Usage: uuid:str()')
     end
-    return ffi.string(builtin.tt_uuid_str(uu), UUID_STR_LEN)
+    local strbuf = uuid_str_stash_take()
+    builtin.tt_uuid_to_string(uu, strbuf)
+    uu = ffi.string(strbuf, UUID_STR_LEN)
+    uuid_str_stash_put(strbuf)
+    return uu
 end
 
 local uuid_fromstr = function(str)
diff --git a/test/app-tap/gh-5632-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-gc-buf-reuse.test.lua
new file mode 100755
index 000000000..b09b1bf3e
--- /dev/null
+++ b/test/app-tap/gh-5632-gc-buf-reuse.test.lua
@@ -0,0 +1,49 @@
+#!/usr/bin/env tarantool
+
+--
+-- gh-5632: 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')
+local ffi = require('ffi')
+local uuid = require('uuid')
+
+local function test_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 = uu:str()
+        local str2 = uu:str()
+        if str1 ~= str2 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, 'uuid in gc')
+end
+
+local test = tap.test('gh-5632-gc-buf-reuse')
+test:plan(1)
+test:test('uuid in __gc', test_uuid)
+
+os.exit(test:check() and 0 or 1)
diff --git a/test/app/uuid.result b/test/app/uuid.result
index 1da8b3e58..b252c497d 100644
--- a/test/app/uuid.result
+++ b/test/app/uuid.result
@@ -106,7 +106,7 @@ uu.node[5]
 -- invalid values
 uuid.fromstr(nil)
 ---
-- error: 'builtin/uuid.lua:50: fromstr(str)'
+- error: 'builtin/uuid.lua:57: fromstr(str)'
 ...
 uuid.fromstr('')
 ---
-- 
2.24.3 (Apple Git-128)