From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Serge Petrenko Subject: [PATCH] lua/trigger: cleanup lua stack after trigger run Date: Tue, 25 Jun 2019 17:32:50 +0300 Message-Id: <20190625143250.97948-1-sergepetrenko@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: vdavydov.dev@gmail.com Cc: tarantool-patches@freelists.org, Serge Petrenko List-ID: This patch adds a stack cleanup after a trigger is run and its return values, if any, have been read. This problem was found in a case when on_schema_init trigger set an on_replace trigger on a space, and the trigger ran during recovery. This lead to Lua stack overflows for the aforementioned reasons. Closes #4275 --- https://github.com/tarantool/tarantool/issues/4275 https://github.com/tarantool/tarantool/tree/sp/gh-4275-before-replace-on-recovery src/lua/trigger.c | 6 ++++ test/box/before_replace.result | 48 ++++++++++++++++++++++++++++++++ test/box/before_replace.test.lua | 27 ++++++++++++++++++ test/box/box.lua | 23 +++++++++++++++ 4 files changed, 104 insertions(+) diff --git a/src/lua/trigger.c b/src/lua/trigger.c index ec4d8aab3..4803e85c5 100644 --- a/src/lua/trigger.c +++ b/src/lua/trigger.c @@ -102,9 +102,15 @@ lbox_trigger_run(struct trigger *ptr, void *event) int nret = lua_gettop(L) - top; if (trigger->pop_event != NULL && trigger->pop_event(L, nret, event) != 0) { + lua_settop(L, top); luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref); diag_raise(); } + /* + * Clear the stack after pop_event saves all + * the needed return values. + */ + lua_settop(L, top); luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref); } diff --git a/test/box/before_replace.result b/test/box/before_replace.result index 5054fdefd..661d999bb 100644 --- a/test/box/before_replace.result +++ b/test/box/before_replace.result @@ -858,3 +858,51 @@ save_type s:drop() --- ... +-- +-- gh-4275 segfault if before_replace +-- trigger set in on_schema_init triggers runs on recovery. +-- +s = box.schema.space.create('test_on_schema_init') +--- +... +_ = s:create_index('pk') +--- +... +test_run:cmd('setopt delimiter ";"') +--- +- true +... +function gen_inserts() + box.begin() + for i = 1,1000 do + s:upsert({1, 1}, {{'+', 2, 1}}) + end + box.commit() +end; +--- +... +test_run:cmd('setopt delimiter ""'); +--- +- true +... +for i = 1,66 do gen_inserts() end +--- +... +test_run:cmd('restart server default with args="true"') +s = box.space.test_on_schema_init +--- +... +s:count() +--- +- 1 +... +-- insertion: 65999 updates from gen_inserts(), starting value is 1. +-- recovery: 66000 invocations of before replace trigger, each +-- invocation updates the tuple, but only in memory: the WAL remains +-- unchanged. +-- A total of 1 + 65999 + 66000 = 132000 +s:get{1}[2] == 2 * 66000 or s:get{1}[2] +--- +- true +... +test_run:cmd('restart server default with cleanup=1') diff --git a/test/box/before_replace.test.lua b/test/box/before_replace.test.lua index c46ec25a0..688b012d0 100644 --- a/test/box/before_replace.test.lua +++ b/test/box/before_replace.test.lua @@ -306,3 +306,30 @@ _ = s:insert{1} save_type s:drop() + +-- +-- gh-4275 segfault if before_replace +-- trigger set in on_schema_init triggers runs on recovery. +-- +s = box.schema.space.create('test_on_schema_init') +_ = s:create_index('pk') +test_run:cmd('setopt delimiter ";"') +function gen_inserts() + box.begin() + for i = 1,1000 do + s:upsert({1, 1}, {{'+', 2, 1}}) + end + box.commit() +end; +test_run:cmd('setopt delimiter ""'); +for i = 1,66 do gen_inserts() end +test_run:cmd('restart server default with args="true"') +s = box.space.test_on_schema_init +s:count() +-- insertion: 65999 updates from gen_inserts(), starting value is 1. +-- recovery: 66000 invocations of before replace trigger, each +-- invocation updates the tuple, but only in memory: the WAL remains +-- unchanged. +-- A total of 1 + 65999 + 66000 = 132000 +s:get{1}[2] == 2 * 66000 or s:get{1}[2] +test_run:cmd('restart server default with cleanup=1') diff --git a/test/box/box.lua b/test/box/box.lua index 2a8e0e4fa..7eaa5043e 100644 --- a/test/box/box.lua +++ b/test/box/box.lua @@ -1,5 +1,28 @@ #!/usr/bin/env tarantool os = require('os') +local enable_before_replace = arg[1] ~= nil + +print('\n\n\n', enable_before_replace, '\n\n\n') + +function test_before_replace_trig(old, new) + return new:update{{'+', 2, 1}} +end + +function space_on_replace_trig(old, new) + if new and new[3] == 'test_on_schema_init' then + box.on_commit(function() + box.space.test_on_schema_init:before_replace(test_before_replace_trig) + end) + end +end + +function on_init_trig() + if enable_before_replace then + box.space._space:on_replace(space_on_replace_trig) + end +end + +box.ctl.on_schema_init(on_init_trig) box.cfg{ listen = os.getenv("LISTEN"), -- 2.20.1 (Apple Git-117)