From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Serge Petrenko Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_92E59EB4-D692-44CC-A885-D74DF34E7B47" Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: Re: [tarantool-patches] [PATCH v5 2/3] lua: patch os.exit() to execute on_shutdown triggers. Date: Tue, 29 Jan 2019 17:49:33 +0300 In-Reply-To: <20190128084533.6qz6cxlsvr6c27rd@esperanza> References: <94ded865ada42cee1b807c738dbdd31399a06914.1548430046.git.sergepetrenko@tarantool.org> <20190128084533.6qz6cxlsvr6c27rd@esperanza> To: Vladimir Davydov Cc: Konstantin Osipov , tarantool-patches@freelists.org List-ID: --Apple-Mail=_92E59EB4-D692-44CC-A885-D74DF34E7B47 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi! Thankyou for review. I addressed your comments. > 28 =D1=8F=D0=BD=D0=B2. 2019 =D0=B3., =D0=B2 11:45, Vladimir Davydov = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0= ): >=20 > On Fri, Jan 25, 2019 at 06:41:33PM +0300, Serge Petrenko wrote: >> Note that the triggers will not be run if tarantool receives a fatal >> signal: `SIGSEGV`, `SIGABORT` or any signal causing immediate program >> termination. >>=20 >=20 >> Closes #1607 >=20 > Should go before the TarantoolBot request. >=20 Fixed. >> --- >> extra/exports | 1 + >> src/lua/init.lua | 7 +++++++ >> src/main.cc | 10 ++++++---- >> src/main.h | 3 +++ >> 4 files changed, 17 insertions(+), 4 deletions(-) >=20 > A test is missing. Please add. >=20 Added a test >>=20 >>=20 >> is_shutting_down =3D true; >> + exit_code =3D code; >> fiber_wakeup(on_shutdown_fiber); >=20 > os.exit() should exit even if the fiber doesn't yield, but the = following > code will hang, which is unexpected. Please fix. >=20 > os.exit() > while true do end Fixed I pushed the fixes on the branch https://github.com/tarantool/tarantool/tree/sp/gh-1607-on-exit-triggers Incremental diff is below. diff --git a/src/lua/init.lua b/src/lua/init.lua index a961db328..9fd56f483 100644 --- a/src/lua/init.lua +++ b/src/lua/init.lua @@ -52,9 +52,14 @@ dostring =3D function(s, ...) return chunk(...) end =20 +local fiber =3D require("fiber") os.exit =3D function(code) code =3D (type(code) =3D=3D 'number') and code or 0 ffi.C.tarantool_exit(code) + -- Make sure we yield even if the code after + -- os.exit() never yields. After on_shutdown + -- fiber completes, we will never wake up again. + while true do fiber.yield() end end =20 local function uptime() diff --git a/test/box/misc.result b/test/box/misc.result index 6912915c1..97189ecbb 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -1321,3 +1321,92 @@ box.space.shutdown:select{} box.space.shutdown:drop() --- ... +-- Check that os.exit invokes triggers +fiber =3D require("fiber") +--- +... +test_run:cmd("create server test with script=3D'box/proxy.lua'") +--- +- true +... +test_run:cmd("start server test") +--- +- true +... +logfile =3D test_run:eval("test", "box.cfg.log")[1] +--- +... +test_run:cmd("stop server test") +--- +- true +... +-- clean up any leftover logs +require("fio").unlink(logfile) +--- +- true +... +test_run:cmd("start server test") +--- +- true +... +test_run:cmd("switch test") +--- +- true +... +_ =3D box.ctl.on_shutdown(function() print("on_shutdown 5") end) +--- +... +-- Check that we don't hang infinitely after os.exit() +-- even if the following code doesn't yield. +fiber =3D require("fiber") +--- +... +_ =3D fiber.create(function() fiber.sleep(0.05) os.exit() while true do = end end) +--- +... +test_run:cmd("switch default") +--- +- true +... +fiber.sleep(0.1) +--- +... +-- The server should be already stopped by os.exit(), +-- but start doesn't work without a prior call to stop. +test_run:cmd("stop server test") +--- +- true +... +test_run:cmd("start server test") +--- +- true +... +test_run:cmd("switch test") +--- +- true +... +test_run:grep_log('test', 'on_shutdown 5', nil, {noreset=3Dtrue}) +--- +- on_shutdown 5 +... +-- make sure we exited because of os.exit(), not a signal. +test_run:grep_log('test', 'signal', nil, {noreset=3Dtrue}) +--- +- null +... +test_run:cmd("switch default") +--- +- true +... +test_run:cmd("stop server test") +--- +- true +... +test_run:cmd("cleanup server test") +--- +- true +... +test_run:cmd("delete server test") +--- +- true +... diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua index f1c9d8e8c..18128b299 100644 --- a/test/box/misc.test.lua +++ b/test/box/misc.test.lua @@ -384,3 +384,33 @@ test_run:grep_log('default', 'on_shutdown 3', nil, = {noreset=3Dtrue}) test_run:grep_log('default', 'on_shutdown 4', nil, {noreset=3Dtrue}) box.space.shutdown:select{} box.space.shutdown:drop() + +-- Check that os.exit invokes triggers +fiber =3D require("fiber") +test_run:cmd("create server test with script=3D'box/proxy.lua'") +test_run:cmd("start server test") +logfile =3D test_run:eval("test", "box.cfg.log")[1] +test_run:cmd("stop server test") +-- clean up any leftover logs +require("fio").unlink(logfile) +test_run:cmd("start server test") +test_run:cmd("switch test") +_ =3D box.ctl.on_shutdown(function() print("on_shutdown 5") end) +-- Check that we don't hang infinitely after os.exit() +-- even if the following code doesn't yield. +fiber =3D require("fiber") +_ =3D fiber.create(function() fiber.sleep(0.05) os.exit() while true do = end end) +test_run:cmd("switch default") +fiber.sleep(0.1) +-- The server should be already stopped by os.exit(), +-- but start doesn't work without a prior call to stop. +test_run:cmd("stop server test") +test_run:cmd("start server test") +test_run:cmd("switch test") +test_run:grep_log('test', 'on_shutdown 5', nil, {noreset=3Dtrue}) +-- make sure we exited because of os.exit(), not a signal. +test_run:grep_log('test', 'signal', nil, {noreset=3Dtrue}) +test_run:cmd("switch default") +test_run:cmd("stop server test") +test_run:cmd("cleanup server test") +test_run:cmd("delete server test")= --Apple-Mail=_92E59EB4-D692-44CC-A885-D74DF34E7B47 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Hi! Thankyou for review. I = addressed your comments.

