From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 997CB43D67A for ; Tue, 15 Oct 2019 18:50:52 +0300 (MSK) From: Ilya Kosarev Date: Tue, 15 Oct 2019 18:50:45 +0300 Message-Id: <20191015155045.7002-1-i.kosarev@tarantool.org> Subject: [Tarantool-patches] [PATCH] refactoring: wrap lua_newthread with luaT_cpcall List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: tarantool-patches@freelists.org Wrap throwing lua_newthread with luaT_cpcall to process arising error properly. Closes #4556 --- https://github.com/tarantool/tarantool/tree/gh-4556-wrap-lua-newthread https://github.com/tarantool/tarantool/issues/4556 src/box/lua/call.c | 18 +++++++++++++++--- src/lua/fiber.c | 6 +++++- src/lua/trigger.c | 13 +++++-------- src/lua/utils.h | 24 ++++++++++++++++++++++++ third_party/lua-yaml/lyaml.cc | 6 +++++- 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/box/lua/call.c b/src/box/lua/call.c index 631003c84..ecc85f7c2 100644 --- a/src/box/lua/call.c +++ b/src/box/lua/call.c @@ -527,7 +527,11 @@ static inline int box_process_lua(lua_CFunction handler, struct execute_lua_ctx *ctx, struct port *ret) { - lua_State *L = lua_newthread(tarantool_L); + struct lua_State *L = NULL; + if (luaT_cpcall(tarantool_L, lua_newthread_wrapper, &L) != 0 || L == NULL) { + return -1; + } + luaL_pushthread(tarantool_L, L); int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); port_lua_create(ret, L); ((struct port_lua *) ret)->ref = coro_ref; @@ -654,7 +658,11 @@ func_persistent_lua_load(struct func_lua *func) * an arbitrary user-defined code * (e.g. body = 'fiber.yield()'). */ - struct lua_State *coro_L = lua_newthread(tarantool_L); + struct lua_State *coro_L = NULL; + if (luaT_cpcall(tarantool_L, lua_newthread_wrapper, &coro_L) != 0 || coro_L == NULL) { + return -1; + } + luaL_pushthread(tarantool_L, coro_L); if (!func->base.def->is_sandboxed) { /* * Keep the original env to apply to a non-sandboxed @@ -811,7 +819,11 @@ lbox_func_call(struct lua_State *L) * before the function call to pass it into the * pcall-sandboxed tarantool_L handler. */ - lua_State *args_L = lua_newthread(tarantool_L); + struct lua_State *args_L = NULL; + if (luaT_cpcall(tarantool_L, lua_newthread_wrapper, &args_L) != 0 || args_L == NULL) { + return luaT_error(L); + } + luaL_pushthread(tarantool_L, args_L); int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); lua_xmove(L, args_L, lua_gettop(L) - 1); struct port args; diff --git a/src/lua/fiber.c b/src/lua/fiber.c index 336be60a2..f71ea2a72 100644 --- a/src/lua/fiber.c +++ b/src/lua/fiber.c @@ -388,7 +388,11 @@ lua_fiber_run_f(MAYBE_UNUSED va_list ap) static struct fiber * fiber_create(struct lua_State *L) { - struct lua_State *child_L = lua_newthread(L); + struct lua_State *child_L = NULL; + if (luaT_cpcall(L, lua_newthread_wrapper, &child_L) != 0 || child_L == NULL) { + luaT_error(L); + } + luaL_pushthread(L, child_L); int coro_ref = luaL_ref(L, LUA_REGISTRYINDEX); struct fiber *f = fiber_new("lua", lua_fiber_run_f); diff --git a/src/lua/trigger.c b/src/lua/trigger.c index 4803e85c5..bf3fce234 100644 --- a/src/lua/trigger.c +++ b/src/lua/trigger.c @@ -73,17 +73,14 @@ lbox_trigger_run(struct trigger *ptr, void *event) * trigger yields, so when it's time to clean * up the coro, we wouldn't know which stack position * it is on. - * - * XXX: lua_newthread() may throw if out of memory, - * this needs to be wrapped with lua_pcall() as well. - * Don't, since it's a stupid overhead on every trigger - * invocation, and in future we plan to hack into Lua - * C API to fix this. */ - struct lua_State *L; + struct lua_State *L = NULL; int coro_ref; if (fiber()->storage.lua.stack == NULL) { - L = lua_newthread(tarantool_L); + if (luaT_cpcall(tarantool_L, lua_newthread_wrapper, &L) != 0 || L == NULL) { + diag_raise(); + } + luaL_pushthread(tarantool_L, L); coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); } else { L = fiber()->storage.lua.stack; diff --git a/src/lua/utils.h b/src/lua/utils.h index f8c34545a..cf4d6d326 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -417,6 +417,30 @@ luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg, int idx, luaL_convertfield(L, cfg, idx, field); } +/** + * @brief A wrapper for lua_newthread() to pass it into luaT_cpcall + * @param L is a Lua State + * @sa lua_newthread() + */ +static inline int +lua_newthread_wrapper(lua_State *L) +{ + *(struct lua_State**)lua_touserdata(L, 1) = lua_newthread(L); + return 0; +} + +/** + * @brief Push L1 thread onto the L stack + * @param L is a Lua State whose stack we are pushing onto + * @param L1 is a Lua State whose thread we are pushing + */ +static inline void +luaL_pushthread(lua_State *L, lua_State *L1) +{ + setthreadV(L, L->top, L1); + incr_top(L); +} + void luaL_register_type(struct lua_State *L, const char *type_name, const struct luaL_Reg *methods); diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc index 7485341fa..78b68e260 100644 --- a/third_party/lua-yaml/lyaml.cc +++ b/third_party/lua-yaml/lyaml.cc @@ -789,7 +789,11 @@ lua_yaml_encode(lua_State *L, struct luaL_serializer *serializer, dumper.cfg = serializer; dumper.error = 0; /* create thread to use for YAML buffer */ - dumper.outputL = lua_newthread(L); + dumper.outputL = NULL; + if (luaT_cpcall(L, lua_newthread_wrapper, &dumper.outputL) != 0 || dumper.outputL == NULL) { + return luaL_error(L, OOM_ERRMSG); + } + luaL_pushthread(L, dumper.outputL); luaL_buffinit(dumper.outputL, &dumper.yamlbuf); if (!yaml_emitter_initialize(&dumper.emitter)) -- 2.17.1