[Tarantool-patches] [PATCH 03/15] tuple: pass global ibuf explicitly where possible

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Thu Mar 25 00:24:29 MSK 2021


Code in lua/tuple.c used global tarantool_lua_ibuf in many places
relying on it never being changed and not reused by other code
until a yield.

But it is not so. In fact, as it was discovered in #5632, in any
Lua function may be started GC. Any GC handler might touch some
API also using tarantool_lua_ibuf inside.

This makes the first usage in lua/tuple.c invalid - the buffer
could be reset or reallocated or its wpos/rpos could change during
GC.

In order to fix this, first of all there should be clear points
where the buffer is taken, and where it becomes not needed
anymore.

The patch makes code in lua/tuple.c take tarantool_lua_ibuf when
it is needed first time. Not during usage. The same is done for
the fiber region for the API symmetry.

Part of #5632

(cherry picked from commit fabf0d57ea31d69be848fa33c98397a9a4a964e9)
---
 src/box/lua/tuple.c | 60 ++++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 617acc9aa..00ac3b44c 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -134,10 +134,8 @@ luaT_istuple(struct lua_State *L, int narg)
  * Helper for <lbox_tuple_new>().
  */
 static int
-luaT_tuple_encode_values(struct lua_State *L)
+luaT_tuple_encode_values(struct lua_State *L, struct ibuf *buf)
 {
-	struct ibuf *buf = tarantool_lua_ibuf;
-	ibuf_reset(buf);
 	struct mpstream stream;
 	mpstream_init(&stream, buf, ibuf_reserve_cb, ibuf_alloc_cb, luamp_error,
 		      L);
@@ -150,22 +148,31 @@ luaT_tuple_encode_values(struct lua_State *L)
 	return 0;
 }
 
-typedef void luaT_mpstream_init_f(struct mpstream *stream, struct lua_State *L);
+typedef void luaT_mpstream_init_f(struct mpstream *stream, struct lua_State *L,
+				  void *buffer);
 
 static void