28 =D1=8F=D0=BD=D0=B2. 2019 =D0=B3., =D0=B2 = 11:45, Vladimir Davydov <vdavydov.dev@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0= =B0=D0=BB(=D0=B0):

On Fri, Jan 25, 2019 at = 06:41:33PM +0300, Serge Petrenko wrote:
Note = that the triggers will not be run if tarantool receives a fatal
signal: `SIGSEGV`, `SIGABORT` or any signal causing immediate = program
termination.


Closes #1607

Should go before the TarantoolBot = request.


Fixed.

---
extra/exports    |  1 +
src/lua/init.lua |  7 +++++++
src/main.cc     =  | 10 ++++++----
src/main.h       | =  3 +++
4 files changed, 17 insertions(+), 4 = deletions(-)

A test is = missing. Please add.


Added a test



is_shutting_down =3D true;
+ = exit_code =3D code;
= fiber_wakeup(on_shutdown_fiber);

os.exit() should exit even if the fiber doesn't yield, but = the following
code will hang, which is unexpected. Please = fix.

os.exit()
while = true do end

Fixed

I pushed the fixes on the branch
https://github.com/tarantool/tarantool/tree/sp/gh-1607-on-exit-= triggers
Incremental diff is below.

diff --git a/src/lua/init.lua b/src/lua/init.lua
index a961db328..9fd56f483 100644
--- = a/src/lua/init.lua
+++ b/src/lua/init.lua
@@ = -52,9 +52,14 @@ dostring =3D function(s, ...)
  =    return chunk(...)
 end
 
+local fiber =3D require("fiber")
 os.exit =3D function(code)
    =  code =3D (type(code) =3D=3D 'number') and code or 0
     ffi.C.tarantool_exit(code)
+    -- Make sure we yield even if the code = after
+    -- os.exit() never yields. After = on_shutdown
+    -- fiber completes, we = will never wake up again.
+    while true = do fiber.yield() end
 end
 
 local function uptime()
