lua: fix tuple leak in <key_def>.compare_with_key

The key difference between lbox_encode_tuple_on_gc() and
luaT_tuple_encode() is that the latter never raises a Lua error, but
passes an error using the diagnostics area.

Aside of the tuple leak, the patch fixes fiber region's memory 'leak'
(till fiber_gc()). Before the patch, the memory that is used for
serialization of the key is not freed (region_truncate()) when the
serialization fails. It is verified in the gh-5388-<...> test.

While I'm here, added a test case that just verifies correct behaviour
in case of a key serialization failure (added into key_def.test.lua).
The case does not verify whether a tuple leaks and it is successful as
before this patch as well after the patch. I don't find a simple way to
check the tuple leak within a test. Verified manually using the
reproducer from the linked issue.

Fixes #5388

changelogs/unreleased/fix-tuple-leak-in-lua-key_def.md

+## bugfix/lua
+
+* Fixed a leak of a tuple object in `key_def:compare_with_key(tuple, key)`,
+  when serialization of the key fails (gh-5388).

src/box/lua/key_def.c

@@ -371,9 +371,8 @@ lbox_key_def_compare_with_key(struct lua_State *L)
 
 	struct region *region = &fiber()->gc;
 	size_t region_svp = region_used(region);
-	size_t key_len;
-	const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len);
-	if (box_key_def_validate_key(key_def, key, NULL)) {
+	const char *key = luaT_tuple_encode(L, 3, NULL);
+	if (key == NULL || box_key_def_validate_key(key_def, key, NULL) != 0) {
 		region_truncate(region, region_svp);
 		tuple_unref(tuple);
 		return luaT_error(L);

test/box-tap/gh-5388-lua-key_def-leak.test.lua

+#!/usr/bin/env tarantool
+
+-- The issue (gh-5388) is about a leak of the tuple, serialized
+-- from a given key in the key_def:compare_with_key(tuple, key)
+-- method.
+--
+-- It is tricky to verify this particular problem (we don't have a
+-- counter of runtime tuples or runtime arena statistics).
+--
+-- The test is not about this problem. Aside of the tuple leak,
+-- there was another problem in the fixed code: fiber's region
+-- memory that is used for serialization of the key is 'leaked'
+-- as well (till fiber_gc()).
+--
+-- The test verifies that the fiber region does not hold any extra
+-- memory after failed serialization of the key.
+
+local ffi = require('ffi')
+local key_def_lib = require('key_def')
+local fiber = require('fiber')
+local tap = require('tap')
+
+ffi.cdef([[
+    void
+    box_region_truncate(size_t size);
+]])
+
+local function fiber_region_memory_used()
+    return fiber.info()[fiber.self().id()].memory.used
+end
+
+local test = tap.test('gh-5388-lua-key_def-leak')
+test:plan(1)
+
+local key_def = key_def_lib.new({
+    {type = 'string', fieldno = 1},
+})
+local tuple = box.tuple.new({'foo', 'bar'})
+
+-- Choice of data size for the key below is a bit tricky.
+--
+-- mpstream does not register every allocation in the region
+-- statistics as 'used', but it does after draining a slab (see
+-- mpstream_reserve_slow(): region_alloc() counts 'used',
+-- region_reserve() does not).
+--
+-- We should ensure that our 'large' key will be larger than a
+-- first slab that the fiber region will give to the mpstream (to
+-- exhaust it and see the 'leak' in the 'used' statistics).
+--
+-- The simplest way is to clean region's slab list: so the next
+-- slab will be one with a minimal order. A slab with minimal
+-- order for the fiber region is 4096 bytes (minus ~50 bytes for
+-- the allocator itself).
+--
+-- It does not look safe to call box_region_truncate(0), because
+-- tarantool itself may hold something in the region's memory (in
+-- theory). But while it works, it is okay for testing purposes.
+--
+-- At least it is easier to call region_truncate() rather than
+-- check that &fiber()->gc.slabs is empty or have the first slab
+-- with amount of unused memory that is lower than given constant
+-- (say, 4096).
+--
+-- It seems, we should improve our memory statistics and/or
+-- debugging tools in a future. This test case should not require
+-- so much explanation of internals.
+ffi.C.box_region_truncate(0)
+
+-- Serialization of this key should fail.
+local large_data = string.rep('x', 4096)
+local key = {large_data, function() end}
+
+-- Verify that data allocated on the fiber region (&fiber()->gc)
+-- during serialization of the key is freed when the serialization
+-- fails.
+local before = fiber_region_memory_used()
+pcall(key_def.compare_with_key, key_def, tuple, key)
+local after = fiber_region_memory_used()
+test:is(after - before, 0, 'fiber region does not leak')
+
+os.exit(test:check() and 0 or 1)

test/box-tap/key_def.test.lua

@@ -390,7 +390,7 @@ end)
 
 -- Case: compare_with_key().
 test:test('compare_with_key()', function(test)
-    test:plan(3)
+    test:plan(4)
 
     local key_def_b = key_def_lib.new({
         {type = 'number', fieldno = 2},
@@ -411,6 +411,12 @@ test:test('compare_with_key()', function(test)
     })
     local ok, err = pcall(key_def.compare_with_key, key_def, {'aa', {}}, {'bb', box.NULL})
     test:is_deeply({ok, tostring(err)}, {false, cmp_err}, 'no composite comparison')
+
+    -- Unserializable key.
+    local exp_err = "unsupported Lua type 'function'"
+    local key = {function() end}
+    local ok, err = pcall(key_def_b.compare_with_key, key_def_b, tuple_a, key)
+    test:is_deeply({ok, tostring(err)}, {false, exp_err}, 'unserializable key')
 end)
 
 -- Case: totable().