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 2DE2B4765E0 for ; Tue, 22 Dec 2020 19:38:49 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\)) From: Sergey Ostanevich In-Reply-To: <20201222130858.16812-1-skaplun@tarantool.org> Date: Tue, 22 Dec 2020 19:37:47 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <60416D4E-B738-4E6D-A9DB-7D3E678E12C1@tarantool.org> References: <20201222130858.16812-1-skaplun@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] lua: avoid panic if HOOK_GC is not active List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Kaplun Cc: Michael Filonenko , tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! Sergos > On 22 Dec 2020, at 16:08, Sergey Kaplun wrote: >=20 > Panic at fiber.yield() occures inside any active hook. >=20 > This is the regression from 96dbc49d097a96af5273cce2b5663db5917f4ea9 ^^^^^^^^^^ It is a ^^^ caused by > ('lua: prohibit fiber yield when GC hook is active'). >=20 > This patch fixes false positive panic in cases when VM is not running > a GC hook. >=20 > Reported-by: Michael Filonenko > Follows up #4727 > Closes #5649 > --- >=20 > 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 >=20 > 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 >=20 > 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)) { It can happens the GC hook is not active? Should we have a second test = for HOOK_ACTIVE? > struct lua_State *L =3D fiber()->storage.lua.stack; > assert(L !=3D 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 =3D require('fiber') > +local tap =3D require('tap') > +local test =3D 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 =3D 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) > --=20 > 2.28.0 >=20