From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id DE7EF24B18 for ; Fri, 5 Jul 2019 04:49:04 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2dgiYCc3Bxq9 for ; Fri, 5 Jul 2019 04:49:04 -0400 (EDT) Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 77F2824AF5 for ; Fri, 5 Jul 2019 04:49:04 -0400 (EDT) From: Serge Petrenko Message-Id: <74FFA62C-6DAC-42CF-83C3-6A9B63C16F95@tarantool.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_510DCC3A-A06C-448A-A80E-0862DFEAEDF9" Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: [tarantool-patches] Re: [PATCH] lua/trigger: cleanup lua stack after trigger run Date: Fri, 5 Jul 2019 11:49:01 +0300 In-Reply-To: <20190625143250.97948-1-sergepetrenko@tarantool.org> References: <20190625143250.97948-1-sergepetrenko@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Alexander Turenko Cc: tarantool-patches@freelists.org --Apple-Mail=_510DCC3A-A06C-448A-A80E-0862DFEAEDF9 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Resend -- Serge Petrenko sergepetrenko@tarantool.org > 25 =D0=B8=D1=8E=D0=BD=D1=8F 2019 =D0=B3., =D0=B2 17:32, Serge Petrenko = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0): >=20 > This patch adds a stack cleanup after a trigger is run and its return > values, if any, have been read. >=20 > 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. >=20 > Closes #4275 > --- > https://github.com/tarantool/tarantool/issues/4275 > = https://github.com/tarantool/tarantool/tree/sp/gh-4275-before-replace-on-r= ecovery >=20 > 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(+) >=20 > 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 =3D lua_gettop(L) - top; > if (trigger->pop_event !=3D NULL && > trigger->pop_event(L, nret, event) !=3D 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); > } >=20 > 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 =3D box.schema.space.create('test_on_schema_init') > +--- > +... > +_ =3D s:create_index('pk') > +--- > +... > +test_run:cmd('setopt delimiter ";"') > +--- > +- true > +... > +function gen_inserts() > + box.begin() > + for i =3D 1,1000 do > + s:upsert({1, 1}, {{'+', 2, 1}}) > + end > + box.commit() > +end; > +--- > +... > +test_run:cmd('setopt delimiter ""'); > +--- > +- true > +... > +for i =3D 1,66 do gen_inserts() end > +--- > +... > +test_run:cmd('restart server default with args=3D"true"') > +s =3D 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 =3D 132000 > +s:get{1}[2] =3D=3D 2 * 66000 or s:get{1}[2] > +--- > +- true > +... > +test_run:cmd('restart server default with cleanup=3D1') > 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 @@ _ =3D s:insert{1} > save_type >=20 > s:drop() > + > +-- > +-- gh-4275 segfault if before_replace > +-- trigger set in on_schema_init triggers runs on recovery. > +-- > +s =3D box.schema.space.create('test_on_schema_init') > +_ =3D s:create_index('pk') > +test_run:cmd('setopt delimiter ";"') > +function gen_inserts() > + box.begin() > + for i =3D 1,1000 do > + s:upsert({1, 1}, {{'+', 2, 1}}) > + end > + box.commit() > +end; > +test_run:cmd('setopt delimiter ""'); > +for i =3D 1,66 do gen_inserts() end > +test_run:cmd('restart server default with args=3D"true"') > +s =3D 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 =3D 132000 > +s:get{1}[2] =3D=3D 2 * 66000 or s:get{1}[2] > +test_run:cmd('restart server default with cleanup=3D1') > 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 =3D require('os') > +local enable_before_replace =3D arg[1] ~=3D 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] =3D=3D '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) >=20 > box.cfg{ > listen =3D os.getenv("LISTEN"), > --=20 > 2.20.1 (Apple Git-117) >=20 >=20 --Apple-Mail=_510DCC3A-A06C-448A-A80E-0862DFEAEDF9 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Resend



25 =D0=B8=D1=8E=D0=BD=D1=8F 2019 =D0=B3., =D0=B2 17:32, Serge = Petrenko <sergepetrenko@tarantool.org> = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):

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-r= eplace-on-recovery

src/lua/trigger.c =             &n= bsp;  |  6 ++++
= test/box/before_replace.result   | 48 = ++++++++++++++++++++++++++++++++
= test/box/before_replace.test.lua | 27 ++++++++++++++++++
= test/box/box.lua =             &n= bsp;   | 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 = =3D lua_gettop(L) - top;
if (trigger->pop_event !=3D = NULL &&
=    trigger->pop_event(L, nret, event) !=3D 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 =3D = box.schema.space.create('test_on_schema_init')
+---
+...
+_ =3D s:create_index('pk')
+---
+...
+test_run:cmd('setopt = delimiter ";"')
+---
+- true
+...
+function gen_inserts()
+ =    box.begin()
+    for i =3D = 1,1000 do
+ =        s:upsert({1, 1}, {{'+', 2, = 1}})
+    end
+ =    box.commit()
+end;
+---
+...
+test_run:cmd('setopt delimiter ""');
+---
+- true
+...
+for i =3D 1,66 do gen_inserts() end
+---
+...
+test_run:cmd('restart server default with = args=3D"true"')
+s =3D 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 =3D 132000
+s:get{1}[2] =3D=3D 2 * = 66000 or s:get{1}[2]
+---
+- true
+...
+test_run:cmd('restart server default with = cleanup=3D1')
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 @@ _ = =3D s:insert{1}
save_type

= s:drop()
+
+--
+-- gh-4275 = segfault if before_replace
+-- trigger set in = on_schema_init triggers runs on recovery.
+--
+s =3D box.schema.space.create('test_on_schema_init')
+_ =3D s:create_index('pk')
+test_run:cmd('setopt= delimiter ";"')
+function gen_inserts()
+ =    box.begin()
+    for i =3D = 1,1000 do
+ =        s:upsert({1, 1}, {{'+', 2, = 1}})
+    end
+ =    box.commit()
+end;
+test_run:cmd('setopt delimiter ""');
+for i =3D = 1,66 do gen_inserts() end
+test_run:cmd('restart server = default with args=3D"true"')
+s =3D = 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 =3D 132000
+s:get{1}[2] =3D=3D 2 * = 66000 or s:get{1}[2]
+test_run:cmd('restart server default = with cleanup=3D1')
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 =3D require('os')
+local enable_before_replace =3D arg[1] ~=3D 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] =3D=3D = 'test_on_schema_init' then
+ =        box.on_commit(function()
+ =            box.spac= e.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(spac= e_on_replace_trig)
+    end
+end
+
+box.ctl.on_schema_init(on_init_trig)

box.cfg{
    listen =             &n= bsp;=3D os.getenv("LISTEN"),
--
2.20.1 = (Apple Git-117)



= --Apple-Mail=_510DCC3A-A06C-448A-A80E-0862DFEAEDF9--