diff --git = a/test/box/misc.result b/test/box/misc.result
index = 6912915c1..97189ecbb 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1321,3 +1321,92 = @@ box.space.shutdown:select{}
 box.space.shutdown:drop()
 ---
 ...
+-- Check that os.exit invokes = triggers
+fiber =3D require("fiber")
+---
+...
+test_run:cmd("create server test with = script=3D'box/proxy.lua'")
+---
+- true
+...
+test_run:cmd("start server test")
+---
+- true
+...
+logfile =3D test_run:eval("test", "box.cfg.log")[1]
+---
+...
+test_run:cmd("stop = server test")
+---
+- true
+...
+-- clean up any leftover logs
+require("fio").unlink(logfile)
+---
+- true
+...
+test_run:cmd("start = server test")
+---
+- true
+...
+test_run:cmd("switch test")
+---
+- true
+...
+_ = =3D box.ctl.on_shutdown(function() print("on_shutdown 5") end)
+---
+...
+-- Check that we don't = hang infinitely after os.exit()
+-- even if the following = code doesn't yield.
+fiber =3D require("fiber")
+---
+...
+_ =3D = fiber.create(function() fiber.sleep(0.05) os.exit() while true do end = end)
+---
+...
+test_run:cmd("switch default")
+---
+- true
+...
+fiber.sleep(0.1)
+---
+...
+-- The server should = be already stopped by os.exit(),
+-- but start doesn't = work without a prior call to stop.
+test_run:cmd("stop = server test")
+---
+- true
+...
+test_run:cmd("start server test")
+---
+- true
+...
+test_run:cmd("switch test")
+---
+- true
+...
+test_run:grep_log('test', 'on_shutdown 5', nil, = {noreset=3Dtrue})
+---
+- on_shutdown 5
+...
+-- make sure we exited because of = os.exit(), not a signal.
+test_run:grep_log('test', = 'signal', nil, {noreset=3Dtrue})
+---
+- = null
+...
+test_run:cmd("switch default")
+---
+- true
+...
+test_run:cmd("stop server test")
+---
+- true
+...
+test_run:cmd("cleanup= server test")
+---
+- true
+...
+test_run:cmd("delete server test")
+---
+- true
+...
diff --git a/test/box/misc.test.lua = b/test/box/misc.test.lua
index f1c9d8e8c..18128b299 = 100644
--- a/test/box/misc.test.lua
+++ = b/test/box/misc.test.lua
@@ -384,3 +384,33 @@ = test_run:grep_log('default', 'on_shutdown 3', nil, {noreset=3Dtrue})
 test_run:grep_log('default', 'on_shutdown 4', nil, = {noreset=3Dtrue})
 box.space.shutdown:select{}
 box.space.shutdown:drop()
+
+-- Check that os.exit invokes triggers
+fiber = =3D require("fiber")
+test_run:cmd("create server test = with script=3D'box/proxy.lua'")
+test_run:cmd("start = server test")
+logfile =3D test_run:eval("test", = "box.cfg.log")[1]
+test_run:cmd("stop server test")
+-- clean up any leftover logs
+require("fio").unlink(logfile)
+test_run:cmd("start server test")
+test_run:cmd("switch test")
+_ =3D = box.ctl.on_shutdown(function() print("on_shutdown 5") end)
+-- Check that we don't hang infinitely after os.exit()
+-- even if the following code doesn't yield.
+fiber =3D require("fiber")
+_ =3D = fiber.create(function() fiber.sleep(0.05) os.exit() while true do end = end)
+test_run:cmd("switch default")
+fiber.sleep(0.1)
+-- The server should be = already stopped by os.exit(),
+-- but start doesn't work = without a prior call to stop.
+test_run:cmd("stop server = test")
+test_run:cmd("start server test")
+test_run:cmd("switch test")
+test_run:grep_log('test', 'on_shutdown 5', nil, = {noreset=3Dtrue})
+-- make sure we exited because of = os.exit(), not a signal.
+test_run:grep_log('test', = 'signal', nil, {noreset=3Dtrue})
+test_run:cmd("switch = default")
+test_run:cmd("stop server test")
+test_run:cmd("cleanup server test")
+test_run:cmd("delete server test")= --Apple-Mail=_92E59EB4-D692-44CC-A885-D74DF34E7B47--