Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org
Cc: yaroslav.dynnikov@tarantool.org, v.shpilevoy@tarantool.org
Subject: [Tarantool-patches] [PATCH v3] net.box: allow to store user-defined fields in future object
Date: Fri, 13 Aug 2021 12:59:29 +0300	[thread overview]
Message-ID: <b996502b403c1698c4b0886de636faa0a63cfb70.1628848572.git.vdavydov@tarantool.org> (raw)
In-Reply-To: <20210813095700.7vumduzrevgmcuyi@esperanza>

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 a Lua table, which is created lazily on the
first __newindex invocation. __index falls back on the metatable methods
if a field isn't found in the table.

Follow-up #6241
Closes #6306
---
https://github.com/tarantool/tarantool/issues/6306
https://github.com/tarantool/tarantool/tree/vdavydov/netbox-allow-to-store-user-data-in-future-using-lua-table

Changes in v3:
 - Use Lua table instead of mhash for storing user data, as suggested
   by imun@.

v2: https://lists.tarantool.org/pipermail/tarantool-patches/2021-August/025402.html

 src/box/lua/net_box.c                         |  51 ++++++++
 test/box/net.box_fiber-async_gh-3107.result   | 109 +++++++++++++++++-
 test/box/net.box_fiber-async_gh-3107.test.lua |  39 ++++++-
 3 files changed, 191 insertions(+), 8 deletions(-)

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 229dec590cf9..078bbfb07b18 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -124,6 +124,15 @@ struct netbox_request {
 	/** Lua references to on_push trigger and its context. */
 	int on_push_ref;
 	int on_push_ctx_ref;
+	/**
+	 * Lua reference to a table with user-defined fields.
+	 * 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. Until then, it's
+	 * LUA_NOREF.
+	 */
+	int index_ref;
 	/**
 	 * Lua reference to the request result or LUA_NOREF if the response
 	 * hasn't been received yet. If the response was decoded to a
@@ -167,6 +176,7 @@ netbox_request_destroy(struct netbox_request *request)
 	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, request->on_push_ref);
 	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, request->on_push_ctx_ref);
 	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, request->result_ref);
+	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, request->index_ref);
 	if (request->error != NULL)
 		error_unref(request->error);
 }
@@ -1420,6 +1430,44 @@ 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);
+	if (request->index_ref != LUA_NOREF) {
+		lua_rawgeti(L, LUA_REGISTRYINDEX, request->index_ref);
+		/* Push the key (2nd argument) to the top. */
+		lua_pushvalue(L, 2);
+		lua_rawget(L, -2);
+		if (lua_type(L, -1) != LUA_TNIL)
+			return 1;
+		/* Pop nil and the index table. */
+		lua_pop(L, 2);
+	}
+	/* Fall back on metatable methods. */
+	lua_getmetatable(L, 1);
+	/* Move the metatable before the key (2nd argument). */
+	lua_insert(L, 2);
+	lua_rawget(L, 2);
+	return 1;
+}
+
+static int
+luaT_netbox_request_newindex(struct lua_State *L)
+{
+	struct netbox_request *request = luaT_check_netbox_request(L, 1);
+	if (request->index_ref == LUA_NOREF) {
+		/* Lazily create the index table on the first invocation. */
+		lua_newtable(L);
+		request->index_ref = luaL_ref(L, LUA_REGISTRYINDEX);
+	}
+	lua_rawgeti(L, LUA_REGISTRYINDEX, request->index_ref);
+	/* Move the index table before the key (2nd argument). */
+	lua_insert(L, 2);
+	lua_rawset(L, 2);
+	return 0;
+}
+
 /**
  * Returns true if the response was received for the given request.
  */
@@ -1664,6 +1712,7 @@ netbox_make_request(struct lua_State *L, int idx,
 		request->format = tuple_format_runtime;
 	tuple_format_ref(request->format);
 	fiber_cond_create(&request->cond);
+	request->index_ref = LUA_NOREF;
 	request->result_ref = LUA_NOREF;
 	request->error = NULL;
 	if (netbox_request_register(request, registry) != 0) {
@@ -2052,6 +2101,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..60033059335b 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,109 @@ 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: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..474dfaa4e35b 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,41 @@ 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: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


  reply	other threads:[~2021-08-13  9:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-10 16:53 [Tarantool-patches] [PATCH] " Vladimir Davydov via Tarantool-patches
2021-08-10 17:15 ` Cyrill Gorcunov via Tarantool-patches
2021-08-11  7:49   ` Vladimir Davydov via Tarantool-patches
2021-08-12 17:07     ` [Tarantool-patches] [PATCH v2] " Vladimir Davydov via Tarantool-patches
2021-08-12 18:02 ` [Tarantool-patches] [PATCH] " Vladislav Shpilevoy via Tarantool-patches
2021-08-12 18:13   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-13 10:01   ` Vladimir Davydov via Tarantool-patches
2021-08-12 18:13 ` Igor Munkin via Tarantool-patches
2021-08-13  9:57   ` Vladimir Davydov via Tarantool-patches
2021-08-13  9:59     ` Vladimir Davydov via Tarantool-patches [this message]
2021-08-14 11:17       ` [Tarantool-patches] [PATCH v3] " Igor Munkin via Tarantool-patches
2021-08-16  8:27         ` Vladimir Davydov via Tarantool-patches
2021-08-16  8:56           ` Igor Munkin via Tarantool-patches
2021-08-16 11:32             ` Vitaliia Ioffe via Tarantool-patches
2021-08-16 11:35               ` Vladimir Davydov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b996502b403c1698c4b0886de636faa0a63cfb70.1628848572.git.vdavydov@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=vdavydov@tarantool.org \
    --cc=yaroslav.dynnikov@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v3] net.box: allow to store user-defined fields in future object' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox