[Tarantool-patches] [PATCH] lua: refactor port_lua_do_dump and encode_lua_call
Sergey Kaplun
skaplun at tarantool.org
Fri Aug 13 12:27:58 MSK 2021
Also, add the branch for 1.10:
https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-refactor-lua-call-1.10
The difference is that the port structure itself is pushed instead ctx
structure on the stack.
The patch is the following:
===================================================================
commit 59eafb8e6d8f41de06bdb28b930c9ff1d3fbe63e
Author: Sergey Kaplun <skaplun at tarantool.org>
Date: Fri Jun 18 18:28:30 2021 +0300
lua: refactor port_lua_do_dump and encode_lua_call
(cherry picked from commit 434d31bdb52d80384af55acd2c3a637e5154e257)
The old code flow was the following:
1) `struct port_lua` given to `port_lua_do_dump()` has Lua stack with
arguments to encode to MessagePack.
2) The main coroutine `tarantool_L` is used to call `encode_lua_call()`
or `encode_lua_call_16`() via `lua_cpcall()`.
3) Objects on port coroutine are encoded via `luamp_encode()` or
`luamp_encode_call16()`.
4) This encoding may raise an error on unprotected `port->L` coroutine.
This coroutine has no protected frame on it and this call should fail
in pure Lua.
Calling anything on unprotected coroutine is not allowed in Lua [1]:
| If an error happens outside any protected environment, Lua calls a
| panic function
Lua 5.1 sets protection only for specific lua_State [2] and calls a
panic function if we raise an error on unprotected lua_State [3].
Nevertheless, no panic occurs now due to two facts:
* The first one is LuaJIT's support of C++ exception handling [4] that
allows to raise an error in Lua and catch it in C++ or vice versa. But
documentation still doesn't allow raising errors on unprotected
coroutines (at least we must use try-catch block).
* The second one is the patch made in LuaJIT to restore currently
executed coroutine, when C function or fast function raises an
error [5][6] (see the related issue here [7][8]).
For these reasons, when an error occurs, the unwinder searches and finds
the C-protected stack frame from the `lua_cpcall()` for `tarantool_L`
coroutine and unwinds until that point (without aforementioned patches
LuaJIT just calls a panic function and exits).
If an error is raised, and `lua_cpcall()` returns not `LUA_OK`, then the
error from `port->L` coroutine is converted into a Tarantool error and a
diagnostic is set.
Such auxiliary usage of `tarantool_L` is not idiomatic for Lua.
Internal unwinder used on M1 is not such flexible, so such misuse leads
to panic call. Also the `tarantool_L` usage is redundant. So this patch
drops it and uses only port coroutine instead with `lua_pcall()`.
Functions to encode are saved to the `LUA_REGISTRY` table to
reduce GC pressure, like it is done for other handlers [9].
[1]: https://www.lua.org/manual/5.2/manual.html#4.6
[2]: https://www.lua.org/source/5.1/lstate.h.html#lua_State
[3]: https://www.lua.org/source/5.1/ldo.c.html#luaD_throw
[4]: https://luajit.org/extensions.html#exceptions
[5]: https://github.com/tarantool/luajit/commit/ed412cd9f55fe87fd32a69c86e1732690fc5c1b0
[6]: https://github.com/tarantool/luajit/commit/97699d9ee2467389b6aea21a098e38aff3469b5f
[7]: https://github.com/tarantool/tarantool/issues/1516
[8]: https://www.freelists.org/post/luajit/Issue-with-PCALL-in-21
[9]: https://github.com/tarantool/tarantool/commit/e88c0d21ab765d4c53bed2437c49d77b3ffe4216
Closes #6248
Closes #4617
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 593317265..f173d97b7 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -53,6 +53,8 @@
*/
enum handlers {
HANDLER_CALL,
+ HANDLER_ENCODE_CALL,
+ HANDLER_ENCODE_CALL_16,
HANDLER_EVAL,
HANDLER_MAX,
};
@@ -347,10 +349,26 @@ execute_lua_eval(lua_State *L)
return lua_gettop(L);
}
+
+/**
+ * Encode call results to msgpack from Lua stack.
+ * Lua stack has the following structure -- the last element is
+ * lightuserdata pointer to encode_lua_ctx, all other values are
+ * arguments to process.
+ * The function encodes all given Lua objects to msgpack stream
+ * from context, sets port's size and returns no value on the Lua
+ * stack.
+ * XXX: This function *MUST* be called under lua_pcall(), because
+ * luamp_encode() may raise an error.
+ */
static int
encode_lua_call(lua_State *L)
{
+ assert(lua_islightuserdata(L, -1));
struct port_lua *port = (struct port_lua *) lua_topointer(L, -1);
+ assert(port->L == L);
+ /* Delete port from the stack. */
+ lua_pop(L, 1);
/*
* Add all elements from Lua stack to the buffer.
*
@@ -361,18 +379,33 @@ encode_lua_call(lua_State *L)
luamp_error, port->L);
struct luaL_serializer *cfg = luaL_msgpack_default;
- int size = lua_gettop(port->L);
+ const int size = lua_gettop(L);
for (int i = 1; i <= size; ++i)
- luamp_encode(port->L, cfg, &stream, i);
+ luamp_encode(L, cfg, &stream, i);
port->size = size;
mpstream_flush(&stream);
return 0;
}
+/**
+ * Encode call_16 results to msgpack from Lua stack.
+ * Lua stack has the following structure -- the last element is
+ * lightuserdata pointer to encode_lua_ctx, all other values are
+ * arguments to process.
+ * The function encodes all given Lua objects to msgpack stream
+ * from context, sets port's size and returns no value on the Lua
+ * stack.
+ * XXX: This function *MUST* be called under lua_pcall(), because
+ * luamp_encode() may raise an error.
+ */
static int
encode_lua_call_16(lua_State *L)
{
+ assert(lua_islightuserdata(L, -1));
struct port_lua *port = (struct port_lua *) lua_topointer(L, -1);
+ assert(port->L == L);
+ /* Delete port from the stack. */
+ lua_pop(L, 1);
/*
* Add all elements from Lua stack to the buffer.
*
@@ -383,42 +416,45 @@ encode_lua_call_16(lua_State *L)
luamp_error, port->L);
struct luaL_serializer *cfg = luaL_msgpack_default;
- port->size = luamp_encode_call_16(port->L, cfg, &stream);
+ port->size = luamp_encode_call_16(L, cfg, &stream);
mpstream_flush(&stream);
return 0;
}
static inline int
-port_lua_do_dump(struct port *base, struct obuf *out, lua_CFunction handler)
+port_lua_do_dump(struct port *base, struct obuf *out, enum handlers handler)
{
struct port_lua *port = (struct port_lua *)base;
assert(port->vtab == &port_lua_vtab);
/* Use port to pass arguments to encoder quickly. */
port->out = out;
+ lua_State *L = port->L;
/*
- * Use the same global state, assuming the encoder doesn't
- * yield.
+ * At the moment Lua stack holds only values to encode.
+ * Insert corresponding encoder to the bottom and push
+ * encode context as lightuserdata to the top.
*/
- struct lua_State *L = tarantool_L;
- int top = lua_gettop(L);
- if (lua_cpcall(L, handler, port) != 0) {
- luaT_toerror(port->L);
+ const int size = lua_gettop(L);
+ lua_rawgeti(L, LUA_REGISTRYINDEX, execute_lua_refs[handler]);
+ assert(lua_isfunction(L, -1) && lua_iscfunction(L, -1));
+ lua_insert(L, 1);
+ lua_pushlightuserdata(L, port);
+ /* nargs -- all arguments + lightuserdata. */
+ if (luaT_call(L, size + 1, 0) != 0)
return -1;
- }
- lua_settop(L, top);
return port->size;
}
static int
port_lua_dump(struct port *base, struct obuf *out)
{
- return port_lua_do_dump(base, out, encode_lua_call);
+ return port_lua_do_dump(base, out, HANDLER_ENCODE_CALL);
}
static int
port_lua_dump_16(struct port *base, struct obuf *out)
{
- return port_lua_do_dump(base, out, encode_lua_call_16);
+ return port_lua_do_dump(base, out, HANDLER_ENCODE_CALL_16);
}
static void
@@ -499,6 +535,8 @@ box_lua_call_init(struct lua_State *L)
lua_CFunction handles[] = {
[HANDLER_CALL] = execute_lua_call,
+ [HANDLER_ENCODE_CALL] = encode_lua_call,
+ [HANDLER_ENCODE_CALL_16] = encode_lua_call_16,
[HANDLER_EVAL] = execute_lua_eval,
};
===================================================================
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list