From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 1E3BA4696C3 for ; Wed, 15 Apr 2020 04:57:40 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: <20200412103721.GV3072@uranus> Date: Wed, 15 Apr 2020 04:57:39 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <0920770A-F34B-4874-A206-68E7A71D09EF@tarantool.org> References: <20200411231034.95903-1-roman.habibov@tarantool.org> <20200412103721.GV3072@uranus> Subject: Re: [Tarantool-patches] [PATCH] console: check on_shutdown() before exit List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review. > On Apr 12, 2020, at 13:37, Cyrill Gorcunov wrote: >=20 > On Sun, Apr 12, 2020 at 02:10:34AM +0300, Roman Khabibov wrote: >> Add check that on_shutdown() triggers were called before exit, >> because in case of EOF or Ctrl+D (no signals) they were ignored. >>=20 >> Closes #4703 >=20 > Ok, I don't see any obvious problems with the patch. Only a nit > due to coding style below. > ... >> diff --git a/test/box/on_shutdown.test.lua = b/test/box/on_shutdown.test.lua >> index 2a9143404..795887202 100644 >> --- a/test/box/on_shutdown.test.lua >> +++ b/test/box/on_shutdown.test.lua >> @@ -65,3 +65,20 @@ test_run:grep_log('test', 'signal', nil, = {noreset=3Dtrue}) > ... >> +on_shutdown_cmd =3D "box.ctl.on_shutdown(function() local fio =3D = require('fio') ".. >> + "fio.open('"..file_name.."', {'O_CREAT', 'O_TRUNC', 'O_WRONLY'}, = 777):clo".. >> + "se() end)"; >=20 > This is ugly as hell. Can we please do not split "close". Something = like >=20 > on_shutdown_cmd =3D "box.ctl.on_shutdown(function() local fio =3D = require('fio') ".. > "fio.open('"..file_name.."', ".. > "{'O_CREAT', 'O_TRUNC', 'O_WRONLY'}, 777):close() end)"; >=20 > Or somehow more readable? Maybe like >=20 > on_shutdown_cmd =3D "box.ctl.on_shutdown(function() local fio =3D = require('fio') ".. > "local flags =3D {'O_CREAT', 'O_TRUNC', 'O_WRONLY'}" .. > "local f =3D fio.open(mode, 777)" .. > "f:close()" >=20 > (I'm sure I've done some typos here but you get the idea) diff --git a/test/box/on_shutdown.test.lua = b/test/box/on_shutdown.test.lua index 2a9143404..d033a7ac5 100644 --- a/test/box/on_shutdown.test.lua +++ b/test/box/on_shutdown.test.lua @@ -65,3 +65,20 @@ test_run:grep_log('test', 'signal', nil, = {noreset=3Dtrue}) test_run:cmd("stop server test") test_run:cmd("cleanup server test") test_run:cmd("delete server test") + +-- +-- gh-4703: Make sure that on_shutdown triggers are executed after +-- EOF. +-- +fio =3D require("fio") +file_name =3D "on_shutdown_triggered.txt" +test_run:cmd("setopt delimiter ';'"); +on_shutdown_cmd =3D "box.ctl.on_shutdown(function() local fio =3D = require('fio') ".. + "fio.open('"..file_name.."', ".. + "{'O_CREAT', 'O_TRUNC', 'O_WRONLY'}, 777):close() = end)"; +test_run:cmd("setopt delimiter ''"); +server =3D io.popen('tarantool -i', 'w') +server:write(on_shutdown_cmd) +server:close() +fio.path.lexists(file_name) =3D=3D true +os.remove(file_name) commit 6466cf42ae5755826af0c616243106a696da044a (HEAD -> = romanhabibov/gh-4703_on_shutdown) Author: Roman Khabibov Date: Fri Apr 10 14:31:17 2020 +0300 console: check on_shutdown() before exit =20 Add check that on_shutdown() triggers were called before exit, because in case of EOF or Ctrl+D (no signals) they were ignored. =20 Closes #4703 diff --git a/src/main.cc b/src/main.cc index bb0794dfe..a2cea3d40 100644 --- a/src/main.cc +++ b/src/main.cc @@ -902,6 +902,12 @@ main(int argc, char **argv) =20 if (start_loop) say_crit("exiting the event loop"); + /* + * If Tarantool was stopped using Ctrl+D, then we need to + * call on_shutdown triggers, because Ctrl+D causes not + * any signals. + */ + tarantool_exit(exit_code); /* freeing resources */ tarantool_free(); return exit_code; diff --git a/test/box/on_shutdown.result b/test/box/on_shutdown.result index ccbdf45cb..8ca8fed2b 100644 --- a/test/box/on_shutdown.result +++ b/test/box/on_shutdown.result @@ -167,3 +167,45 @@ test_run:cmd("delete server test") --- - true ... +-- +-- gh-4703: Make sure that on_shutdown triggers are executed after +-- EOF. +-- +fio =3D require("fio") +--- +... +file_name =3D "on_shutdown_triggered.txt" +--- +... +test_run:cmd("setopt delimiter ';'"); +--- +- true +... +on_shutdown_cmd =3D "box.ctl.on_shutdown(function() local fio =3D = require('fio') ".. + "fio.open('"..file_name.."', ".. + "{'O_CREAT', 'O_TRUNC', 'O_WRONLY'}, 777):close() = end)"; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +server =3D io.popen('tarantool -i', 'w') +--- +... +server:write(on_shutdown_cmd) +--- +- true +... +server:close() +--- +- true +... +fio.path.lexists(file_name) =3D=3D true +--- +- true +... +os.remove(file_name) +--- +- true +... diff --git a/test/box/on_shutdown.test.lua = b/test/box/on_shutdown.test.lua index 2a9143404..d033a7ac5 100644 --- a/test/box/on_shutdown.test.lua +++ b/test/box/on_shutdown.test.lua @@ -65,3 +65,20 @@ test_run:grep_log('test', 'signal', nil, = {noreset=3Dtrue}) test_run:cmd("stop server test") test_run:cmd("cleanup server test") test_run:cmd("delete server test") + +-- +-- gh-4703: Make sure that on_shutdown triggers are executed after +-- EOF. +-- +fio =3D require("fio") +file_name =3D "on_shutdown_triggered.txt" +test_run:cmd("setopt delimiter ';'"); +on_shutdown_cmd =3D "box.ctl.on_shutdown(function() local fio =3D = require('fio') ".. + "fio.open('"..file_name.."', ".. + "{'O_CREAT', 'O_TRUNC', 'O_WRONLY'}, 777):close() = end)"; +test_run:cmd("setopt delimiter ''"); +server =3D io.popen('tarantool -i', 'w') +server:write(on_shutdown_cmd) +server:close() +fio.path.lexists(file_name) =3D=3D true +os.remove(file_name)=