[Tarantool-patches] [PATCH 0/3] lua: fix tarantool -e always enters interactive mode

[Tarantool-patches] [PATCH 0/3] lua: fix tarantool -e always enters interactive mode

[Artem Starshov via Tarantool-patches]

Issue: https://github.com/tarantool/tarantool/issues/5040
Branch: https://github.com/tarantool/tarantool/tree/artemreyt/gh-5040-interactive-mode-bug

First commit is the same as in gh-4983 (-e assert false).

Artem Starshov (3):
  test: add separated module for proccess operations with timeout
  core: add setting error injections via env
  lua: fix tarantool -e always enters interactive mode

 src/lib/core/errinj.c                         |  26 ++++
 src/lib/core/errinj.h                         |   6 +
 src/lua/init.c                                |  28 +++-
 src/main.cc                                   |   1 +
 ...40-always-enters-interactive-mode.test.lua | 121 ++++++++++++++++++
 test/app-tap/lua/process_timeout.lua          |  59 +++++++++
 test/app-tap/suite.ini                        |   2 +-
 test/box/errinj.result                        |   1 +
 8 files changed, 239 insertions(+), 5 deletions(-)
 create mode 100755 test/app-tap/gh-5040-always-enters-interactive-mode.test.lua
 create mode 100644 test/app-tap/lua/process_timeout.lua

-- 
2.28.0

[Tarantool-patches] [PATCH 1/3] test: add separated module for proccess operations with timeout

[Artem Starshov via Tarantool-patches]

Functions with timeout transferred to module test/app-tap/lua/process_timeout.lua
for further using in other tests.
(from test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua)

Follows up #4983
---
 test/app-tap/lua/process_timeout.lua | 59 ++++++++++++++++++++++++++++
 test/app-tap/suite.ini               |  2 +-
 2 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 test/app-tap/lua/process_timeout.lua

diff --git a/test/app-tap/lua/process_timeout.lua b/test/app-tap/lua/process_timeout.lua
new file mode 100644
index 000000000..1ca3636b9
--- /dev/null
+++ b/test/app-tap/lua/process_timeout.lua
@@ -0,0 +1,59 @@
+local fiber = require('fiber')
+local clock = require('clock')
+local ffi = require('ffi')
+local fio = require('fio')
+local errno = require('errno')
+
+-- For process_is_alive.
+ffi.cdef([[
+    int kill(pid_t pid, int sig);
+]])
+
+--- Verify whether a process is alive.
+local function process_is_alive(pid)
+    local rc = ffi.C.kill(pid, 0)
+    return rc == 0 or errno() ~= errno.ESRCH
+end
+
+--- Returns true if process completed before timeout, false otherwise.
+local function wait_process_completion(pid, timeout)
+    local start_time = clock.monotonic()
+    local process_completed = false
+    while clock.monotonic() - start_time < timeout do
+        if not process_is_alive(pid) then
+            process_completed = true
+            break
+        end
+        fiber.sleep(0.01)
+    end
+    return process_completed
+end
+
+--- Open file on reading with timeout.
+local function open_with_timeout(filename, timeout)
+    local fh
+    local start_time = clock.monotonic()
+    while not fh and clock.monotonic() - start_time < timeout do
+        fh = fio.open(filename, {'O_RDONLY'})
+        fiber.sleep(0.01)
+    end
+    return fh
+end
+
+--- Try to read from file with timeout with interval.
+local function read_with_timeout(fh, timeout, interval)
+    local data = ''
+    local start_time = clock.monotonic()
+    while #data == 0 and clock.monotonic() - start_time < timeout do
+        data = fh:read()
+        if #data == 0 then fiber.sleep(interval) end
+    end
+    return data
+end
+
+return {
+    process_is_alive = process_is_alive,
+    wait_process_completion = wait_process_completion,
+    open_with_timeout = open_with_timeout,
+    read_with_timeout = read_with_timeout
+}
diff --git a/test/app-tap/suite.ini b/test/app-tap/suite.ini
index 2f16128f9..6e27f88d6 100644
--- a/test/app-tap/suite.ini
+++ b/test/app-tap/suite.ini
@@ -1,7 +1,7 @@
 [default]
 core = app
 description = application server tests (TAP)
