Hi! Thankyou for review. I addressed your comments. > 28 янв. 2019 г., в 11:45, Vladimir Davydov написал(а): > > 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 = true; >> + exit_code = 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 = function(s, ...) return chunk(...) end +local fiber = require("fiber") os.exit = function(code) code = (type(code) == '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 = require("fiber") +--- +... +test_run:cmd("create server test with script='box/proxy.lua'") +--- +- true +... +test_run:cmd("start server test") +--- +- true +... +logfile = 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 +... +_ = 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 = require("fiber") +--- +... +_ = 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=true}) +--- +- on_shutdown 5 +... +-- make sure we exited because of os.exit(), not a signal. +test_run:grep_log('test', 'signal', nil, {noreset=true}) +--- +- 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=true}) test_run:grep_log('default', 'on_shutdown 4', nil, {noreset=true}) box.space.shutdown:select{} box.space.shutdown:drop() + +-- Check that os.exit invokes triggers +fiber = require("fiber") +test_run:cmd("create server test with script='box/proxy.lua'") +test_run:cmd("start server test") +logfile = 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") +_ = 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 = require("fiber") +_ = 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=true}) +-- make sure we exited because of os.exit(), not a signal. +test_run:grep_log('test', 'signal', nil, {noreset=true}) +test_run:cmd("switch default") +test_run:cmd("stop server test") +test_run:cmd("cleanup server test") +test_run:cmd("delete server test")