From: Vladimir Davydov <vdavydov.dev@gmail.com> To: tarantool-patches@freelists.org Cc: georgy@tarantool.org, kyukhin@tarantool.org Subject: [PATCH] Revert "lua: fix tuple cdata collecting" Date: Sat, 22 Dec 2018 18:33:19 +0300 [thread overview] Message-ID: <2c351664bcd3f923cfb4c4892cb33db528c05e32.1545492518.git.vdavydov.dev@gmail.com> (raw) 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 8662a3a0..63ea73e4 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 e035cb93..b4201248 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 9df978d4..276bb0f6 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") -- 2.11.0
next reply other threads:[~2018-12-22 15:33 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-22 15:33 Vladimir Davydov [this message] 2018-12-22 15:39 ` Vladimir Davydov
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=2c351664bcd3f923cfb4c4892cb33db528c05e32.1545492518.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=georgy@tarantool.org \ --cc=kyukhin@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH] Revert "lua: fix tuple cdata collecting"' \ /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