From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D9BA04696C0 for ; Fri, 13 Dec 2019 02:38:01 +0300 (MSK) References: <20191211235703.GK1214@tarantool.org> From: Vladislav Shpilevoy Message-ID: <175e0c98-8f2d-ff20-4280-63c7a3f90432@tarantool.org> Date: Fri, 13 Dec 2019 00:38:00 +0100 MIME-Version: 1.0 In-Reply-To: <20191211235703.GK1214@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 1/2] fiber: unref fiber.storage via global Lua state List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.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(). "