diff --git a/src/lua/utils.c b/src/lua/utils.c index 8e98f760768e7abfb147ccdb86e44b36335112e6..bb2287162b29e6c06546ff209ab97030de3ff5eb 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -29,6 +29,7 @@ * SUCH DAMAGE. */ #include "lua/utils.h" +#include <lj_trace.h> #include <assert.h> #include <errno.h> @@ -1309,10 +1310,49 @@ tarantool_lua_utils_init(struct lua_State *L) return 0; } +/* + * XXX: There is already defined <panic> macro in say.h header + * (included in diag.h). As a result the call below is misexpanded + * and compilation fails with the corresponding error. To avoid + * this error the macro is undefined since it's not used anymore + * in scope of this translation unit. + */ +#undef panic + /** * This routine encloses the checks and actions to be done when * the running fiber yields the execution. + * Since Tarantool fibers don't switch-over the way Lua coroutines + * do the platform ought to notify JIT engine when one lua_State + * substitutes another one. */ void cord_on_yield(void) { + struct global_State *g = G(tarantool_L); + /* + * XXX: Switching fibers while running the trace leads to + * code misbehaviour and failures, so stop its execution. + */ + if (unlikely(tvref(g->jit_base))) { + /* + * XXX: mcode is executed only in scope of Lua + * world and one can obtain the corresponding Lua + * coroutine from the fiber storage. + */ + struct lua_State *L = fiber()->storage.lua.stack; + assert(L != NULL); + lua_pushfstring(L, "fiber %d is switched while running the" + " compiled code (it's likely a function with" + " a yield underneath called via LuaJIT FFI)", + fiber()->fid); + if (g->panic) + g->panic(L); + exit(EXIT_FAILURE); + } + /* + * Unconditionally abort trace recording whether fibers + * switch each other. Otherwise, further compilation may + * lead to a failure on any next compiler phase. + */ + lj_trace_abort(g); } diff --git a/test/app-tap/CMakeLists.txt b/test/app-tap/CMakeLists.txt index ee67cf533af5d984f2dddf1a5d84789af0e1125f..bf7d28136b41e44846e52adb1c67415e66780151 100644 --- a/test/app-tap/CMakeLists.txt +++ b/test/app-tap/CMakeLists.txt @@ -1 +1,2 @@ build_module(module_api module_api.c) +build_module(libyield libyield.c) diff --git a/test/app-tap/gh-1700-abort-recording-on-fiber-switch.skipcond b/test/app-tap/gh-1700-abort-recording-on-fiber-switch.skipcond new file mode 100644 index 0000000000000000000000000000000000000000..2a2ec4d9736d934a01c2c74f9ae00d44ea4e0335 --- /dev/null +++ b/test/app-tap/gh-1700-abort-recording-on-fiber-switch.skipcond @@ -0,0 +1,7 @@ +import platform + +# Disabled on FreeBSD due to #4819. +if platform.system() == 'FreeBSD': + self.skip = 1 + +# vim: set ft=python: diff --git a/test/app-tap/gh-1700-abort-recording-on-fiber-switch.test.lua b/test/app-tap/gh-1700-abort-recording-on-fiber-switch.test.lua new file mode 100755 index 0000000000000000000000000000000000000000..7b9156c22a0d90a40e1adda150d669279201d370 --- /dev/null +++ b/test/app-tap/gh-1700-abort-recording-on-fiber-switch.test.lua @@ -0,0 +1,78 @@ +#!/usr/bin/env tarantool + +if #arg == 0 then + local checks = { + { + arg = { + 1, -- hotloop (arg[1]) + 1, -- trigger (arg[2]) + }, + res = 'OK', + msg = 'Trace is aborted', + }, + { + arg = { + 1, -- hotloop (arg[1]) + 2, -- trigger (arg[2]) + }, + res = 'fiber %d+ is switched while running the compiled code %b()', + msg = 'Trace is recorded', + }, + } + + local tap = require('tap') + local test = tap.test('gh-1700-abort-recording-on-fiber-switch') + + test:plan(#checks) + + local vars = { + LUABIN = arg[-1], + SCRIPT = arg[0], + -- To support out-of-source build use relative paths in repo + PATH = arg[-1]:gsub('src/tarantool$', 'test/app-tap'), + SUFFIX = package.cpath:match('?.(%a+);'), + } + + local cmd = string.gsub('LUA_CPATH="$LUA_CPATH;<PATH>/?.<SUFFIX>" ' .. + 'LUA_PATH="$LUA_PATH;<PATH>/?.lua" ' .. + 'LD_LIBRARY_PATH=<PATH> ' .. + '<LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars) + + for _, ch in pairs(checks) do + local res + local proc = io.popen((cmd .. (' %s'):rep(#ch.arg)):format(unpack(ch.arg))) + for s in proc:lines() do res = s end + assert(res, 'proc:lines failed') + test:like(res, ch.res, ch.msg) + end + + os.exit(test:check() and 0 or 1) +end + + +-- Test body. + +local cfg = { + hotloop = arg[1] or 1, + trigger = arg[2] or 1, +} + +local ffi = require('ffi') +local ffiyield = ffi.load('libyield') +ffi.cdef('void yield(struct yield *state, int i)') + +-- Set the value to trigger <yield> call switch the running fuber. +local yield = require('libyield')(cfg.trigger) + +-- Depending on trigger and hotloop values the following contexts +-- are possible: +-- * if trigger <= hotloop -> trace recording is aborted +-- * if trigger > hotloop -> trace is recorded but execution +-- leads to panic +jit.opt.start("3", string.format("hotloop=%d", cfg.hotloop)) + +for i = 0, cfg.trigger + cfg.hotloop do + ffiyield.yield(yield, i) +end +-- Panic didn't occur earlier. +print('OK') diff --git a/test/app-tap/gh-4491-coio-wait-leads-to-segfault.test.lua b/test/app-tap/gh-4491-coio-wait-leads-to-segfault.test.lua new file mode 100755 index 0000000000000000000000000000000000000000..0dd8dfbeee120d038f05540fc16ccc5696ac0f1b --- /dev/null +++ b/test/app-tap/gh-4491-coio-wait-leads-to-segfault.test.lua @@ -0,0 +1,53 @@ +#!/usr/bin/env tarantool + +local tap = require('tap') + +local test = tap.test('gh-4491-coio-wait-leads-to-segfault') + +-- Test file to demonstrate platform failure due to fiber switch +-- while trace recording, details: +-- https://github.com/tarantool/tarantool/issues/4491 + +local fiber = require('fiber') +local ffi = require('ffi') +ffi.cdef('int coio_wait(int fd, int event, double timeout);') + +local cfg = { + hotloop = arg[1] or 1, + fibers = arg[1] or 2, + timeout = { put = 1, get = 1 }, +} + +test:plan(cfg.fibers + 1) + +local args = { + fd = 1 , -- STDIN file descriptor + event = 0x1 , -- COIO_READ event + timeout = 0.05, -- Timeout value +} + +local function run(iterations, channel) + for _ = 1, iterations do + ffi.C.coio_wait(args.fd, args.event, args.timeout) + end + channel:put(true, cfg.timeout.put) +end + +local channels = { } + +jit.opt.start('3', string.format('hotloop=%d', cfg.hotloop)) + +for _ = 1, cfg.fibers do + channels[_] = fiber.channel(1) + fiber.new(run, cfg.hotloop + 1, channels[_]) +end + +-- Finalize the existing fibers +for _ = 1, cfg.fibers do + test:ok(channels[_]:get(cfg.timeout.get), + string.format('fiber #%d successfully finished', _)) +end + +test:ok(true, 'trace is not recorded due to fiber switch underneath coio_wait') + +os.exit(test:check() and 0 or 1) diff --git a/test/app-tap/libyield.c b/test/app-tap/libyield.c new file mode 100644 index 0000000000000000000000000000000000000000..1b7221a8e2428898f5ba78ab0fd23c2e966adb18 --- /dev/null +++ b/test/app-tap/libyield.c @@ -0,0 +1,29 @@ +#include <lua.h> +#include <module.h> + +struct yield { + int trigger; /* Trigger for yielding the fiber execution */ +}; + +void yield(struct yield *state, int i) +{ + if (i < state->trigger) + return; + + /* Fiber yields the execution for a jiffy */ + fiber_sleep(0); +} + +static int init(lua_State *L) +{ + struct yield *state = lua_newuserdata(L, sizeof(struct yield)); + + state->trigger = lua_tonumber(L, 1); + return 1; +} + +LUA_API int luaopen_libyield(lua_State *L) +{ + lua_pushcfunction(L, init); + return 1; +}