[Tarantool-patches] [PATCH 09/16] 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
---
 src/exports.h                              |  2 +-
 src/lua/uuid.lua                           | 13 ++++--
 test/app-tap/gh-5632-gc-buf-reuse.test.lua | 49 ++++++++++++++++++++++
 3 files changed, 60 insertions(+), 4 deletions(-)
 create mode 100755 test/app-tap/gh-5632-gc-buf-reuse.test.lua

diff --git a/src/exports.h b/src/exports.h
index ddbe57230..a90b9406a 100644
--- a/src/exports.h
+++ b/src/exports.h
@@ -517,7 +517,7 @@ EXPORT(tt_uuid_create)
 EXPORT(tt_uuid_from_string)
 EXPORT(tt_uuid_is_equal)
 EXPORT(tt_uuid_is_nil)
-EXPORT(tt_uuid_str)
+EXPORT(tt_uuid_to_string)
 EXPORT(uri_format)
 EXPORT(uri_parse)
 EXPORT(uuid_nil)
diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua
index 74f8c924c..3047b665c 100644
--- a/src/lua/uuid.lua
+++ b/src/lua/uuid.lua
@@ -17,8 +17,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);
 int
 tt_uuid_compare(const struct tt_uuid *a, const struct tt_uuid *b);
 extern const struct tt_uuid uuid_nil;
@@ -31,6 +29,11 @@ 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 is_uuid = function(value)
     return ffi.istype(uuid_t, value)
 end
@@ -39,7 +42,11 @@ local uuid_tostring = function(uu)
     if not is_uuid(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..8fe662d3f
--- /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 i = 1, gc_count do
+            ffi.gc(ffi.new('char[1]'), function() uuid_to_str() end)
+        end
+    end
+
+    for i = 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)
-- 
2.24.3 (Apple Git-128)