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

[Igor Munkin <imun@tarantool.org>]

Sorry, forgot branch, issues and ChangeLog entry. Too fast, too furious.

On 02.10.20, 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>
> ---

@ChangeLog:
* Fixed fibers switch-over to prevent implicit GC disabling. The yield
  occurring while user-defined __gc metatmethod is running leads to the
  platform panic.

Branch: https://github.com/tarantool/tarantool/tree/imun/gh-4518-forbid-yield-in-GC-finalizer
Issues:
* https://github.com/tarantool/tarantool/issues/4518
* https://github.com/tarantool/tarantool/issues/4727

> 
> 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.
> 

Benchmark is the same[1].

> Here are also the benchmark results for the Release build:
> * Vanilla (2711797) -> Patched (61072ba) (min, median, mean, max):
> | fibers: 10; iters: 100	0%	0%	0%	2%
> | fibers: 10; iters: 1000	-3%	0%	0%	1%
> | fibers: 10; iters: 10000	-3%	0%	-1%	-2%
> | fibers: 10; iters: 100000	0%	0%	0%	1%
> | fibers: 100; iters: 100	-1%	-2%	-2%	-9%
> | fibers: 100; iters: 1000	0%	0%	0%	5%
> | fibers: 100; iters: 10000	0%	0%	0%	0%
> | fibers: 100; iters: 100000	0%	0%	0%	-3%
> | fibers: 1000; iters: 100	0%	0%	0%	0%
> | fibers: 1000; iters: 1000	0%	0%	0%	0%
> | fibers: 1000; iters: 10000	0%	0%	0%	1%
> | fibers: 1000; iters: 100000	0%	0%	0%	2%
> | fibers: 10000; iters: 100	0%	-3%	-1%	-2%
> | fibers: 10000; iters: 1000	0%	-2%	-3%	-2%
> | fibers: 10000; iters: 10000	0%	0%	0%	-2%
> | fibers: 10000; iters: 100000	0%	-3%	-3%	-6%
> 
>  src/lua/utils.c                             | 26 ++++++++++++++-
>  test/app-tap/yield-in-gc-finalizer.test.lua | 36 +++++++++++++++++++++
>  2 files changed, 61 insertions(+), 1 deletion(-)
>  create mode 100755 test/app-tap/yield-in-gc-finalizer.test.lua
> 

<snipped>

> -- 
> 2.25.0
> 

[1]: https://gist.github.com/igormunkin/7e0cf48005bd003ffbdf30181eedb40e

-- 
Best regards,
IM