<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><br class=""><blockquote type="cite" class="">18 июля 2018 г., в 12:19, Sergey Petrenko <<a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a>> написал(а):<br class=""><br class="">Hi!<br class=""><br class="">18.07.2018 8:19, Konstantin Osipov пишет:<br class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">* Serge Petrenko <<a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a>> [18/07/16 15:14]:<br class=""><br class=""><blockquote type="cite" class="">Tarantoolctl enter didn't check whether connection to a socket was established<br class="">if a socket file existed. It just executed a local console.<br class="">Fix this by adding a check and an error, also add a test case.<br class=""><br class="">Closes #3364<br class=""> extra/dist/<a href="http://tarantoolctl.in" class="">tarantoolctl.in</a> | 19 ++++++++++++-------<br class=""> test/app-tap/tarantoolctl.test.lua | 29 ++++++++++++++++++++++++++++-<br class=""> 2 files changed, 40 insertions(+), 8 deletions(-)<br class=""><br class="">+ local status, err = pcall(function()<br class="">+ test:test("check error codes in case of enter", function(test_i)<br class="">+ test_i:plan(6)<br class="">+ check_ok(test_i, dir, 'enter', 'script', 1, nil, "Can't connect to")<br class="">+ check_ok(test_i, dir, 'start', 'script', 0)<br class="">+ os.execute(("kill $(cat %s/script.pid)"):format(dir))<br class=""></blockquote>I don't understand how this works. Simply sending<br class="">SIGTERM to tarantool would should it down gracefully and remove<br class="">the socket. If it doesn't, it's a bug. You should use SIGKILL if<br class=""></blockquote>Doing<br class=""> tarantoolctl start testctl<br class=""> kill (tarantool instance pid)<br class="">Leaves the socket intact on my pc.<br class="">tarantoolctl stop also first explicitly deletes the socket<br class="">file and then sends SIGTERM to the instance.<br class="">Don't know if it's ok.<br class=""></blockquote><br class="">Hi! As we discussed verbally, tarantool shouldn’t leave a unix socket after its exit, so it’s a bug.<br class="">I fixed it and rebased the patch on top of latest 1.10<br class="">The thing is that iproto thread should unlink the file (if any) upon its stop, but we never stop<br class="">iproto thread. I tried to implement a proper stop function, like the one I wrote for replication_shutdown(),<br class="">but couldn’t make it work.<br class="">So all I have is a simple function that is executed in tx thread and <br class="">calls evio_service_stop() on binary service, which, in turn, unlinks the socket.<br class="">I don’t know if it’s ok to call evio_service_stop() from another thread, so I need your comments.<br class=""><br class="">I also added some test cases to check for correct creation and deletion of unix socket under tarantoolctl.<br class="">Please see new diff below.<br class=""><br class=""><blockquote type="cite" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">you wanted to create a dead socket. But then again why do you need<br class="">to start an instance in order to get a dead socket? Can't you make<br class="">it simpler and create a dead socket with fio API?<br class=""></blockquote>Do you mean create a file with fio.open(sock, 'O_CREAT')?<br class="">It won't be a 'real' socket, but I tried it, and the tests seem to<br class="">work.<br class="">Because AFAIU there is no way to create a unix socket without<br class="">starting to listen on it.<br class="">That's what I'm doing now:<br class=""> test_i:plan(4)<br class=""> check_ok(test_i, dir, 'enter', 'script', 1, nil, "Can't connect to")<br class=""> local console_sock = 'script.control'<br class=""> console_sock = fio.pathjoin(fio.dirname(dir), console_sock)<br class=""> fio.open(console_sock, 'O_CREAT')<br class=""> check_ok(test_i, dir, 'enter', 'script', 1, nil, "Can't connect to")<br class=""> fio.unlink(console_sock)<br class=""></blockquote><br class=""> extra/dist/<a href="http://tarantoolctl.in" class="">tarantoolctl.in</a> | 19 +++++++++------<br class=""> src/box/<a href="http://box.cc" class="">box.cc</a> | 2 ++<br class=""> src/box/<a href="http://iproto.cc" class="">iproto.cc</a> | 16 +++++++++++++<br class=""> src/box/iproto.h | 3 +++<br class=""> test/app-tap/tarantoolctl.test.lua | 48 +++++++++++++++++++++++++++++++++++++-<br class=""> 5 files changed, 80 insertions(+), 8 deletions(-)<br class=""><br class="">diff --git a/extra/dist/<a href="http://tarantoolctl.in" class="">tarantoolctl.in</a> b/extra/dist/<a href="http://tarantoolctl.in" class="">tarantoolctl.in</a><br class="">index f0529734f..8d52a4d9c 100755<br class="">--- a/extra/dist/<a href="http://tarantoolctl.in" class="">tarantoolctl.in</a><br class="">+++ b/extra/dist/<a href="http://tarantoolctl.in" class="">tarantoolctl.in</a><br class="">@@ -638,15 +638,20 @@ local function enter()<br class=""> end<br class=""> return 1<br class=""> end<br class="">-<br class="">- local cmd = string.format(<br class="">- "require('console').connect('%s', { connect_timeout = %s })",<br class="">- console_sock, TIMEOUT_INFINITY<br class="">- )<br class="">-<br class="">- console.on_start(function(self) self:eval(cmd) end)<br class="">+ local status, ret<br class="">+ console.on_start(function(self)<br class="">+ status, ret = pcall(console.connect, console_sock,<br class="">+ {connect_timeout = TIMEOUT_INFINITY})<br class="">+ if not status then<br class="">+ log.error("Can't connect to %s (%s)", console_sock_path, ret)<br class="">+ self.running = false<br class="">+ end<br class="">+ end)<br class=""> console.on_client_disconnect(function(self) self.running = false end)<br class=""> console.start()<br class="">+ if not status then<br class="">+ return 1<br class="">+ end<br class=""> return 0<br class=""> end<br class=""> <br class="">diff --git a/src/box/<a href="http://box.cc" class="">box.cc</a> b/src/box/<a href="http://box.cc" class="">box.cc</a><br class="">index ee12d5738..adc3204bf 100644<br class="">--- a/src/box/<a href="http://box.cc" class="">box.cc</a><br class="">+++ b/src/box/<a href="http://box.cc" class="">box.cc</a><br class="">@@ -1606,6 +1606,8 @@ box_free(void)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>tuple_free();<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>port_free();<br class=""> #endif<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>/* If listening on a unix socket, unlink it. */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>iproto_stop_listen();<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>sequence_free();<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>gc_free();<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>engine_shutdown();<br class="">diff --git a/src/box/<a href="http://iproto.cc" class="">iproto.cc</a> b/src/box/<a href="http://iproto.cc" class="">iproto.cc</a><br class="">index bb7d2b868..0bd3f7a4f 100644<br class="">--- a/src/box/<a href="http://iproto.cc" class="">iproto.cc</a><br class="">+++ b/src/box/<a href="http://iproto.cc" class="">iproto.cc</a><br class="">@@ -2015,6 +2015,22 @@ iproto_listen(const char *uri)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>iproto_do_cfg(&cfg_msg);<br class=""> }<br class=""> <br class="">+<br class="">+/**<br class="">+ * Stop binary service.<br class="">+ *<br class="">+ * We don't have a proper iproto shutdown, iproto thread<br class="">+ * just dies and never stops listening on socket. This leaves<br class="">+ * dead unix sockets after tarantool. So we have to manually<br class="">+ * call evio_service_stop() to unink unix socket, if any.<br class="">+ * When iproto_shutdown() will be implemented, this function<br class="">+ * won't be needed anymore<br class="">+ */<br class="">+void<br class="">+iproto_stop_listen(void) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>evio_service_stop(&binary);<br class="">+}<br class="">+<br class=""> size_t<br class=""> iproto_mem_used(void)<br class=""> {<br class="">diff --git a/src/box/iproto.h b/src/box/iproto.h<br class="">index b9a6cf8f7..304af3b67 100644<br class="">--- a/src/box/iproto.h<br class="">+++ b/src/box/iproto.h<br class="">@@ -77,6 +77,9 @@ iproto_init();<br class=""> void<br class=""> iproto_listen(const char *uri);<br class=""> <br class="">+void<br class="">+iproto_stop_listen(void);<br class="">+<br class=""> void<br class=""> iproto_set_msg_max(int iproto_msg_max);<br class=""> <br class="">diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua<br class="">index 6946c8312..1ba31ccf6 100755<br class="">--- a/test/app-tap/tarantoolctl.test.lua<br class="">+++ b/test/app-tap/tarantoolctl.test.lua<br class="">@@ -113,6 +113,14 @@ local function tctl_wait(dir, name)<br class=""> end<br class=""> end<br class=""> <br class="">+local function wait_delete(path)<br class="">+ if path then<br class="">+ while fio.path.exists(path) do<br class="">+ fiber.sleep(0.001)<br class="">+ end<br class="">+ end<br class="">+end<br class="">+<br class=""> local function tctl_command(dir, cmd, args, name)<br class=""> local pid = nil<br class=""> if not fio.stat(fio.pathjoin(dir, '.tarantoolctl')) then<br class="">@@ -150,7 +158,7 @@ local function check_ok(test, dir, cmd, args, e_res, e_stdout, e_stderr)<br class=""> end<br class=""> <br class=""> local test = tap.test('tarantoolctl')<br class="">-test:plan(6)<br class="">+test:plan(7)<br class=""> <br class=""> -- basic start/stop test<br class=""> -- must be stopped afterwards<br class="">@@ -250,6 +258,44 @@ do<br class=""> end<br class=""> end<br class=""> <br class="">+-- check enter<br class="">+do<br class="">+ local dir = fio.tempdir()<br class="">+<br class="">+ local code = [[ box.cfg{} ]]<br class="">+ create_script(dir, 'script.lua', code)<br class="">+<br class="">+ local status, err = pcall(function()<br class="">+ test:test("check error codes in case of enter", function(test_i)<br class="">+ test_i:plan(10)<br class="">+ check_ok(test_i, dir, 'enter', 'script', 1, nil, "Can't connect to")<br class="">+ local console_sock = 'script.control'<br class="">+ console_sock = fio.pathjoin(dir, console_sock)<br class="">+ test_i:is(fio.path.exists(console_sock), false, "directory clean")<br class="">+ check_ok(test_i, dir, 'start', 'script', 0)<br class="">+ tctl_wait(dir, 'script')<br class="">+ test_i:is(fio.path.exists(console_sock), true,<br class="">+ "unix socket created")<br class="">+ check_ok(test_i, dir, 'stop', 'script', 0)<br class="">+ --wait_delete(console_sock)<br class="">+ test_i:is(fio.path.exists(console_sock), false,<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> "remove unix socket upon exit")<br class="">+ fio.open(console_sock, 'O_CREAT')<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> test_i:is(fio.path.exists(console_sock), true, "file created")<br class="">+ check_ok(test_i, dir, 'enter', 'script', 1, nil, "Can't connect to")<br class="">+ fio.unlink(console_sock)<br class="">+ end)<br class="">+ end)<br class="">+<br class="">+ cleanup_instance(dir, 'script')<br class="">+ recursive_rmdir(dir)<br class="">+<br class="">+ if status == false then<br class="">+ print(("Error: %s"):format(err))<br class="">+ os.exit()<br class="">+ end<br class="">+end<br class="">+<br class=""> -- check basic help<br class=""> do<br class=""> local dir = fio.tempdir()<br class="">-- <br class="">2.15.2 (Apple Git-101.1)</body></html>