From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 47C0F41D0BD for ; Thu, 17 Oct 2019 13:49:17 +0300 (MSK) From: Ilya Kosarev Date: Thu, 17 Oct 2019 13:49:12 +0300 Message-Id: <20191017104912.31390-1-i.kosarev@tarantool.org> Subject: [Tarantool-patches] [PATCH v2] refactoring: wrap lua_newthread using 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 in luaT_newthread using luaT_cpcall to process arising error properly. Closes #4556 --- https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4556-wrap-lua-newthread https://github.com/tarantool/tarantool/issues/4556 Changes in v2: - introduced luaT_newthread instead of repeated slocs - renamed luaL_pushthread to luaT_pushthread due to namespaces agreement src/box/lua/call.c | 12 +++++++--- src/lua/fiber.c | 4 +++- src/lua/trigger.c | 12 ++++------ src/lua/utils.h | 41 +++++++++++++++++++++++++++++++++++ third_party/lua-yaml/lyaml.cc | 5 ++++- 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/src/box/lua/call.c b/src/box/lua/call.c index 631003c84..51cad286a 100644 --- a/src/box/lua/call.c +++ b/src/box/lua/call.c @@ -527,7 +527,9 @@ static inline int box_process_lua(lua_CFunction handler, struct execute_lua_ctx *ctx, struct port *ret) { - lua_State *L = lua_newthread(tarantool_L); + lua_State *L = luaT_newthread(tarantool_L); + if (L == NULL) + return -1; int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); port_lua_create(ret, L); ((struct port_lua *) ret)->ref = coro_ref; @@ -654,7 +656,9 @@ 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); + lua_State *coro_L = luaT_newthread(tarantool_L); + if (coro_L == NULL) + return -1; if (!func->base.def->is_sandboxed) { /* * Keep the original env to apply to a non-sandboxed @@ -811,7 +815,9 @@ 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); + lua_State *args_L = luaT_newthread(tarantool_L); + if (args_L == NULL) + return luaT_error(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..124908a05 100644 --- a/src/lua/fiber.c +++ b/src/lua/fiber.c @@ -388,7 +388,9 @@ 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); + lua_State *child_L = luaT_newthread(L); + if (child_L == NULL) + luaT_error(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..6df048a8d 100644 --- a/src/lua/trigger.c +++ b/src/lua/trigger.c @@ -73,17 +73,13 @@ 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; + lua_State *L; int coro_ref; if (fiber()->storage.lua.stack == NULL) { - L = lua_newthread(tarantool_L); + L = luaT_newthread(tarantool_L); + if (L == NULL) + diag_raise(); 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..f82b30fbc 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -591,6 +591,47 @@ luaL_checkfinite(struct lua_State *L, struct luaL_serializer *cfg, luaL_error(L, "number must not be NaN or Inf"); } +/** + * @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) +{ + *(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 +luaT_pushthread(lua_State *L, lua_State *L1) +{ + setthreadV(L, L->top, L1); + incr_top(L); +} + +/** + * @brief Safe wrapper for lua_newthread() + * @param L is a Lua State + * @sa lua_newthread() + */ +static inline lua_State * +luaT_newthread(lua_State *L) +{ + lua_State *L1 = NULL; + if (luaT_cpcall(L, lua_newthread_wrapper, &L1) != 0) { + return NULL; + } + assert(L1 != NULL); + luaT_pushthread(L, L1); + return L1; +} + /** * Check if a value on @a L stack by index @a idx is an ibuf * object. Both 'struct ibuf' and 'struct ibuf *' are accepted. diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc index 7485341fa..af4f2f5d5 100644 --- a/third_party/lua-yaml/lyaml.cc +++ b/third_party/lua-yaml/lyaml.cc @@ -789,7 +789,10 @@ 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 = luaT_newthread(L); + if (dumper.outputL == NULL) { + return luaL_error(L, OOM_ERRMSG); + } luaL_buffinit(dumper.outputL, &dumper.yamlbuf); if (!yaml_emitter_initialize(&dumper.emitter)) -- 2.17.1