-lua_libs = lua/require_mod.lua lua/serializer_test.lua
+lua_libs = lua/require_mod.lua lua/serializer_test.lua lua/process_timeout.lua
 is_parallel = True
 pretest_clean = True
 use_unix_sockets_iproto = True
-- 
2.28.0

[Tarantool-patches] [PATCH 2/3] core: add setting error injections via env

[Artem Starshov via Tarantool-patches]

Sometimes, it's useful to set error injections via environment
variables and this commit adds this opportunity.

e.g: `ERRINJ_WAL_WRITE=true tarantool` will be launched with
ERRINJ_WAL_WRITE setted to true.

Errinjs with bool parameters can should be set as "true", "TRUE",
"false" or "FALSE".

Errinjs with int or double parameters should be whole valid ("123s" is invalid).
e.g. for int or double: "123", "-1", "2.34", "+2.34".
---
 src/lib/core/errinj.c | 26 ++++++++++++++++++++++++++
 src/lib/core/errinj.h |  5 +++++
 src/main.cc           |  1 +
 3 files changed, 32 insertions(+)

diff --git a/src/lib/core/errinj.c b/src/lib/core/errinj.c
index d3aa0ca4f..576811073 100644
--- a/src/lib/core/errinj.c
+++ b/src/lib/core/errinj.c
@@ -66,3 +66,29 @@ int errinj_foreach(errinj_cb cb, void *cb_ctx) {
 	}
 	return 0;
 }
+
+void errinj_scan_env() {
+    for (enum errinj_id i = 0 ; i < errinj_id_MAX ; i++) {
+        struct errinj *inj = &errinjs[i];
+        const char *env_value = getenv(inj->name);
+        if (!env_value || *env_value == '\0')
+            continue;
+
+        if (inj->type == ERRINJ_INT) {
+            char *end;
+            int64_t int_value = strtoll(env_value, &end, 10);
+            if (*end == '\0')
+                inj->iparam = int_value;
+        } else if (inj->type == ERRINJ_BOOL) {
+            if (strcmp(env_value, "false") == 0 || strcmp(env_value, "FALSE"))
+                inj->bparam = false;
+            else if (strcmp(env_value, "true") == 0 || strcmp(env_value, "TRUE"))
+                inj->bparam = true;
+        } else if (inj->type == ERRINJ_DOUBLE) {
+            char *end;
+            double double_value = strtod(env_value, &end);
+            if (*end == '\0')
+                inj->dparam = double_value;
+        }
+    }
+}
diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
index 814c57c2e..dc0657614 100644
--- a/src/lib/core/errinj.h
+++ b/src/lib/core/errinj.h
@@ -168,6 +168,11 @@ typedef int (*errinj_cb)(struct errinj *e, void *cb_ctx);
 int
 errinj_foreach(errinj_cb cb, void *cb_ctx);
 
+/**
+ * Set injections by scanning ERRINJ_$(NAME) in environment variables
+ */
+void errinj_scan_env();
+
 #ifdef NDEBUG
 #  define ERROR_INJECT(ID, CODE)
 #  define ERROR_INJECT_WHILE(ID, CODE)
diff --git a/src/main.cc b/src/main.cc
index 2fce81bb3..2d85c0b24 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -710,6 +710,7 @@ main(int argc, char **argv)
 	memtx_tx_manager_init();
 	crypto_init();
 	systemd_init();
+	errinj_scan_env();
 	tarantool_lua_init(tarantool_bin, main_argc, main_argv);
 
 	start_time = ev_monotonic_time();
-- 
2.28.0

[Tarantool-patches] [PATCH 3/3] lua: fix tarantool -e always enters interactive mode

[Artem Starshov via Tarantool-patches]

The reason why tarantool -e always enters interactive mode is that
statement after option -e isn't considered as a script.

In man PUC-Rio lua there are different names for statement -e (stat)
and script, but they have the same behavior regarding interactive
mode. (Also cases, when interpreter loads stdin, have the same behaviour).

Fixes #5040
---
 src/lib/core/errinj.h                         |   1 +
 src/lua/init.c                                |  28 +++-
 ...40-always-enters-interactive-mode.test.lua | 121 ++++++++++++++++++
 test/box/errinj.result                        |   1 +
 4 files changed, 147 insertions(+), 4 deletions(-)
 create mode 100755 test/app-tap/gh-5040-always-enters-interactive-mode.test.lua

diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
index dc0657614..89d2d3314 100644
--- a/src/lib/core/errinj.h
+++ b/src/lib/core/errinj.h
@@ -149,6 +149,7 @@ struct errinj {
 	_(ERRINJ_AUTO_UPGRADE, ERRINJ_BOOL, {.bparam = false})\
 	_(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_APPLIER_SLOW_ACK, ERRINJ_BOOL, {.bparam = false}) \
+	_(ERRINJ_STDIN_ISATTY, ERRINJ_INT, {.iparam = -1}) \
 
 ENUM0(errinj_id, ERRINJ_LIST);
 extern struct errinj errinjs[];
diff --git a/src/lua/init.c b/src/lua/init.c
index 25e8884a6..c27b2c7da 100644
--- a/src/lua/init.c
+++ b/src/lua/init.c
@@ -61,6 +61,7 @@
 #include "lua/utf8.h"
 #include "lua/swim.h"
 #include "lua/decimal.h"
+#include "errinj.h"
 #include "digest.h"
 #include <small/ibuf.h>
 
@@ -583,6 +584,7 @@ run_script_f(va_list ap)
 	 */
 	struct diag *diag = va_arg(ap, struct diag *);
 	bool aux_loop_is_run = false;
+	bool option_e_runned = false;
 
 	/*
 	 * Load libraries and execute chunks passed by -l and -e
@@ -613,6 +615,7 @@ run_script_f(va_list ap)
 			if (luaT_call(L, 0, 0) != 0)
 				goto error;
 			lua_settop(L, 0);
+			option_e_runned = true;
 			break;
 		default:
 			unreachable(); /* checked by getopt() in main() */
@@ -627,25 +630,42 @@ run_script_f(va_list ap)
 	fiber_sleep(0.0);
 	aux_loop_is_run = true;
 
+	/*
+	 * Detect is stdin a tty if it's set in error injection,
+	 * otherwise use standard isatty.
+	 * Specially use direct referring to errinjs cause in
+	 * non debug mode it's not available, but needed for tests.
+	 */
+	int is_atty;
+	assert(ERRINJ_STDIN_ISATTY >= 0 && ERRINJ_STDIN_ISATTY < errinj_id_MAX);
+	assert(errinjs[ERRINJ_STDIN_ISATTY].type == ERRINJ_INT);
+	struct errinj *inj = &errinjs[ERRINJ_STDIN_ISATTY];
+	if (inj != NULL && inj->iparam >= 0) {
+		is_atty = inj->iparam;
+	} else {
+		is_atty = isatty(STDIN_FILENO);
+	}
+
 	if (path && strcmp(path, "-") != 0 && access(path, F_OK) == 0) {
 		/* Execute script. */
 		if (luaL_loadfile(L, path) != 0)
 			goto luajit_error;
 		if (lua_main(L, argc, argv) != 0)
 			goto error;
-	} else if (!isatty(STDIN_FILENO) || (path && strcmp(path, "-") == 0)) {
+	} else if (!is_atty || (path && strcmp(path, "-") == 0)) {
 		/* Execute stdin */
 		if (luaL_loadfile(L, NULL) != 0)
 			goto luajit_error;
 		if (lua_main(L, argc, argv) != 0)
 			goto error;
-	} else {
+	} else if (!option_e_runned) {
 		interactive = true;
 	}
 
 	/*
-	 * Start interactive mode when it was explicitly requested
-	 * by "-i" option or stdin is TTY or there are no script.
+	 * Start interactive mode in any of the cases:
+	 * - it was explicitly requested by "-i" option;
+	 * - stdin is TTY and there are no script (-e is considered as a script).
 	 */
 	if (interactive) {
 		say_crit("%s %s\ntype 'help' for interactive help",
diff --git a/test/app-tap/gh-5040-always-enters-interactive-mode.test.lua b/test/app-tap/gh-5040-always-enters-interactive-mode.test.lua
new file mode 100755
index 000000000..de7e990be
--- /dev/null
+++ b/test/app-tap/gh-5040-always-enters-interactive-mode.test.lua
@@ -0,0 +1,121 @@
+#!/usr/bin/env tarantool
+
+local process_timeout = require('process_timeout')
+local ffi = require('ffi')
+local tap = require('tap')
+local fio = require('fio')
+
+--
+-- Tests to check if the tarantool binary enters
+-- interactive mode or not.
+--
+
+local TARANTOOL_PATH = arg[-1]
+local output_file = fio.abspath('out.txt')
+local cmd_end = (' >%s & echo $!'):format(output_file)
+
+-- Like a default timeout for `cond_wait` in test-run
+local process_waiting_timeout = 60.0
+local file_read_timeout = 60.0
+local file_read_interval = 0.2
+local file_open_timeout = 60.0
+
+-- Each command consists of:
+--  * cmd_args - command line arguments for tarantool binary
+--  * stdin - stdin for tarantool
+--  * interactive - true if interactive mode expected
+--  * empty_output - true if command should have empty output
+local commands = {
+    {
+        cmd_args = '',
+        stdin = 'tty',
+        interactive = true
+    },
+    {
+        cmd_args = '',
+        stdin = '/dev/null',
+        interactive = false,
+        empty_output = true
+    },
+
+    {
+        cmd_args = ' -e "print(_VERSION)"',
+        stdin = 'tty',
+        interactive = false
+    },
+    {
+        cmd_args = ' -e "print(_VERSION)"',
+        stdin = '/dev/null',
+        interactive = false
+    },
+
+    {
+        cmd_args = ' -i -e "print(_VERSION)"',
+        stdin = 'tty',
+        interactive = true
+    },
+    {
+        cmd_args = ' -i -e "print(_VERSION)"',
+        stdin = '/dev/null',
+        interactive = true
+    }
+}
+
+local test = tap.test('gh-5040')
+test:plan(#commands)
+
+for _, cmd in pairs(commands) do
+    local full_cmd = ''
+    if cmd.stdin == 'tty' then
+        cmd.stdin = ''
+        full_cmd = 'ERRINJ_STDIN_ISATTY=1 '
+    else
+        cmd.stdin = '< ' .. cmd.stdin
+    end
+
+    local full_cmd = full_cmd .. ('%s %s %s %s'):format(
+            TARANTOOL_PATH,
+            cmd.cmd_args,
+            cmd.stdin,
+            cmd_end
+    )
+    test:test(full_cmd, function(test)
+        test:plan(cmd.interactive and 1 or 2)
+
+        local pid = tonumber(io.popen(full_cmd):read("*line"))
+        assert(pid, "pipe error for: " .. cmd.cmd_args)
+
+        local fh = process_timeout.open_with_timeout(output_file,
+                file_open_timeout)
+        assert(fh, 'error while opening ' .. output_file)
+
+        if cmd.interactive then
+            local data = process_timeout.read_with_timeout(fh,
+                    file_read_timeout,
+                    file_read_interval)
+            test:like(data, 'tarantool>', 'interactive mode detected')
+        else
+            local process_completed = process_timeout.wait_process_completion(
+                    pid,
+                    process_waiting_timeout)
+            test:ok(process_completed, 'process completed')
+
+            -- If empty output expected, then don't wait full file_read_timeout
+            -- for non-empty output_file, only a little time to be sure that
+            -- file is empty.
+            local read_timeout = cmd.empty_output and file_read_interval
+                    or file_read_timeout
+            local data = process_timeout.read_with_timeout(fh, read_timeout,
+                    file_read_interval)
+            if cmd.empty_output then
+                test:ok(#data == 0, 'output is empty')
+            else
+                test:unlike(data, 'tarantool>', 'iteractive mode wasn\'t detected')
+            end
+        end
+        if process_timeout.process_is_alive(pid) then ffi.C.kill(pid, 9) end
+        fh:close()
+    end)
+end
+
+os.exit(test:check() and 0 or 1)
diff --git a/test/box/errinj.result b/test/box/errinj.result
index b8c2476c3..a962dbe2d 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -74,6 +74,7 @@ evals
   - ERRINJ_SNAP_COMMIT_DELAY: false
   - ERRINJ_SNAP_WRITE_DELAY: false
   - ERRINJ_SQL_NAME_NORMALIZATION: false
+  - ERRINJ_STDIN_ISATTY: -1
   - ERRINJ_SWIM_FD_ONLY: false
   - ERRINJ_TESTING: false
   - ERRINJ_TUPLE_ALLOC: false
-- 
2.28.0