[Tarantool-patches] [PATCH v2] net.box: allow to store user-defined fields in future object

[Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Before commit 954194a1ca5c ("net.box: rewrite request implementation in
C"), net.box future was a plain Lua table so that the caller could
attach extra information to it. Now it isn't true anymore - a future is
a userdata object, and it doesn't have indexing methods.

For backward compatibility, let's add __index and __newindex fields and
store user-defined fields in an mhash, which is created lazily on the
first __newindex invocation. __index falls back on the metatable methods
if a field isn't found in the mhash.

Follow-up #6241
Closes #6306
---
Changes in v2:
 - When a key is deleted from the hash (by setting its value to nil),
   check that it's actually present in the hash.

v1: https://lists.tarantool.org/pipermail/tarantool-patches/2021-August/025346.html

 src/box/lua/net_box.c                         |  93 ++++++++++++++
 test/box/net.box_fiber-async_gh-3107.result   | 117 +++++++++++++++++-
 test/box/net.box_fiber-async_gh-3107.test.lua |  41 +++++-
 3 files changed, 243 insertions(+), 8 deletions(-)

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 06e574cdf746..6ab0f71cdbe9 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -134,6 +134,14 @@ struct netbox_request {
 	 * the response hasn't been received yet.
 	 */
 	struct error *error;
+	/**
+	 * User-defined fields: field name -> Lua object reference.
+	 * We allow the user to attach extra information to a future object,
+	 * e.g. a reference to a connection or the invoked method name/args.
+	 * All the information is stored in this table, which is created
+	 * lazily, on the first __newindex invocation.
+	 */
+	struct mh_strnptr_t *index;
 };
 
 static const char netbox_registry_typename[] = "net.box.registry";
@@ -166,6 +174,15 @@ netbox_request_destroy(struct netbox_request *request)
 	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, request->result_ref);
 	if (request->error != NULL)
 		error_unref(request->error);
+	if (request->index != NULL) {
+		struct mh_strnptr_t *h = request->index;
+		mh_int_t k;
+		mh_foreach(h, k) {
+			int ref = (intptr_t)mh_strnptr_node(h, k)->val;
+			luaL_unref(tarantool_L, LUA_REGISTRYINDEX, ref);
+		}
+		mh_strnptr_delete(h);
+	}
 }
 
 /**
@@ -1358,6 +1375,79 @@ luaT_netbox_request_gc(struct lua_State *L)
 	return 0;
 }
 
+static int
+luaT_netbox_request_index(struct lua_State *L)
+{
+	struct netbox_request *request = luaT_check_netbox_request(L, 1);
+	size_t field_name_len;
+	const char *field_name = lua_tolstring(L, 2, &field_name_len);
+	if (field_name == NULL)
+		return luaL_error(L, "invalid index");
+	if (request->index != NULL) {
+		mh_int_t k = mh_strnptr_find_inp(request->index, field_name,
+						 field_name_len);
+		if (k != mh_end(request->index)) {
+			int field_value_ref = (intptr_t)mh_strnptr_node(
+				request->index, k)->val;
+			lua_rawgeti(L, LUA_REGISTRYINDEX, field_value_ref);
+			return 1;
+		}
+	}
+	/* Fall back on metatable methods. */
+	return luaL_getmetafield(L, 1, field_name) != LUA_TNIL ? 1 : 0;
+}
+
+static int
+luaT_netbox_request_newindex(struct lua_State *L)
+{
+	struct netbox_request *request = luaT_check_netbox_request(L, 1);
+	struct mh_strnptr_t *h = request->index;
+	size_t field_name_len;
+	const char *field_name = lua_tolstring(L, 2, &field_name_len);
+	if (field_name == NULL)
+		return luaL_error(L, "invalid index");
+	int field_value_ref = luaL_ref(L, LUA_REGISTRYINDEX);
+	if (field_value_ref == LUA_REFNIL) {
+		/* The field is set to nil. Delete it from the index. */
+		if (h == NULL)
+			return 0;
+		mh_int_t k = mh_strnptr_find_inp(h, field_name,
+						 field_name_len);
+		if (k != mh_end(h)) {
+			int ref = (intptr_t)mh_strnptr_node(h, k)->val;
+			luaL_unref(L, LUA_REGISTRYINDEX, ref);
+			mh_strnptr_del(h, k, NULL);
+		}
+		return 0;
+	}
+	if (h == NULL) {
+		/* Lazily create the index on the first invocation. */
+		h = request->index = mh_strnptr_new();
+		if (h == NULL) {
+			luaL_unref(L, LUA_REGISTRYINDEX, field_value_ref);
+			return luaL_error(L, "out of memory");
+		}
+	}
+	/* Insert a reference to the new value into the index. */
+	struct mh_strnptr_node_t node = {
+		.str = field_name,
+		.len = field_name_len,
+		.hash = mh_strn_hash(field_name, field_name_len),
+		.val = (void *)(intptr_t)field_value_ref,
+	};
+	struct mh_strnptr_node_t old_node = { NULL, 0, 0, NULL };
+	struct mh_strnptr_node_t *p_old_node = &old_node;
+	if (mh_strnptr_put(h, &node, &p_old_node,
+			   NULL) == mh_end(h)) {
+		luaL_unref(L, LUA_REGISTRYINDEX, field_value_ref);
+		return luaL_error(L, "out of memory");
+	}
+	/* Drop the reference to the overwritten value. */
+	if (p_old_node != NULL)
+		luaL_unref(L, LUA_REGISTRYINDEX, (intptr_t)old_node.val);
+	return 0;
+}
+
 /**
  * Returns true if the response was received for the given request.
  */
