From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 86D076EC40; Mon, 16 Aug 2021 11:27:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 86D076EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629102429; bh=pvT/GNudrgYrLc75hOD9+xV7o9e/lsVmtHGxyk+SSoM=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=BAJ+p8QeGdbawGuMkqxInhPJTcpcieK57vTE+s5B0Phpv9P0cneyEBGGSJEKF3C+S t3x4RDvIG6bTCu3ODEjGbGnZFkFQ+BHWzCuDljbuXv+aVWqKs3xLB5e4wJ/ksQlaI7 4Qmuz21cRKSJ736x/hkMI3ka5PQvu+IXJLeDEKt8= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 74D3B6EC40 for ; Mon, 16 Aug 2021 11:27:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 74D3B6EC40 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mFXxb-0001uK-4f; Mon, 16 Aug 2021 11:27:07 +0300 Date: Mon, 16 Aug 2021 11:27:05 +0300 To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org, yaroslav.dynnikov@tarantool.org Message-ID: <20210816082705.scypk477pp335ahb@esperanza> References: <20210813095700.7vumduzrevgmcuyi@esperanza> <20210814111659.GQ27855@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210814111659.GQ27855@tarantool.org> X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD910164DC12A5633065676A9727AC27C74182A05F53808504001346B0B494032E698CAF02435A7FF1381287B08DDD6A081706935AE7E20DDA4 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A4C4638C9DDF45FCEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379347C0682FC030B08638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8281531C0259C58B09F193EA758A92BAA117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD186FD1C55BDD38FC3FD2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A45692FFBBD75A6A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A56A2E7EE0377846A40EEFAB3F32C6334B6F1F721E60609AF1D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA752DA3D96DA0CEF5C48E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34E420FF71F2F0FE03F93A7E064E0EC7972DC0CE0803A1FD5585F677256CD6E17D260AB2A5668087C81D7E09C32AA3244C1CEDAFB28CD750093D1B76A0D49D31651DD47778AE04E04D83B48618A63566E0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojIrFL/N5KnVE998v5GhsmPg== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D1C950E24947538F30F26F2FDFF2EB7A9274CEFED1673C562683ABF942079399BFB559BB5D741EB966A65DFF43FF7BE03240331F90058701C67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v3] net.box: allow to store user-defined fields in future object X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladimir Davydov via Tarantool-patches Reply-To: Vladimir Davydov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Sat, Aug 14, 2021 at 02:17:00PM +0300, Igor Munkin wrote: > On 13.08.21, Vladimir Davydov wrote: > > Before commit 954194a1ca5c ("net.box: rewrite request implementation in > > Typo: AFAICS full hash has to be used to mention particular commits. I > failed to find this rule in the dev guide[1], but I've found that Cyrill > has recently added the example to Code review procedure in our Wiki[2] > (however, the command doesn't yield the full hash). Fixed. > > +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. */ > > Minor: It's worth to mention, why you don't use here > instead (like you do in __newindex). The reason is to save the given key > for the fall back path. You can also add an assert, that coroutine stack > size at the end of this block is the same as at its beginning. Added a comment. > > + 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) { > > Minor: Looks like this condition is met only once. Will this place > perform better with UNLIKELY attribute? Checked. Using UNLIKELY here in __index and/or __newindex makes no difference. Leaving as is. > > +future.abc = nil > > +--- > > +... > > +future.abc -- nil > > +--- > > +- null > > +... > > +future.abc = nil > > Looks like a typo: future.abc already equals to nil above. It's actually not a typo. I wanted to ensure that deleting a key that has no value works fine. Added clarifying comments. -- >From db18219e23d9d42c56125ec45af28876017e38f0 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Tue, 10 Aug 2021 14:38:33 +0300 Subject: [PATCH] net.box: allow to store user-defined fields in future object Before commit 954194a1ca5c2cd1826a8a689663c827313dbbef ("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 diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c index 229dec590cf9..5d3d7826270d 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,48 @@ 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); + /* + * Copy the key (2nd argument) to the top. Note, we don't move + * it with lua_insert, like we do in __newindex, because we want + * to save it for the fallback path below. + */ + 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 +1716,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 +2105,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..4ff25ed3817c 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 -- delete a key that has never been set +--- +... +future.abc -- nil +--- +- null +... +future.abc = 'abc' -- set a value for a key +--- +... +future.abc -- abc +--- +- abc +... +future.abc = nil -- delete a value for a key +--- +... +future.abc -- nil +--- +- null +... +future.abc = nil -- delete a key with no value +--- +... +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..7b07ed2def14 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 -- delete a key that has never been set +future.abc -- nil +future.abc = 'abc' -- set a value for a key +future.abc -- abc +future.abc = nil -- delete a value for a key +future.abc -- nil +future.abc = nil -- delete a key with no value +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()