<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Resend<br class=""><div class="">
<div dir="auto" style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div>--</div><div>Serge Petrenko</div><div><a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a></div><div class=""><br class=""></div></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<div style=""><br class=""><blockquote type="cite" class=""><div class="">25 июня 2019 г., в 17:32, Serge Petrenko <<a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a>> написал(а):</div><br class="Apple-interchange-newline"><div class=""><div class="">This patch adds a stack cleanup after a trigger is run and its return<br class="">values, if any, have been read.<br class=""><br class="">This problem was found in a case when on_schema_init trigger set an<br class="">on_replace trigger on a space, and the trigger ran during recovery.<br class="">This lead to Lua stack overflows for the aforementioned reasons.<br class=""><br class="">Closes #4275<br class="">---<br class=""><a href="https://github.com/tarantool/tarantool/issues/4275" class="">https://github.com/tarantool/tarantool/issues/4275</a><br class="">https://github.com/tarantool/tarantool/tree/sp/gh-4275-before-replace-on-recovery<br class=""><br class=""> src/lua/trigger.c | 6 ++++<br class=""> test/box/before_replace.result | 48 ++++++++++++++++++++++++++++++++<br class=""> test/box/before_replace.test.lua | 27 ++++++++++++++++++<br class=""> test/box/box.lua | 23 +++++++++++++++<br class=""> 4 files changed, 104 insertions(+)<br class=""><br class="">diff --git a/src/lua/trigger.c b/src/lua/trigger.c<br class="">index ec4d8aab3..4803e85c5 100644<br class="">--- a/src/lua/trigger.c<br class="">+++ b/src/lua/trigger.c<br class="">@@ -102,9 +102,15 @@ lbox_trigger_run(struct trigger *ptr, void *event)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>int nret = lua_gettop(L) - top;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (trigger->pop_event != NULL &&<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> trigger->pop_event(L, nret, event) != 0) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>lua_settop(L, top);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>diag_raise();<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>/*<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * Clear the stack after pop_event saves all<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * the needed return values.<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>lua_settop(L, top);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref);<br class=""> }<br class=""><br class="">diff --git a/test/box/before_replace.result b/test/box/before_replace.result<br class="">index 5054fdefd..661d999bb 100644<br class="">--- a/test/box/before_replace.result<br class="">+++ b/test/box/before_replace.result<br class="">@@ -858,3 +858,51 @@ save_type<br class=""> s:drop()<br class=""> ---<br class=""> ...<br class="">+--<br class="">+-- gh-4275 segfault if before_replace<br class="">+-- trigger set in on_schema_init triggers runs on recovery.<br class="">+--<br class="">+s = box.schema.space.create('test_on_schema_init')<br class="">+---<br class="">+...<br class="">+_ = s:create_index('pk')<br class="">+---<br class="">+...<br class="">+test_run:cmd('setopt delimiter ";"')<br class="">+---<br class="">+- true<br class="">+...<br class="">+function gen_inserts()<br class="">+ box.begin()<br class="">+ for i = 1,1000 do<br class="">+ s:upsert({1, 1}, {{'+', 2, 1}})<br class="">+ end<br class="">+ box.commit()<br class="">+end;<br class="">+---<br class="">+...<br class="">+test_run:cmd('setopt delimiter ""');<br class="">+---<br class="">+- true<br class="">+...<br class="">+for i = 1,66 do gen_inserts() end<br class="">+---<br class="">+...<br class="">+test_run:cmd('restart server default with args="true"')<br class="">+s = box.space.test_on_schema_init<br class="">+---<br class="">+...<br class="">+s:count()<br class="">+---<br class="">+- 1<br class="">+...<br class="">+-- insertion: 65999 updates from gen_inserts(), starting value is 1.<br class="">+-- recovery: 66000 invocations of before replace trigger, each<br class="">+-- invocation updates the tuple, but only in memory: the WAL remains<br class="">+-- unchanged.<br class="">+-- A total of 1 + 65999 + 66000 = 132000<br class="">+s:get{1}[2] == 2 * 66000 or s:get{1}[2]<br class="">+---<br class="">+- true<br class="">+...<br class="">+test_run:cmd('restart server default with cleanup=1')<br class="">diff --git a/test/box/before_replace.test.lua b/test/box/before_replace.test.lua<br class="">index c46ec25a0..688b012d0 100644<br class="">--- a/test/box/before_replace.test.lua<br class="">+++ b/test/box/before_replace.test.lua<br class="">@@ -306,3 +306,30 @@ _ = s:insert{1}<br class=""> save_type<br class=""><br class=""> s:drop()<br class="">+<br class="">+--<br class="">+-- gh-4275 segfault if before_replace<br class="">+-- trigger set in on_schema_init triggers runs on recovery.<br class="">+--<br class="">+s = box.schema.space.create('test_on_schema_init')<br class="">+_ = s:create_index('pk')<br class="">+test_run:cmd('setopt delimiter ";"')<br class="">+function gen_inserts()<br class="">+ box.begin()<br class="">+ for i = 1,1000 do<br class="">+ s:upsert({1, 1}, {{'+', 2, 1}})<br class="">+ end<br class="">+ box.commit()<br class="">+end;<br class="">+test_run:cmd('setopt delimiter ""');<br class="">+for i = 1,66 do gen_inserts() end<br class="">+test_run:cmd('restart server default with args="true"')<br class="">+s = box.space.test_on_schema_init<br class="">+s:count()<br class="">+-- insertion: 65999 updates from gen_inserts(), starting value is 1.<br class="">+-- recovery: 66000 invocations of before replace trigger, each<br class="">+-- invocation updates the tuple, but only in memory: the WAL remains<br class="">+-- unchanged.<br class="">+-- A total of 1 + 65999 + 66000 = 132000<br class="">+s:get{1}[2] == 2 * 66000 or s:get{1}[2]<br class="">+test_run:cmd('restart server default with cleanup=1')<br class="">diff --git a/test/box/box.lua b/test/box/box.lua<br class="">index 2a8e0e4fa..7eaa5043e 100644<br class="">--- a/test/box/box.lua<br class="">+++ b/test/box/box.lua<br class="">@@ -1,5 +1,28 @@<br class=""> #!/usr/bin/env tarantool<br class=""> os = require('os')<br class="">+local enable_before_replace = arg[1] ~= nil<br class="">+<br class="">+print('\n\n\n', enable_before_replace, '\n\n\n')<br class="">+<br class="">+function test_before_replace_trig(old, new)<br class="">+ return new:update{{'+', 2, 1}}<br class="">+end<br class="">+<br class="">+function space_on_replace_trig(old, new)<br class="">+ if new and new[3] == 'test_on_schema_init' then<br class="">+ box.on_commit(function()<br class="">+ box.space.test_on_schema_init:before_replace(test_before_replace_trig)<br class="">+ end)<br class="">+ end<br class="">+end<br class="">+<br class="">+function on_init_trig()<br class="">+ if enable_before_replace then<br class="">+ box.space._space:on_replace(space_on_replace_trig)<br class="">+ end<br class="">+end<br class="">+<br class="">+box.ctl.on_schema_init(on_init_trig)<br class=""><br class=""> box.cfg{<br class=""> listen = os.getenv("LISTEN"),<br class="">-- <br class="">2.20.1 (Apple Git-117)<br class=""><br class=""><br class=""></div></div></blockquote></div><br class=""></body></html>