Re: [Tarantool-patches] [PATCH 2.X 5/7] module api: luaT_temp_luastate & luaT_release_temp_luastate

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

I strongly don't like this export. It seems to be too internal.

But I have no better ideas than propose to implement your own
lua_State cache in the merger module. It does not seem to be too
hard, and I don't think it would occupy much memory.

Just a simple list of lua_State objects, created by luaT_newthread
on demand, and put back into the list when unused. What is wrong
with that?

Adding Igor as Lua master to help with this.

See 2 comments below.

> diff --git a/src/lua/utils.h b/src/lua/utils.h
> index 9b1fe7e57..da0140076 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -554,6 +554,41 @@ luaL_iscallable(lua_State *L, int idx);
>  struct lua_State *
>  luaT_newthread(struct lua_State *L);
>  
> +/**
> + * Get a temporary Lua state.
> + *
> + * Use case: a function does not accept a Lua state as an argument
> + * to allow using from C code, but uses a Lua value, which is
> + * referenced in LUA_REGISTRYINDEX. A temporary Lua stack is needed
> + * to get and process the value.
> + *
> + * The resulting Lua state has a separate Lua stack, but the same
> + * globals and registry as `tarantool_L` (and all Lua states in

1. Users don't know what is tarantool_L.

> + * tarantool at the moment of writing this).
> + *
> + * This Lua state should be used only from one fiber: otherwise
> + * one fiber may change the stack and another one will access a
> + * wrong stack slot when it will be scheduled for execution after
> + * yield.
> + *
> + * Return a Lua state on success and set @a coro_ref and @a top.
> + * These values should be passed to
> + * `luaT_release_temp_luastate()`, when the state is not needed
> + * anymore.

2. I would encapsulate the out parameters into some kind of
luaT_temp_state_ctx or something like that. Probably not even
opaque, but extendible.

Also maybe worth grouping these methods by using a common prefix:

	luaT_temp_luastate_get
	luaT_temp_luastate_release

> + *
> + * Return NULL and set a diag at failure.
> + */
> +struct lua_State *
> +luaT_temp_luastate(int *coro_ref, int *top);
> +
> +/**
> + * Release a temporary Lua state.
> + *
> + * It complements `luaT_temp_luastate()`.
> + */
> +void
> +luaT_release_temp_luastate(struct lua_State *L, int coro_ref, int top);
> +
>  /** \endcond public */
>  
>  /**
>