[Tarantool-patches] [PATCH 1/2] fiber: unref fiber.storage via global Lua state
Igor Munkin
imun at tarantool.org
Thu Dec 12 02:57:03 MSK 2019
Vlad,
Thanks for the patch!
I spent some time knee deep into fibers machinery and totally agree with
this fix: the most safe approach is "unrefing" storage entity via
tarantool_L coro, taking into account the possible unref of the fiber's
one and its consequtive collection.
However, I don't get why we need to apply these changes, considering
your follow-up patch. Could you please provide a bit more detailed
rationale?
On 08.12.19, Vladislav Shpilevoy wrote:
> Fiber.storage is a table, available from anywhere in the fiber. It
> is destroyed after fiber function is finished. That provides a
> reliable fiber-local storage, similar to thread-local in C/C++.
>
> But there is a problem that the storage may be created via one
> struct lua_State, and destroyed via another. Here is an example:
>
> function test_storage()
> fiber.self().storage.key = 100
> end
> box.schema.func.create('test_storage')
> _ = fiber.create(function()
> box.func.test_storage:call()
> end)
>
> There are 3 struct lua_State:
> tarantool_L - global always alive Lua state;
> L1 - stack of the fiber, created by fiber.create();
> L2 - stack created by that fiber to execute test_storage().
Minor: Strictly saying, lua_State is a Lua coroutine, and not just a
guest stack. Please, consider adjusting the commit message, but feel
free to ignore this remark.
>
> Fiber.storage is created on stack of L2 and referenced by global
> LUA_REGISTRYINDEX. Then it is unreferenced from L1 when the fiber
> is being destroyed.
>
> That is generally ok as soon as the storage object is always in
> LUA_REGISTRYINDEX, which is shared by all Lua states.
>
> But soon during destruction of the fiber.storage there will be
> only tarantool_L and the original L2. Original L2 may be already
> deleted by the time the storage is being destroyed. So this patch
> makes unref of the storage via reliable tarantool_L.
>
> Needed for #4662
> ---
> src/lua/fiber.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/lua/fiber.c b/src/lua/fiber.c
> index 53ebec9aa..a7b75f9bf 100644
> --- a/src/lua/fiber.c
> +++ b/src/lua/fiber.c
> @@ -445,7 +445,7 @@ lua_fiber_run_f(MAYBE_UNUSED va_list ap)
> /* Destroy local storage */
> int storage_ref = f->storage.lua.ref;
> if (storage_ref > 0)
> - luaL_unref(L, LUA_REGISTRYINDEX, storage_ref);
> + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, storage_ref);
> /*
> * If fiber is not joinable
> * We can unref child stack here,
> --
> 2.21.0 (Apple Git-122.2)
>
--
Best regards,
IM
More information about the Tarantool-patches
mailing list