[Tarantool-patches] [PATCH 1/2] fiber: unref fiber.storage via global Lua state
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri Dec 13 02:38:00 MSK 2019
Hi! Thanks for the review!
On 12/12/2019 00:57, Igor Munkin wrote:
> 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?
My follow up patch is about a different bug and I wanted to keep
these independent fixes separated. That would have been strange,
if in the second commit I had just silently replaced the old L
with tarantool_L.
>
> 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.
>
Agree, here is a new version of this paragraph:
"
There are 3 struct lua_State:
tarantool_L - global always alive state;
L1 - Lua coroutine of the fiber, created by fiber.create();
L2 - Lua coroutine created by that fiber to execute
test_storage().
"
More information about the Tarantool-patches
mailing list