@@ -1602,6 +1692,7 @@ netbox_make_request(struct lua_State *L, int idx,
 	fiber_cond_create(&request->cond);
 	request->result_ref = LUA_NOREF;
 	request->error = NULL;
+	request->index = NULL;
 	if (netbox_request_register(request, registry) != 0) {
 		netbox_request_destroy(request);
 		luaT_error(L);
@@ -1988,6 +2079,8 @@ luaopen_net_box(struct lua_State *L)
 
 	static const struct luaL_Reg netbox_request_meta[] = {
 		{ "__gc",           luaT_netbox_request_gc },
+		{ "__index",        luaT_netbox_request_index },
+		{ "__newindex",     luaT_netbox_request_newindex },
 		{ "is_ready",       luaT_netbox_request_is_ready },
 		{ "result",         luaT_netbox_request_result },
 		{ "wait_result",    luaT_netbox_request_wait_result },
diff --git a/test/box/net.box_fiber-async_gh-3107.result b/test/box/net.box_fiber-async_gh-3107.result
index aaaca351a579..1c48a115e6b5 100644
--- a/test/box/net.box_fiber-async_gh-3107.result
+++ b/test/box/net.box_fiber-async_gh-3107.result
@@ -10,10 +10,7 @@ net = require('net.box')
 cond = nil
 ---
 ...
-box.schema.func.create('long_function')
----
-...
-box.schema.user.grant('guest', 'execute', 'function', 'long_function')
+box.schema.user.grant('guest', 'execute', 'universe')
 ---
 ...
 function long_function(...) cond = fiber.cond() cond:wait() return ... end
@@ -104,7 +101,117 @@ err:find('Usage') ~= nil
 ---
 - true
 ...
-box.schema.func.drop('long_function')
+-- Storing user data in future object.
+future = c:eval('return 123', {}, {is_async = true})
+---
+...
+future.abc -- nil
+---
+- null
+...
+future.abc = nil
+---
+...
+future.abc -- nil
+---
+- null
+...
+future.abc = 'abc'
+---
+...
+future.abc -- abc
+---
+- abc
+...
+future.abc = nil
+---
+...
+future.abc -- nil
+---
+- null
+...
+future.abc = nil
+---
+...
+future.abc -- nil
+---
+- null
+...
+future[{}] -- error
+---
+- error: '[string "return future[{}] -- error "]:1: invalid index'
+...
+future[{}] = 'abc' -- error
+---
+- error: '[string "future[{}] = ''abc'' -- error "]:1: invalid index'
+...
+future:wait_result() -- 123
+---
+- [123]
+...
+-- Garbage collection of stored user data.
+future = c:eval('return 123', {}, {is_async = true})
+---
+...
+future.data1 = {1}
+---
+...
+future.data2 = {2}
+---
+...
+future.data3 = {3}
+---
+...
+gc = setmetatable({}, {__mode = 'v'})
+---
+...
+gc.data1 = future.data1
+---
+...
+gc.data2 = future.data2
+---
+...
+gc.data3 = future.data3
+---
+...
+future.data1 = nil
+---
+...
+_ = collectgarbage('collect')
+---
+...
+gc.data1 == nil
+---
+- true
+...
+future.data2 = 123
+---
+...
+_ = collectgarbage('collect')
+---
+...
+gc.data2 == nil
+---
+- true
+...
+future:wait_result() -- 123
+---
+- [123]
+...
+future = nil
+---
+...
+_ = collectgarbage('collect')
+---
+...
+_ = collectgarbage('collect')
+---
+...
+gc.data3 == nil
+---
+- true
+...
+box.schema.user.revoke('guest', 'execute', 'universe')
 ---
 ...
 c:close()
diff --git a/test/box/net.box_fiber-async_gh-3107.test.lua b/test/box/net.box_fiber-async_gh-3107.test.lua
index d23f368cbce4..e08bad056cba 100644
--- a/test/box/net.box_fiber-async_gh-3107.test.lua
+++ b/test/box/net.box_fiber-async_gh-3107.test.lua
@@ -5,8 +5,7 @@ net = require('net.box')
 -- gh-3107: fiber-async netbox.
 --
 cond = nil
-box.schema.func.create('long_function')
-box.schema.user.grant('guest', 'execute', 'function', 'long_function')
+box.schema.user.grant('guest', 'execute', 'universe')
 function long_function(...) cond = fiber.cond() cond:wait() return ... end
 function finalize_long() while not cond do fiber.sleep(0.01) end cond:signal() cond = nil end
 s = box.schema.create_space('test')
@@ -36,7 +35,43 @@ err:find('Usage') ~= nil
 _, err = pcall(future.wait_result, future, '100')
 err:find('Usage') ~= nil
 
-box.schema.func.drop('long_function')
+-- Storing user data in future object.
+future = c:eval('return 123', {}, {is_async = true})
+future.abc -- nil
+future.abc = nil
+future.abc -- nil
+future.abc = 'abc'
+future.abc -- abc
+future.abc = nil
+future.abc -- nil
+future.abc = nil
+future.abc -- nil
+future[{}] -- error
+future[{}] = 'abc' -- error
+future:wait_result() -- 123
+
+-- Garbage collection of stored user data.
+future = c:eval('return 123', {}, {is_async = true})
+future.data1 = {1}
+future.data2 = {2}
+future.data3 = {3}
+gc = setmetatable({}, {__mode = 'v'})
+gc.data1 = future.data1
+gc.data2 = future.data2
+gc.data3 = future.data3
+future.data1 = nil
+_ = collectgarbage('collect')
+gc.data1 == nil
+future.data2 = 123
+_ = collectgarbage('collect')
+gc.data2 == nil
+future:wait_result() -- 123
+future = nil
+_ = collectgarbage('collect')
+_ = collectgarbage('collect')
+gc.data3 == nil
+
+box.schema.user.revoke('guest', 'execute', 'universe')
 
 c:close()
 s:drop()
-- 
2.25.1