From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Serge Petrenko Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_9A61C471-84E6-4295-BF8B-ACE58FD99E3C" Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: Re: [tarantool-patches] Re: [PATCH v2] Fix fiber_join() hang in case fiber_cancel() was called Date: Wed, 6 Feb 2019 15:58:39 +0300 In-Reply-To: <20190206095555.j5krlav7eawshkkh@esperanza> References: <20190205150111.63224-1-sergepetrenko@tarantool.org> <20190206095555.j5krlav7eawshkkh@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: --Apple-Mail=_9A61C471-84E6-4295-BF8B-ACE58FD99E3C Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi, thankyou for review. All fixed. Please see v3. > 6 =D1=84=D0=B5=D0=B2=D1=80. 2019 =D0=B3., =D0=B2 12:55, Vladimir = Davydov =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB= (=D0=B0): >=20 > Anyway, now I seem to understand why you wanted to ignore fiber_cancel > in fiber_join - making it cancellable complicates the function = protocol, > rendering the function barely usable. Guess I was wrong when asked you > to rework the patch, sorry. Let's revert to v1. >=20 >> if (child_L !=3D NULL) { >> coro_ref =3D lua_tointeger(child_L, -1); >> lua_pop(child_L, 1); >> diff --git a/test/app/fiber.result b/test/app/fiber.result >> index ab7c1941b..f73d32671 100644 >> --- a/test/app/fiber.result >> +++ b/test/app/fiber.result >> @@ -1411,6 +1411,39 @@ l =3D nil >> l1 =3D nil >> --- >> ... >> +-- gh-3948 fiber.join() blocks if fiber is cancelled. >> +function another_func() fiber.yield() end >> +--- >> +... >> +test_run:cmd("setopt delimiter ';'") >> +--- >> +- true >> +... >> +function func() >> + local fib =3D fiber.create(another_func) >> + fib:set_joinable(true) >> + fib:join() >> +end; >> +--- >> +... >> +f =3D fiber.create(func) >> +f:cancel() >> +while f:status() ~=3D 'dead' do fiber.sleep(0.01) end; >=20 > AFAICS the test highly depends on the scheduler algorithm. Let's = rewrite > it using fiber.channel please. >=20 >> +--- >> +... >> +test_run:cmd("setopt delimiter ''"); >> +--- >> +- true >> +... >=20 >> +f =3D nil >> +--- >> +... >> +func =3D nil >> +--- >> +... >> +another_func =3D nil >> +--- >> +... >=20 > These assignments aren't necessary. --Apple-Mail=_9A61C471-84E6-4295-BF8B-ACE58FD99E3C Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Hi, = thankyou for review.
All fixed. Please see v3.

6 =D1=84=D0=B5=D0=B2=D1=80. 2019 =D0=B3., =D0=B2 12:55, = Vladimir Davydov <vdavydov.dev@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0= =B0=D0=BB(=D0=B0):

Anyway, now I seem to understand why you wanted to ignore = fiber_cancel
in fiber_join = - making it cancellable complicates the function protocol,
rendering the function barely = usable. Guess I was wrong when asked you
to rework the patch, sorry. Let's revert to v1.

if = (child_L !=3D NULL) {
coro_ref =3D = lua_tointeger(child_L, -1);
lua_pop(child_L, 1);
diff --git a/test/app/fiber.result b/test/app/fiber.result
index ab7c1941b..f73d32671 100644
--- = a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -1411,6 +1411,39 @@ l =3D nil
l1 =3D nil
---
...
+-- gh-3948 fiber.join() = blocks if fiber is cancelled.
+function another_func() = fiber.yield() end
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+function func()
+    local fib =3D = fiber.create(another_func)
+ =    fib:set_joinable(true)
+ =    fib:join()
+end;
+---
+...
+f =3D fiber.create(func)
+f:cancel()
+while f:status() ~=3D 'dead' do = fiber.sleep(0.01) end;

AFAICS the test highly depends = on the scheduler algorithm. Let's rewrite
it using fiber.channel please.

+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...

+f =3D = nil
+---
+...
+func =3D nil
+---
+...
+another_func =3D = nil
+---
+...

These assignments aren't = necessary.

= --Apple-Mail=_9A61C471-84E6-4295-BF8B-ACE58FD99E3C--