Re: [Tarantool-patches] [PATCH] lua: prohibit fiber yield when GC hook is active

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

Hi! Thanks for the patch!

On 02.10.2020 16:39, Igor Munkin wrote:
> While running GC hook (i.e. __gc  metamethod) garbage collector engine
> is "stopped": the memory penalty threshold is set to LJ_MAX_MEM and
> incremental GC step is not triggered as a result. Ergo, yielding the
> execution at the finalizer body leads to further running platform with
> disabled LuaJIT GC. It is not re-enabled until the yielded fiber doesn't
> get the execution back.
> 
> This changeset extends <cord_on_yield> routine with the check whether GC
> hook is active. If the switch-over occurs in scope of __gc metamethod
> the platform is forced to stop its execution with EXIT_FAILURE and calls
> panic routine before the exit.
> 
> Relates to #4518
> Follows up #4727
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> Vlad introduced the internal interface and local internal background
> fiber in scope of 8443bd9 ("fiber: introduce schedule_task() internal
> function") to postpone any yielding finalization (e.g. 3d5b4da ("fio:
> close unused descriptors automatically") and f073834 ("swim: use
> fiber._internal.schedule_task() for GC")). After this patch is merged we
> need to update docs and provide users a correct scenario to detect and
> fix yielding finalizers.

What is the scenario to fix? If you propose to expose schedule_task(),
I would better avoid it. We need a properly designed pool of Lua fibers
with stable API to be able to expose it. Until we did it, users should
do their own 'schedule_task()' or a similar alternative.

The patch LGTM.