[Tarantool-patches] [PATCH 1/2] fiber: unref fiber.storage via global Lua state
Igor Munkin
imun at tarantool.org
Fri Dec 13 13:44:26 MSK 2019
Vlad,
On 13.12.19, Vladislav Shpilevoy wrote:
> 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.
>
Yes, on second thought I agree with you and Kostja, so here is my LGTM
for this patch.
> >
> > 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().
> "
--
Best regards,
IM
More information about the Tarantool-patches
mailing list