Tarantool development patches archive
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: tml <tarantool-patches@freelists.org>
Cc: Konstantin Osipov <kostja@tarantool.org>,
	Alexander Turenko <alexander.turenko@tarantool.org>,
	Cyrill Gorcunov <gorcunov@gmail.com>
Subject: [tarantool-patches] [PATCH 4/6] box/console: Fix hang in remote console lua mode
Date: Fri,  6 Sep 2019 00:28:13 +0300	[thread overview]
Message-ID: <20190905212815.7311-5-gorcunov@gmail.com> (raw)
In-Reply-To: <20190905212815.7311-1-gorcunov@gmail.com>

If we change output mode on remote machine via
text-based session

 node a (server)
 ------
 require('console').listen('unix/:/tmp/X.sock')
 ...

 node b (client)
 ------
 require('console').connect('unix/:/tmp/X.sock')
 connected to unix/:/tmp/X.sock
 ...
 unix/:/tmp/X.sock> \set output lua

the client get stuck forever, it is because the text
wire protocol waits for yaml eos terminator which of
course never hit the peer, because lua mode uses own
one.

Thus to fix this problem we have to preprocess the text
we're passing to the server, just like we do in local
console. So we reuse command parsing code and remember
current output terminator in text_connection_mt instance.

Another problem is that named default output mode. There
could be a mixed environment where server operates in
default lua mode but client connects with yaml mode. To
break a tie we yield "\set output" command with current
output mode when establishing a connection. Since the
output format is per-session bound this doesn't affect
any other connections on a server.

Part-of #3834
---
v3:
 - Use current_output() helper in current_eos()
   instead of local variable
 - Rename output_cmd_string() to output_to_cmd_string()
 - Use string.format in output_to_cmd_string
 - Leave preprocess_eval inside metatable because moving
   this method out of text_connection_mt is really confusing,
   everything specific to console text connection should sit
   inside text_connection_mt for better design
 - More comments

 src/box/lua/console.lua | 57 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 04cca35ca..a6ff0e659 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -157,6 +157,24 @@ local function current_output()
     return d
 end
 
+--
+-- Return current EOS value for currently
+-- active output format.
+local function current_eos()
+    return output_eos[current_output()["fmt"]]
+end
+
+--
+-- Map output format descriptor into a "\set" command.
+local function output_to_cmd_string(desc)
+    if desc["opts"] then
+        string.format("\\set output %s,%s", desc["fmt"], desc["opts"])
+    else
+        string.format("\\set output %s", desc["fmt"])
+    end
+    return cmd
+end
+
 local function format(status, ...)
     local d = current_output()
     return output_handlers[d["fmt"]](status, d["opts"], ...)
@@ -346,15 +364,39 @@ local text_connection_mt = {
         -- @retval     nil Error.
         --
         read = function(self)
-            local ret = self._socket:read(output_eos["yaml"])
+            local ret = self._socket:read(self.eos)
             if ret and ret ~= '' then
                 return ret
             end
         end,
         --
+        -- The command might modify EOS so
+        -- we have to parse and preprocess it
+        -- first to not stuck in :read() forever.
+        --
+        -- Same time the EOS is per connection
+        -- value so we keep it inside the metatable
+        -- instace, and because we can't use same
+        -- name as output_eos() function we name
+        -- it simplier as self.eos.
+        preprocess_eval = function(self, text)
+            local command = get_command(text)
+            if command == nil then
+                return
+            end
+            local nr_items, items = parse_operators(command)
+            if nr_items == 3 then
+                local err, fmt, opts = output_parse(items[3])
+                if not err then
+                    self.eos = output_eos[fmt]
+                end
+            end
+        end,
+        --
         -- Write + Read.
         --
         eval = function(self, text)
+            self:preprocess_eval(text)
             text = text..'$EOF$\n'
             if not self:write(text) then
                 error(self:set_error())
@@ -409,9 +451,18 @@ local function wrap_text_socket(connection, url, print_f)
         host = url.host or 'localhost',
         port = url.service,
         print_f = print_f,
+        eos = current_eos(),
     }, text_connection_mt)
-    if not conn:write('require("console").delimiter("$EOF$")\n') or
-       not conn:read() then
+    --
+    -- Prepare the connection: setup EOS symbol
+    -- by executing the \set command on remote machine
+    -- explicitly, and then setup a delimiter.
+    local cmd_set_output = output_to_cmd_string(current_output()) .. '\n'
+    if not conn:write(cmd_set_output) or not conn:read() then
+        conn:set_error()
+    end
+    local cmd_delimiter = 'require("console").delimiter("$EOF$")\n'
+    if not conn:write(cmd_delimiter) or not conn:read() then
         conn:set_error()
     end
     return conn
-- 
2.20.1

  parent reply	other threads:[~2019-09-05 21:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05 21:28 [tarantool-patches] [PATCH v3 0/6] box/console: Improve lua mode for remote console Cyrill Gorcunov
2019-09-05 21:28 ` [tarantool-patches] [PATCH 1/6] box/console: Add mapping for direct symbols Cyrill Gorcunov
2019-09-09 15:11   ` [tarantool-patches] " Sergey Ostanevich
2019-09-09 15:54     ` [tarantool-patches] " Cyrill Gorcunov
2019-09-10  3:59       ` [tarantool-patches] Re[2]: [tarantool-patches] " Sergey Ostanevich
2019-09-05 21:28 ` [tarantool-patches] [PATCH 2/6] box/console: Add explicit output EOS mapping Cyrill Gorcunov
2019-09-05 21:28 ` [tarantool-patches] [PATCH 3/6] box/console: Refactor command handling Cyrill Gorcunov
2019-09-05 21:28 ` Cyrill Gorcunov [this message]
2019-09-05 21:28 ` [tarantool-patches] [PATCH 5/6] box/console: Provide console.eos() api Cyrill Gorcunov
2019-09-05 21:28 ` [tarantool-patches] [PATCH 6/6] box/console: Rename output_parse to parse_output Cyrill Gorcunov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190905212815.7311-5-gorcunov@gmail.com \
    --to=gorcunov@gmail.com \
    --cc=alexander.turenko@tarantool.org \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH 4/6] box/console: Fix hang in remote console lua mode' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox