diff --git a/changelogs/unreleased/gh-4450-logger-sigsegv.md b/changelogs/unreleased/gh-4450-logger-sigsegv.md new file mode 100644 index 0000000000000000000000000000000000000000..3b898a605a76f344e2a832a4c9c91a15d8b1496f --- /dev/null +++ b/changelogs/unreleased/gh-4450-logger-sigsegv.md @@ -0,0 +1,4 @@ +## bugfix/core + +* Fix potential nil dereference and crash in case of an active + log rotation during the program exit stage (gh-4450). diff --git a/src/lib/core/say.c b/src/lib/core/say.c index 69c826264b1141b250da1fc5bf3962983afe28dc..e38237e358ebac06e40edefef660cb4d7a22c1d3 100644 --- a/src/lib/core/say.c +++ b/src/lib/core/say.c @@ -267,7 +267,7 @@ log_rotate(struct log *log) if (pm_atomic_load(&log->type) != SAY_LOGGER_FILE) return 0; - ERROR_INJECT(ERRINJ_LOG_ROTATE, { usleep(10); }); + ERROR_INJECT(ERRINJ_LOG_ROTATE, { usleep(1000); }); int fd = open(log->path, O_WRONLY | O_APPEND | O_CREAT, S_IRUSR | S_IWUSR | S_IRGRP); @@ -750,11 +750,24 @@ say_logger_init(const char *init_str, int level, int nonblock, panic("failed to initialize logging subsystem"); } +/* + * Logger uses events loop and to free resources we need + * to wait until logs rotation is complete. Thus make sure + * this routine is called before events loop is finished. + */ void say_logger_free(void) { - if (say_logger_initialized()) + if (say_logger_initialized()) { log_destroy(&log_std); + /* + * Once destroyed switch to boot logger + * because boot logger is safe to use + * anytime and might be suitable for + * debugging. + */ + log_default = &log_boot; + } } /** {{{ Formatters */ diff --git a/src/main.cc b/src/main.cc index 6b5f5ede85086ea375b8d60af553db677576d2c2..3c52d1a80e661b4d1c8623878616a8d83a0f0209 100644 --- a/src/main.cc +++ b/src/main.cc @@ -141,6 +141,7 @@ on_shutdown_f(va_list ap) (void) ap; trigger_fiber_run(&box_on_shutdown_trigger_list, NULL, on_shutdown_trigger_timeout); + say_logger_free(); ev_break(loop(), EVBREAK_ALL); return 0; } @@ -583,7 +584,6 @@ tarantool_free(void) memtx_tx_manager_free(); coll_free(); systemd_free(); - say_logger_free(); fiber_free(); memory_free(); random_free(); diff --git a/test/app-tap/gh-4450-log-rotate-exit.test.lua b/test/app-tap/gh-4450-log-rotate-exit.test.lua new file mode 100755 index 0000000000000000000000000000000000000000..a1ca9abee24f1030860c5a6891609c10b4413eb6 --- /dev/null +++ b/test/app-tap/gh-4450-log-rotate-exit.test.lua @@ -0,0 +1,46 @@ +#!/usr/bin/env tarantool + +local tap = require('tap') +local test = tap.test("gh-4450-log-rotate-exit") +local fio = require('fio') +local tarantool_bin = arg[-1] + +test:plan(1) + +local function run_script(code) + local dir = fio.tempdir() + local script_path = fio.pathjoin(dir, 'gh-4450-script.lua') + local script = fio.open(script_path, + {'O_CREAT', 'O_WRONLY', 'O_APPEND'}, + tonumber('0777', 8)) + script:write(code) + script:close() + local output_file = fio.pathjoin(fio.cwd(), 'gh-4450-out.txt') + local cmd = [[/bin/sh -c 'cd "%s" && "%s" ./gh-4450-script.lua 0> %s 2> %s']] + local code = os.execute( + string.format(cmd, dir, tarantool_bin, output_file, output_file) + ) + fio.rmtree(dir) + local out_fd = fio.open(output_file, {'O_RDONLY'}) + local output = out_fd:read(100000) + out_fd:close() + return code, output +end + +-- +-- gh-4382: an error in main script should be handled gracefully, +-- with proper logging. +-- +local code = "log = require('log')\n" +code = code .. "box.cfg{log = 'gh-4550.log'}\n" +code = code .. "box.error.injection.set('ERRINJ_LOG_ROTATE', true)\n" +code = code .. "log.rotate()\n" +code = code .. "os.exit()\n" +local _, output = run_script(code) + +test:diag(output) +test:ok(not string.find(output, "SEGV_MAPERR"), + "program exited without SEGV_MAPERR") + +test:check() +os.exit(0) diff --git a/test/app-tap/suite.ini b/test/app-tap/suite.ini index 06f549ca36cecedbeed598264296fa2e8a64fe88..3263185067889ff94bfca6da6ba551d4cec76ead 100644 --- a/test/app-tap/suite.ini +++ b/test/app-tap/suite.ini @@ -4,7 +4,7 @@ description = application server tests (TAP) lua_libs = lua/require_mod.lua lua/serializer_test.lua lua/process_timeout.lua is_parallel = True use_unix_sockets_iproto = True -release_disabled = gh-5040-inter-mode-isatty-via-errinj.test.lua gh-2717-no-quit-sigint.test.lua +release_disabled = gh-5040-inter-mode-isatty-via-errinj.test.lua gh-2717-no-quit-sigint.test.lua gh-4450-log-rotate-exit.test.lua fragile = { "retries": 10, "tests": {