[tarantool-patches] Re: [PATCH v3 3/4] session: introduce binary box.session.push
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Jun 7 20:02:35 MSK 2018
On 07/06/2018 15:53, Konstantin Osipov wrote:
> Hi,
>
> My code review for this patch is on branch
> box-session-push-kostja.
>
> What still needs to be done:
>
> - convert push.test to box-tap
I have removed assertions and left the test be output checking.
Push test does many table members checking, and it is simpler to
just output the tables.
Assertions were in a single place were a table contains 400 members.
But ok, now I did it like in vinyl/select_consistency test.
> - think about alternatives which would preserve sync in a Lua
> routine. Looks like using an upvalue would do
> https://www.lua.org/pil/27.3.3.html
I had investigated it before you have recommended and before I
started the patch. But ok, I have investigated it again, and
wrote the simple patch, that shows upvalues to be unusable as
sync storage. See the diff below and the explanation under the diff.
+++ b/src/box/lua/call.c
@@ -434,8 +434,9 @@ box_process_lua(struct call_request *request, struct port *base,
- lua_pushcfunction(L, handler);
+ uint64_t sync = request->header->sync;
+ lua_pushinteger(L, sync);
+ lua_pushcclosure(L, handler, 1);
+++ b/src/box/lua/session.c
@@ -370,6 +370,9 @@ lbox_session_push(struct lua_State *L)
+ uint64_t sync = lua_tointeger(L, lua_upvalueindex(1));
+ uint64_t f_sync = fiber_sync(fiber());
+ say_info("%llu, %llu", (unsigned long long) sync, f_sync);
if (session_push(current_session(), &port) != 0) {
After running push.test.lua I found say_info() printing always
0 + real sync. So the upvalue is actually nil here.
C upvalues are not the same as Lua upvalues. C upvalue can be
associated with a single function only, while the Lua ones can
be referenced in multiple and enclosed functions. C upvalue
can be accessed only from the function owning it.
In the diff above I have pushed sync as an upvalue and tried to
get it in lbox_session_push function. But the upvalue was not
available. Lets look at the implementation of
lua_tointeger(L, lua_upvalueindex(1)).
lua_upvalueindex(idx):
#define LUA_GLOBALSINDEX (-10002)
#define lua_upvalueindex(i) (LUA_GLOBALSINDEX-(i))
Nothing is interesting. Upvalue index is just an index.
lua_tointeger(L, idx):
LUA_API lua_Integer lua_tointeger(lua_State *L, int idx)
{
cTValue *o = index2adr(L, idx);
...
index2addr(L, idx):
....
} else {
GCfunc *fn = curr_func(L);
api_check(L, fn->c.gct == ~LJ_TFUNC && !isluafunc(fn));
if (idx == LUA_ENVIRONINDEX) {
TValue *o = &G(L)->tmptv;
settabV(L, o, tabref(fn->c.env));
return o;
} else {
idx = LUA_GLOBALSINDEX - idx;
return idx <= fn->c.nupvalues ? &fn->c.upvalue[idx-1] : niltv(L);
}
}
As you see, upvalue array is got from curr_func(L).
lbox_session_push != C closure I had pushed earlier. And it has no
upvalues.
Summary: C upvalues can not be used as a sync storage.
More information about the Tarantool-patches
mailing list