From: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] lua: avoid panic if HOOK_GC is not active
Date: Wed, 23 Dec 2020 13:23:42 +0300 [thread overview]
Message-ID: <20201223102342.GA77688@hpalx> (raw)
In-Reply-To: <20201222130858.16812-1-skaplun@tarantool.org>
Hi Sergey, thanks for the patch, as I see no new degradation found in
gitlab-ci testing commit criteria pipeline [1], patch LGTM.
[1] - https://gitlab.com/tarantool/tarantool/-/pipelines/233455482
On Tue, Dec 22, 2020 at 04:08:58PM +0300, Sergey Kaplun via Tarantool-patches wrote:
> Panic at fiber.yield() occures inside any active hook.
>
> This is the regression from 96dbc49d097a96af5273cce2b5663db5917f4ea9
> ('lua: prohibit fiber yield when GC hook is active').
>
> This patch fixes false positive panic in cases when VM is not running
> a GC hook.
>
> Reported-by: Michael Filonenko <filonenko.mikhail@gmail.com>
> Follows up #4727
> Closes #5649
> ---
>
> Branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-5649-fix-for-debug-hook
> Issue: https://github.com/tarantool/tarantool/issues/5649
> CI: https://gitlab.com/tarantool/tarantool/-/pipelines/233249505
>
> src/lua/utils.c | 2 +-
> .../gh-5649-yield-in-debug-hook.test.lua | 25 +++++++++++++++++++
> 2 files changed, 26 insertions(+), 1 deletion(-)
> create mode 100755 test/app-tap/gh-5649-yield-in-debug-hook.test.lua
>
> diff --git a/src/lua/utils.c b/src/lua/utils.c
> index 23fbdd4ad..b5a6ca5b7 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -1375,7 +1375,7 @@ void cord_on_yield(void)
> * earlier. As a result fiber switch is prohibited when
> * GC hook is active and the platform is forced to stop.
> */
> - if (unlikely(g->hookmask & (HOOK_ACTIVE|HOOK_GC))) {
> + if (unlikely(g->hookmask & HOOK_GC)) {
> struct lua_State *L = fiber()->storage.lua.stack;
> assert(L != NULL);
> lua_pushfstring(L, "fiber %d is switched while running GC"
> diff --git a/test/app-tap/gh-5649-yield-in-debug-hook.test.lua b/test/app-tap/gh-5649-yield-in-debug-hook.test.lua
> new file mode 100755
> index 000000000..9c1cca51f
> --- /dev/null
> +++ b/test/app-tap/gh-5649-yield-in-debug-hook.test.lua
> @@ -0,0 +1,25 @@
> +#!/usr/bin/env tarantool
> +
> +local fiber = require('fiber')
> +local tap = require('tap')
> +local test = tap.test('yield-in-debug-hook')
> +
> +test:plan(2)
> +
> +-- Test that HOOK_ACTIVE is not enough to panic and
> +-- fiber still can use general hooks at switches.
> +fiber.create(function()
> + local old_hook, mask, count = debug.gethook()
> + debug.sethook(function()
> + fiber.yield()
> + end, 'c')
> + -- All ok if panic doesn't occure.
> + -- Yield before hook is set back.
> + debug.sethook(old_hook, mask, count)
> + test:ok(true)
> +end)
> +-- Return to second fiber.
> +fiber.yield()
> +test:ok(true)
> +
> +os.exit(test:check() and 0 or 1)
> --
> 2.28.0
>
next prev parent reply other threads:[~2020-12-23 10:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-22 13:08 Sergey Kaplun
2020-12-22 16:05 ` Igor Munkin
2020-12-22 16:09 ` Igor Munkin
2020-12-22 17:51 ` Sergey Kaplun
2020-12-22 17:55 ` Sergey Kaplun
2020-12-22 16:37 ` Sergey Ostanevich
2020-12-22 17:47 ` Sergey Kaplun
2020-12-22 18:01 ` Sergey Kaplun
2020-12-23 9:47 ` Sergey Ostanevich
2020-12-23 10:23 ` Alexander V. Tikhonov [this message]
2020-12-23 13:05 ` Kirill Yukhin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201223102342.GA77688@hpalx \
--to=avtikhon@tarantool.org \
--cc=skaplun@tarantool.org \
--cc=tarantool-patches@dev.tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH] lua: avoid panic if HOOK_GC is not active' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox