Interrupt infinite lua loop

[filonenko-mikhail]

related #1898 #2857

Motivation

There is case:

• we want to provide lua sandbox for user to write only business script
• user write while true do end
• tarantool goes to infinite loop and unavailable to make a service

Showstopper forever

• If you stop the fiber before ffi.gc(), the memory leaks.

Solution

Consider tarantool is in infinite loop.

The solution is based on three features:

• POSIX signal, OS still can send signal to lua thread (and incapsulated fiber)
• debug.sethook, we have access to lua fiber context from signal handler and set hook
• LUAJIT_ENABLE_CHECKHOOK=1, luajit flag enables debug.sethook for compiled code
  • luajit probably have to be patched to use runtime setting for such thing for perf reasons

Proof Of Concept

Apply patch

diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
index 10df633d5..84844c733 100644
--- a/cmake/luajit.cmake
+++ b/cmake/luajit.cmake
@@ -1,4 +1,3 @@
-
 #
 # LuaJIT configuration file.
 #
@@ -218,6 +217,7 @@ macro(luajit_build)
         set (luajit_ldflags ${luajit_ldflags} -fsanitize=address)
     endif()
     add_definitions(-DLUAJIT_SMART_STRINGS=1)
+    add_definitions(-DLUAJIT_ENABLE_CHECKHOOK=1)
     # Add all COMPILE_DEFINITIONS to xcflags
     get_property(defs DIRECTORY PROPERTY COMPILE_DEFINITIONS)
     foreach(def ${defs})
diff --git a/src/main.cc b/src/main.cc
index 6255f2505..93060aeea 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -104,6 +104,59 @@ tarantool_uptime(void)
        return ev_monotonic_now(loop()) - start_time;
 }
 
+
+static pthread_t main_thread_id;
+
+#include "lua.h"
+#include <dlfcn.h>
+typedef int (*lua_sethook_t) (lua_State *L, lua_Hook f, int mask, int count);
+typedef int (*lua_error_t) (lua_State *L);
+typedef void (*lua_pushstring_t) (lua_State *L, const char *s);
+
+char interrupted_fiber_name[512];
+
+static void interrupt_fiber(lua_State *L, lua_Debug *ar) {
+    (void)ar;
+    lua_sethook_t lua_sethook = (lua_sethook_t) dlsym(RTLD_DEFAULT, "lua_sethook");
+    lua_error_t lua_error = (lua_error_t) dlsym(RTLD_DEFAULT, "lua_error");
+    lua_pushstring_t lua_pushstring = (lua_pushstring_t) dlsym(RTLD_DEFAULT, "lua_pushstring");
+
+
+    lua_sethook(L, interrupt_fiber, 0, 0);
+
+    snprintf(interrupted_fiber_name, sizeof(interrupted_fiber_name),
+           "Interrupted %s", fiber_name(fiber()));
+
+    lua_pushstring(L, interrupted_fiber_name);
+    lua_error(L);
+}
+
+extern lua_State* tarantool_L;
+static void sig_fiber_interrupt(int signum, struct __siginfo *, void* ctx) {
+
+    if (main_thread_id != pthread_self()) {
+       say_error("SIGUSR2: Wrong thread");
+       return;
+    }
+    (void)signum;
+    (void)ctx;
+
+    lua_sethook_t lua_sethook = (lua_sethook_t) dlsym(RTLD_DEFAULT, "lua_sethook");
+    struct fiber* f = fiber();
+
+    if (!f) {
+       say_error("SIGUSR2: fiber not found");
+       return;
+    }
+    if (!f->storage.lua.stack) {
+       say_error("SIGUSR2: fiber have no lua thread context %s", fiber_name(f));
+       return;
+    }
+
+    lua_sethook(f->storage.lua.stack, interrupt_fiber,
+               LUA_MASKCALL | LUA_MASKRET | LUA_MASKLINE | LUA_MASKCOUNT, 1);
+}
+
 /**
  * Create a checkpoint from signal handler (SIGUSR1)
  */
@@ -353,7 +406,20 @@ tarantool_atfork()
 static void
 signal_init(void)
 {
-       struct sigaction sa;
+    struct sigaction sa_yield;
+
+    /* Reset all signals to their defaults. */
+    memset(&sa_yield, 0, sizeof(sa_yield));
+    sigemptyset(&sa_yield.sa_mask);
+    sa_yield.sa_sigaction = sig_fiber_interrupt;
+    sa_yield.sa_flags = SA_NODEFER | SA_SIGINFO | SA_ONSTACK;
+    sigemptyset(&sa_yield.sa_mask);
+
+    if (sigaction(SIGUSR2, &sa_yield, NULL) == -1) {
+       say_syserror("sigaction");
+    }
+
+        struct sigaction sa;
        memset(&sa, 0, sizeof(sa));
 
        sa.sa_handler = SIG_IGN;
@@ -801,6 +867,7 @@ main(int argc, char **argv)
 
        exception_init();
 
+       main_thread_id = pthread_self();
        fiber_init(fiber_cxx_invoke);
        coio_init();
        coio_enable();

Open two consoles.

Run tarantool with infinite fiber

Tarantool Enterprise For Mac 2.2.1-132-g6af1d7f34
type 'help' for interactive help
tarantool> fiber = require('fiber')
---
...

tarantool> do f = fiber.new(function () while true do end end) f:name('INFERNO') end
---
...

Kill with SIGUSR2 from other console

pkill -SIGUSR2 tarantool   

Expected:

LuajitError: Interrupted INFERNO

Tarantool version:
any
OS version:
any

[kostja]

who said lua_sethook is async-signal-safe?

[sergos]

Mike himself:

You can dynamically set the hook from a signal or a different
thread (lua_sethook is the only thread-safe call).

https://www.freelists.org/post/luajit/Efficient-query-timeouts-in-LuaJIT,1

[kostja]

The correct solution is to always run the hook, and check a RCU variable in it, which is set by a real time signal handler. This can simply yield the current fiber, so will even work more nicely.

[igormunkin]

@filonenko-mikhail, I glanced over the ticket and need some time to analyze all proposals and changes. Meanwhile the similar mechanism is and already implemented in LuaVela. Even if we wouldn't be able to replace our fork with LuaVela, I wonder whether we can incorporate that machinery into our LuaJIT fork.

@kostja, since I'm not that deep into Tarantool internals for now, could you please take a look on the spec and provide pros and cons you see regarding this approach for Tarantool fibers?

[filonenko-mikhail]

@igormunkin

As a special case and as of |PROJECT| 0.16, timeouts are not checked during normal coroutine execution if the JIT compiler is active.

I think we have to support compiled code interruption.

[filonenko-mikhail]

I mean that LUAJIT_ENABLE_CHECKHOOK=1 enable sethook for jitted code

[igormunkin]

@filonenko-mikhail, could you please provide some use cases you see as a rationale for trace executing interuption? I see the comment left by Mike but I can state nothing right now regarding it.

Let's investigate the problem from both sides simultaneously and return to the question in a while?

[filonenko-mikhail]

There are:
Case 1, when developer interactively uses tarantool and mistake turns into full scan query on large space. This way developer has to restart whole process.
Case 2, application server provides lua sandbox for customer scripts, which can contain recursion or full scan queries.

I think we can support two semantics at runtime:

• timeout
• interrupt
  And for perf reason we can make such semantic enabled by fiber option.

[sergos]

I measured the impact of the hooks enabled in traces. The one liner

time tarantool -e "require('jit.dump').start('Tbitmsr') x = 0 for i = 1,1e10 do x = x + 1 end return x"

gives the following output (the tail of it, see the loop contents) without hooks compiled in:

->LOOP:
1361affe0  addsd xmm7, xmm1
1361affe4  movsd [rax], xmm7
1361affe8  addsd xmm6, xmm1
1361affec  ucomisd xmm0, xmm6
1361afff0  jnb 0x1361affe0	->LOOP
1361afff2  jmp 0x1361a001c	->3
---- TRACE 1 stop -> loop


real	0m9.903s
user	0m9.830s
sys	0m0.031s

and with the hooks enabled (two extra instructions in the loop):

->LOOP:
130b5ffd0  test byte [r14-0xeaf], 0xc
130b5ffd8  jnz 0x130b50018	->2
130b5ffde  addsd xmm7, xmm1
130b5ffe2  movsd [rax], xmm7
130b5ffe6  addsd xmm6, xmm1
130b5ffea  ucomisd xmm0, xmm6
130b5ffee  jnb 0x130b5ffd0	->LOOP
130b5fff0  jmp 0x130b5001c	->3
---- TRACE 1 stop -> loop


real	0m9.918s
user	0m9.843s
sys	0m0.031s

Which means about 0.15% slowdown on such a loop - just an increment. If we put any sort of workload from box it'll be scaled down considerably.

[filonenko-mikhail]

The many thanks.

[filonenko-mikhail]

Additional case is fiber introspection
Consider:

• cluster setup
• one node of cluster wants to change something on several neighbours
• one neighbour takes too long time for task
• we want to know what kind of things go wrong

[Gerold103]

Regardless of how you do it, you need to consider how are you going to break the fibers in the places which never are allowed to yield/stop.

• What if the fiber was in the middle of a transaction? How are you going to free it, and what will happen to its changes?
• Some code relies on certain commands to never yield nor throw OOM. For instance, parts of bucket_recv() and bucket_send() in vshard. Stopping on an arbitrary Lua line can make the other fibers see inconsistent changes, which were not supposed to be seen. This is the same issue as with __gc. Except that in __gc you can avoid touching anything global in complex ways, and this auto-stop won't be avoidable. Another example is when a fiber does ffi.new() but didn't do ffi.gc() yet. If you stop the fiber before ffi.gc(), the memory leaks.

[filonenko-mikhail]

If you stop the fiber before ffi.gc(), the memory leaks.

The problem(

[filonenko-mikhail]

The golang runtime makes safe and unsafe points for such cases.
The things have to be done with user responsibility

fiber.unsafepoint(TRUE)
ffi.gc(ffi.new())
fiber.unsafepoint(FALSE)