From a8ae1a1d415f66c8e1c26c5248f7ebdf09454a55 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Sat, 31 Jul 2021 21:57:31 +0300 Subject: [PATCH] lua: fix crash on running in background mode Tarantool with background mode enabled in environment variable could lead a crash: NO_WRAP ``` $ TT_PID_FILE=tarantool.pid TT_LOG=tarantool.log TT_BACKGROUND=true TT_LISTEN=3301 tarantool -e 'box.cfg{}' $ tail -3 tarantool.log 2021-11-02 16:05:43.672 [2341202] main init.c:696 E> LuajitError: cannot read stdin: Resource temporarily unavailable 2021-11-02 16:05:43.672 [2341202] main F> fatal error, exiting the event loop 2021-11-02 16:05:43.672 [2341202] main F> fatal error, exiting the event loop Patch fixes a crash. Other testcases were tested too: $ TT_PID_FILE=tarantool.pid TT_LOG=tarantool.log TT_BACKGROUND=true TT_LISTEN=3301 ./build/src/tarantool < box.cfg.lua $ echo "box.cfg{}" | TT_PID_FILE=tarantool.pid TT_LOG=tarantool.log TT_BACKGROUND=true TT_LISTEN=3301 ./build/src/tarantool - $ ./build/src/tarantool -l fiber -e 'fiber.sleep(1) box.cfg{background=true, log="tarantool.log", pid_file="tarantool.pid"}' (T="$(realpath ./build/src/tarantool)"; I=$(realpath third_party/luajit/test/luajit-test-init.lua); cd third_party/luajit/test/lua-Harness-tests; export LUA_PATH="$(realpath .)/?.lua;;"; "${T}" -e "dofile[[${I}]]" -l profile_tarantool 241-standalone.t) ``` NO_WRAP I dig a bit to a Git history and found that case with TT_BACKGROUND=true never worked: - commit 1b330121ec7d ("box: set box.cfg options via environment variables") - commit 9db64aedbfe2 ("lua: add '-' command line option") Closes #6128 NO_DOC=bugfix --- ...28-fix-crash-with-background-env-option.md | 4 ++ src/cfg.c | 40 ++++++++++++++++++- src/cfg.h | 12 ++++++ src/lua/init.c | 34 +++++++++++++++- .../gh-5602-environment-cfg-test-cases.lua | 7 ++++ .../gh-5602-environment-vars-cfg.result | 15 ++++++- .../gh-5602-environment-vars-cfg.test.lua | 26 ++++++++---- 7 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 changelogs/unreleased/gh-6128-fix-crash-with-background-env-option.md diff --git a/changelogs/unreleased/gh-6128-fix-crash-with-background-env-option.md b/changelogs/unreleased/gh-6128-fix-crash-with-background-env-option.md new file mode 100644 index 000000000000..a6af33429727 --- /dev/null +++ b/changelogs/unreleased/gh-6128-fix-crash-with-background-env-option.md @@ -0,0 +1,4 @@ +## bugfix/core + +* Fixed a crash that could happen when Tarantool is started with environment + variable `TT_BACKGROUND=true` (gh-6128). diff --git a/src/cfg.c b/src/cfg.c index 109753871571..7fe37fe74cc1 100644 --- a/src/cfg.c +++ b/src/cfg.c @@ -36,12 +36,42 @@ enum { MAX_OPT_NAME_LEN = 256, MAX_OPT_VAL_LEN = 256 }; +/* + * @brief Toggle panics in cfg_get(). + */ +static int cfg_panic; + +/* + * @brief Enables panics in cfg_get(). + */ +void +cfg_enable_panic(void) +{ + cfg_panic = 1; +} + +/* + * @brief Disables panics in cfg_get(). + */ +void +cfg_disable_panic(void) +{ + cfg_panic = 0; +} + static void cfg_get(const char *param) { + /* int panic = 1; */ + /* va_list args; */ + /* va_start(args, param); */ + /* panic = va_arg(args, int); */ + /* printf("cfg_get background %d\n", panic); */ + /* va_end(args); */ + const char *buf = tt_snprintf(MAX_OPT_NAME_LEN, "return box.cfg.%s", param); - if (luaL_dostring(tarantool_L, buf) != 0) + if ((luaL_dostring(tarantool_L, buf) != 0) && cfg_panic) panic("cfg_get('%s')", param); } @@ -76,6 +106,14 @@ cfg_isnumber(const char *param) return ret; } +/* + * @brief Returns a value of a boolean parameter whose name is passed. + * + * @param param - name of a boolean parameter + * @return -1 - undefined + * 1 - false or nil + * 0 - true + */ int cfg_getb(const char *param) { diff --git a/src/cfg.h b/src/cfg.h index 1213780e9144..79a48ab639cf 100644 --- a/src/cfg.h +++ b/src/cfg.h @@ -39,6 +39,18 @@ struct uri_set; extern "C" { #endif /* defined(__cplusplus) */ +/* + * @brief Enables panics in cfg_get(). + */ +void +cfg_enable_panic(void); + +/* + * @brief Disables panics in cfg_get(). + */ +void +cfg_disable_panic(void); + /** * Fill @a uri_set according to cfg @a param. * Return 0 if success, otherwise return -1 diff --git a/src/lua/init.c b/src/lua/init.c index 5b46503e8d4d..f6c9dbdfcf49 100644 --- a/src/lua/init.c +++ b/src/lua/init.c @@ -45,6 +45,7 @@ #include #include "version.h" +#include "cfg.h" #include "coio.h" #include "core/backtrace.h" #include "core/tt_static.h" @@ -979,14 +980,43 @@ run_script_f(va_list ap) int is_a_tty = isatty(STDIN_FILENO); + /* + * - interactive mode: + * - option '-i' is passed. + * Example: tarantool -i. + * - stdin is terminal AND + * there are no script ('-e' is considered as a script). + * Example: tarantool. + * - background mode: + * - explicitly enabled with box.cfg{ background=true }. + * Example: tarantool -e 'box.cfg{background=true}'. + * - explicitly enabled with TT_BACKGROUND via environment. + * - Example: TT_BACKGROUND=1 tarantool. + * - is_a_tty mode: + * - whether stdin refers to a terminal. + * Example: tarantool < script.lua + * Example: tarantool - < script.lua + * Example: cat script.lua | tarantool + */ + + cfg_disable_panic(); + int background = cfg_geti("background"); + cfg_enable_panic(); + + /* tarantool script.lua */ 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 ((!interactive && !is_a_tty) || - (path && strcmp(path, "-") == 0)) { + /* + * tarantool < script.lua + * tarantool - < script.lua + * cat script.lua | tarantool - + */ + } else if (((!interactive && !is_a_tty) || + (path && strcmp(path, "-") == 0)) && (!background)) { /* Execute stdin */ if (luaL_loadfile(L, NULL) != 0) goto luajit_error; diff --git a/test/box-tap/gh-5602-environment-cfg-test-cases.lua b/test/box-tap/gh-5602-environment-cfg-test-cases.lua index 4c3bc2716a41..f1ad764fbd12 100755 --- a/test/box-tap/gh-5602-environment-cfg-test-cases.lua +++ b/test/box-tap/gh-5602-environment-cfg-test-cases.lua @@ -70,5 +70,12 @@ if arg[1] == '5' then end test:is(err_msg, exp_err, 'bad strip_core value') end +if arg[1] == '6' then + test:plan(4) + test:ok(status, 'box.cfg is successful') + test:is(box.cfg['listen'], listen_sock, 'listen') + test:is(box.cfg['pid_file'], 'tarantool.pid', 'pid file') + test:is(box.cfg['log'], 'tarantool.log', 'log file') +end os.exit(test:check() and 0 or 1) diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result index 3050d57478a8..bc09f0a64bab 100644 --- a/test/box-tap/gh-5602-environment-vars-cfg.result +++ b/test/box-tap/gh-5602-environment-vars-cfg.result @@ -29,5 +29,16 @@ TAP version 13 ok - box.cfg is not successful ok - bad strip_core value TAP version 13 -1..1 -ok - exit status list +1..4 +ok - box.cfg is successful +ok - listen +ok - pid file +ok - log file +TAP version 13 +1..6 +ok - tarantool's exit code is ok (0) +ok - tarantool's exit code is ok (0) +ok - tarantool's exit code is ok (0) +ok - tarantool's exit code is ok (0) +ok - tarantool's exit code is ok (0) +ok - tarantool's exit code is ok (0) diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua index 477d5fc38aee..dbd05c586fa9 100755 --- a/test/box-tap/gh-5602-environment-vars-cfg.test.lua +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua @@ -29,30 +29,42 @@ local repl1_sock = fio.pathjoin(fio.cwd(), 'repl1.sock') local repl2_sock = fio.pathjoin(fio.cwd(), 'repl2.sock') local cases = { + -- testcase 1 ('%s %s %s %s %s'):format( string.format('TT_LISTEN=%s', listen_sock), 'TT_READAHEAD=10000', 'TT_STRIP_CORE=false', 'TT_LOG_FORMAT=json', 'TT_LOG_NONBLOCK=false'), + + -- testcase 2 ('%s %s %s %s'):format( string.format('TT_LISTEN=%s', listen_sock), string.format('TT_REPLICATION=%s,%s', repl1_sock,repl2_sock), 'TT_REPLICATION_CONNECT_TIMEOUT=0.01', 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\''), + + -- testcase 3 'TT_BACKGROUND=true TT_VINYL_TIMEOUT=60.1', + + -- testcase 4 'TT_SQL_CACHE_SIZE=a', + + -- testcase 5 'TT_STRIP_CORE=a', + + -- testcase 6 + ('%s %s %s %s'):format( + string.format('TT_LISTEN=%s', listen_sock), + 'TT_LOG=tarantool.log', + 'TT_PID_FILE=tarantool.pid', + 'TT_BACKGROUND=true'), } -test:plan(1) -local exit_status_list = {} -local exit_status_list_exp = {} +test:plan(#cases) for i, case in ipairs(cases) do - exit_status_list[i] = os.execute(shell_command(case, i)) - exit_status_list_exp[i] = 0 + local rc = os.execute(shell_command(case, i)) + test:is(rc, 0, "tarantool's exit code is ok (0)") end -test:is_deeply(exit_status_list, exit_status_list_exp, 'exit status list') - os.exit(test:check() and 0 or 1)