[Tarantool-patches] [PATCH] refactoring: wrap lua_newthread with luaT_cpcall

[Ilya Kosarev]

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