From 2c351664bcd3f923cfb4c4892cb33db528c05e32 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Date: Sat, 22 Dec 2018 18:08:26 +0300
Subject: [PATCH] Revert "lua: fix tuple cdata collecting"
This reverts commit 022a3c5026fffcecd9bc85f5484d5b5c53ae887d.
According to FFI documentation [1], ffi.cast() creates a new object for
the given type and initializes it with the given value. So setting a
finalizer for a tuple before casting it to const_tuple_ref_t is totally
wrong as it allows the Lua interpreter to invoke the finalizer as soon
as the cast is complete. That said, we must revert the commit that
swapped ffi.cast and ffi.gc order in tuple_bless() and reopen the issue
it intended to fix - see #3751 - no wonder it improved the garbage
collector performance as it basically made all tuples collectable right
from the moment they are blessed.
To make sure we won't step on the same issue again in future, let's also
add a relevant test case.
[1] http://luajit.org/ext_ffi_api.html
Closes #3902
---
src/box/lua/tuple.lua | 2 +-
test/box/tuple.result | 19 +++++++++++++++++++
test/box/tuple.test.lua | 9 +++++++++
3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/src/box/lua/tuple.lua b/src/box/lua/tuple.lua
index 8662a3a02c..63ea73e43b 100644
--- a/src/box/lua/tuple.lua
+++ b/src/box/lua/tuple.lua
@@ -100,7 +100,7 @@ local tuple_bless = function(tuple)
-- overflow checked by tuple_bless() in C
builtin.box_tuple_ref(tuple)
-- must never fail:
- return ffi.cast(const_tuple_ref_t, ffi.gc(tuple, tuple_gc))
+ return ffi.gc(ffi.cast(const_tuple_ref_t, tuple), tuple_gc)
end
local tuple_check = function(tuple, usage)
diff --git a/test/box/tuple.result b/test/box/tuple.result
index e035cb932e..b420124852 100644
--- a/test/box/tuple.result
+++ b/test/box/tuple.result
@@ -1141,6 +1141,25 @@ assert(err ~= nil)
---
- true
...
+--
+-- gh-3902: tuple is collected while it's still in use.
+--
+t1 = box.tuple.new(1)
+---
+...
+t1 = t1:update{{'+', 1, 1}}
+---
+...
+collectgarbage()
+---
+- 0
+...
+t2 = box.tuple.new(2)
+---
+...
+t1 = t1:update{{'+', 1, 1}}
+---
+...
test_run:cmd("clear filter")
---
- true
diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
index 9df978d456..276bb0f675 100644
--- a/test/box/tuple.test.lua
+++ b/test/box/tuple.test.lua
@@ -374,4 +374,13 @@ s:drop()
_, err = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
assert(err ~= nil)
+--
+-- gh-3902: tuple is collected while it's still in use.
+--
+t1 = box.tuple.new(1)
+t1 = t1:update{{'+', 1, 1}}
+collectgarbage()
+t2 = box.tuple.new(2)
+t1 = t1:update{{'+', 1, 1}}
+
test_run:cmd("clear filter")
--
GitLab