Re: [Tarantool-patches] [PATCH 1/2] fiber: unref fiber.storage via global Lua state

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

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().
"