-luaT_mpstream_init_lua_ibuf(struct mpstream *stream, struct lua_State *L)
+luaT_mpstream_init_lua_ibuf(struct mpstream *stream, struct lua_State *L,
+			    void *buffer)
 {
-	mpstream_init(stream, tarantool_lua_ibuf, ibuf_reserve_cb,
+	mpstream_init(stream, (struct ibuf *)buffer, ibuf_reserve_cb,
 		      ibuf_alloc_cb, luamp_error, L);
 }
 
 static void
-luaT_mpstream_init_box_region(struct mpstream *stream, struct lua_State *L)
+luaT_mpstream_init_box_region(struct mpstream *stream, struct lua_State *L,
+			      void *buffer)
 {
-	mpstream_init(stream, &fiber()->gc, region_reserve_cb, region_alloc_cb,
-		      luamp_error, L);
+	mpstream_init(stream, (struct region *)buffer, region_reserve_cb,
+		      region_alloc_cb, luamp_error, L);
 }
 
+/** Helper to pass parameters into luaT_tuple_encode_table via the Lua stack. */
+struct luaT_tuple_encode_ctx {
+	luaT_mpstream_init_f *mpstream_init_f;
+	void *buffer;
+};
+
 /**
  * Encode a Lua table or a tuple as MsgPack.
  *
@@ -177,8 +184,8 @@ static int
 luaT_tuple_encode_table(struct lua_State *L)
 {
 	struct mpstream stream;
-	luaT_mpstream_init_f *luaT_mpstream_init_f = lua_topointer(L, 1);
-	luaT_mpstream_init_f(&stream, L);
+	const struct luaT_tuple_encode_ctx *ctx = lua_topointer(L, 1);
+	ctx->mpstream_init_f(&stream, L, ctx->buffer);
 	luamp_encode_tuple(L, &tuple_serializer, &stream, 2);
 	mpstream_flush(&stream);
 	return 0;
@@ -189,7 +196,8 @@ luaT_tuple_encode_table(struct lua_State *L)
  */
 static int
 luaT_tuple_encode_on_mpstream(struct lua_State *L, int idx,
-			      luaT_mpstream_init_f *luaT_mpstream_init_f)
+			      luaT_mpstream_init_f *luaT_mpstream_init_f,
+			      void *buffer)
 {
 	assert(idx != 0);
 	if (!lua_istable(L, idx) && !luaT_istuple(L, idx)) {
@@ -212,7 +220,11 @@ luaT_tuple_encode_on_mpstream(struct lua_State *L, int idx,
 	lua_rawgeti(L, LUA_REGISTRYINDEX, luaT_tuple_encode_table_ref);
 	assert(lua_isfunction(L, -1));
 
-	lua_pushlightuserdata(L, luaT_mpstream_init_f);
+	struct luaT_tuple_encode_ctx ctx = {
+		.mpstream_init_f = luaT_mpstream_init_f,
+		.buffer = buffer,
+	};
+	lua_pushlightuserdata(L, &ctx);
 	lua_pushvalue(L, idx);
 
 	int rc = luaT_call(L, 2, 0);
@@ -225,12 +237,11 @@ luaT_tuple_encode_on_mpstream(struct lua_State *L, int idx,
  */
 static char *
 luaT_tuple_encode_on_lua_ibuf(struct lua_State *L, int idx,
-			      size_t *tuple_len_ptr)
+			      size_t *tuple_len_ptr, struct ibuf *buf)
 {
-	struct ibuf *buf = tarantool_lua_ibuf;
-	ibuf_reset(buf);
 	if (luaT_tuple_encode_on_mpstream(L, idx,
-					  luaT_mpstream_init_lua_ibuf) != 0)
+					  luaT_mpstream_init_lua_ibuf,
+					  buf) != 0)
 		return NULL;
 	if (tuple_len_ptr != NULL)
 		*tuple_len_ptr = ibuf_used(buf);
@@ -246,7 +257,8 @@ luaT_tuple_encode(struct lua_State *L, int idx, size_t *tuple_len_ptr)
 	struct region *region = &fiber()->gc;
 	size_t region_svp = region_used(region);
 	if (luaT_tuple_encode_on_mpstream(L, idx,
-					  luaT_mpstream_init_box_region) != 0) {
+					  luaT_mpstream_init_box_region,
+					  region) != 0) {
 		region_truncate(region, region_svp);
 		return NULL;
 	}
@@ -267,15 +279,16 @@ luaT_tuple_encode(struct lua_State *L, int idx, size_t *tuple_len_ptr)
 box_tuple_t *
 luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format)
 {
+	struct ibuf *ibuf = tarantool_lua_ibuf;
+	ibuf_reset(ibuf);
 	size_t tuple_len;
-	char *tuple_data = luaT_tuple_encode_on_lua_ibuf(L, idx, &tuple_len);
+	char *tuple_data = luaT_tuple_encode_on_lua_ibuf(L, idx, &tuple_len,
+							 ibuf);
 	if (tuple_data == NULL)
 		return NULL;
 	box_tuple_t *tuple = box_tuple_new(format, tuple_data,
 					   tuple_data + tuple_len);
-	if (tuple == NULL)
-		return NULL;
-	ibuf_reinit(tarantool_lua_ibuf);
+	ibuf_reinit(ibuf);
 	return tuple;
 }
 
@@ -295,7 +308,8 @@ lbox_tuple_new(lua_State *L)
 	box_tuple_format_t *fmt = box_tuple_format_default();
 	if (argc != 1 || (!lua_istable(L, 1) && !luaT_istuple(L, 1))) {
 		struct ibuf *buf = tarantool_lua_ibuf;
-		luaT_tuple_encode_values(L); /* may raise */
+		ibuf_reset(buf);
+		luaT_tuple_encode_values(L, buf); /* may raise */
 		struct tuple *tuple = box_tuple_new(fmt, buf->buf,
 						    buf->buf + ibuf_used(buf));
 		ibuf_reinit(buf);
-- 
2.24.3 (Apple Git-128)



More information about the Tarantool-patches mailing list