From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (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 9EEAF469719 for ; Fri, 13 Nov 2020 11:27:25 +0300 (MSK) References: <20201031162911.61876-1-sergos@tarantool.org> <7e2cf16f-f443-5a4b-d658-8d4e3ecd6f74@tarantool.org> <7DB50565-93F7-444F-9812-F73A5DBAFCA9@tarantool.org> From: Leonid Vasiliev Message-ID: <8e3d200a-d97c-6dd6-7f1e-d88e7f4acdac@tarantool.org> Date: Fri, 13 Nov 2020 11:26:45 +0300 MIME-Version: 1.0 In-Reply-To: <7DB50565-93F7-444F-9812-F73A5DBAFCA9@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH v2] core: handle fiber cancellation for fiber.cond List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy , alexander.turenko@tarantool.org Hi! Generally - LGTM. Yet another iteration of the review with me is not needed. But please see a comment on diag_set under the condition. On 12.11.2020 23:15, Sergey Ostanevich wrote: > Sorry, the patch was not updated. Here’s a new one and the branch is updated > > commit 42ea06f7cf69ee6b492cefcbb201bc9bba15c686 > Author: Sergey Ostanevich > Date: Tue Nov 3 12:52:26 2020 +0300 > > core: handle fiber cancellation for fiber.cond > > Before this patch fiber.cond():wait() just returns for cancelled > fiber. In contrast fiber.channel():get() threw "fiber is > canceled" error. > This patch unify behaviour of channels and condvars and also fixes > related net.box module problem - it was impossible to interrupt > net.box call with fiber.cancel because it used fiber.cond under > the hood. Test cases for both bugs are added. > > Closes #4834 > Closes #5013 > > Co-authored-by: Oleg Babin > > @TarantoolBot document > Title: fiber.cond():wait() throws if fiber is cancelled > > Currently fiber.cond():wait() throws an error if waiting fiber is > cancelled. > > --- > > Github: https://gitlab.com/tarantool/tarantool/-/commits/sergos/gh-5013-fiber-cond > Issue: https://github.com/tarantool/tarantool/issues/5013 > > @Changelog > * fiber.cond().wait() now throws if fiber is cancelled fiber.cond():wait() > > diff --git a/src/box/box.cc b/src/box/box.cc > index 18568df3b..29f74e94b 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -305,10 +305,9 @@ box_wait_ro(bool ro, double timeout) > { > double deadline = ev_monotonic_now(loop()) + timeout; > while (is_box_configured == false || box_is_ro() != ro) { > - if (fiber_cond_wait_deadline(&ro_cond, deadline) != 0) > - return -1; > - if (fiber_is_cancelled()) { > - diag_set(FiberIsCancelled); > + if (fiber_cond_wait_deadline(&ro_cond, deadline) != 0) { > + if (fiber_is_cancelled()) > + diag_set(FiberIsCancelled); > return -1; > } > } > diff --git a/src/lib/core/fiber_cond.c b/src/lib/core/fiber_cond.c > index 904a350d9..cc59eaafb 100644 > --- a/src/lib/core/fiber_cond.c > +++ b/src/lib/core/fiber_cond.c > @@ -108,6 +108,11 @@ fiber_cond_wait_timeout(struct fiber_cond *c, double timeout) > diag_set(TimedOut); > return -1; > } > + if (fiber_is_cancelled()) { > + if (diag_is_empty(diag_get())) > + diag_set(FiberIsCancelled); I read your argumentation. AFAIU sounds like:"We have a lot of tests that check an error in the diag in such case. Also, some user applications can check the diag in a similar way. And stack diag can be used only in the new versions of tarantool." So, it's true. What I propose to think about: - this is not simple logic. If a fiber has been cancelled, maybe an error will be added to the diag, maybe no - 50/50. - this behavior should be documented. - ask Mons or Turenko for advice: "What do they think about it?" All this up to you. > + return -1; > + } > return 0; > } > > diff --git a/src/lib/core/fiber_cond.h b/src/lib/core/fiber_cond.h > index 87c6f2ca2..2662e0654 100644 > --- a/src/lib/core/fiber_cond.h > +++ b/src/lib/core/fiber_cond.h > @@ -114,7 +114,7 @@ fiber_cond_broadcast(struct fiber_cond *cond); > * @param cond condition > * @param timeout timeout in seconds > * @retval 0 on fiber_cond_signal() call or a spurious wake up > - * @retval -1 on timeout, diag is set to TimedOut > + * @retval -1 on timeout or fiber cancellation, diag is set > */ > int > fiber_cond_wait_timeout(struct fiber_cond *cond, double timeout); > diff --git a/test/app-tap/gh-5013-fiber-cancel.test.lua b/test/app-tap/gh-5013-fiber-cancel.test.lua > new file mode 100755 > index 000000000..ca4ca2c90 > --- /dev/null > +++ b/test/app-tap/gh-5013-fiber-cancel.test.lua > @@ -0,0 +1,23 @@ > +#!/usr/bin/env tarantool > + > +local tap = require('tap') > +local fiber = require('fiber') > +local test = tap.test("gh-5013-fiber-cancel") > + > +test:plan(2) > + > +local result = {} > + > +function test_f() > + local cond = fiber.cond() > + local res, err = pcall(cond.wait, cond) > + result.res = res > + result.err = err > +end > + > +local f = fiber.create(test_f) > +f:cancel() > +fiber.yield() > + > +test:ok(result.res == false, 'expected result is false') > +test:ok(tostring(result.err) == 'fiber is cancelled', 'fiber cancellation should be reported') > diff --git a/test/box/net.box_fiber_cancel_gh-4834.result b/test/box/net.box_fiber_cancel_gh-4834.result > new file mode 100644 > index 000000000..4ed04bb61 > --- /dev/null > +++ b/test/box/net.box_fiber_cancel_gh-4834.result > @@ -0,0 +1,62 @@ > +-- test-run result file version 2 > +remote = require 'net.box' > + | --- > + | ... > +fiber = require 'fiber' > + | --- > + | ... > +test_run = require('test_run').new() > + | --- > + | ... > + > +-- #4834: Cancelling fiber doesn't interrupt netbox operations > +function infinite_call() fiber.channel(1):get() end > + | --- > + | ... > +box.schema.func.create('infinite_call') > + | --- > + | ... > +box.schema.user.grant('guest', 'execute', 'function', 'infinite_call') > + | --- > + | ... > + > +error_msg = nil > + | --- > + | ... > +test_run:cmd("setopt delimiter ';'") > + | --- > + | - true > + | ... > +function netbox_runner() > + local cn = remote.connect(box.cfg.listen) > + local f = fiber.new(function() > + _, error_msg = pcall(cn.call, cn, 'infinite_call') > + end) > + f:set_joinable(true) > + fiber.yield() > + f:cancel() > + f:join() > + cn:close() > +end; > + | --- > + | ... > +test_run:cmd("setopt delimiter ''"); > + | --- > + | - true > + | ... > +netbox_runner() > + | --- > + | ... > +error_msg > + | --- > + | - fiber is cancelled > + | ... > +box.schema.func.drop('infinite_call') > + | --- > + | ... > +infinite_call = nil > + | --- > + | ... > +error_msg = nil > + | --- > + | ... > diff --git a/test/box/net.box_fiber_cancel_gh-4834.test.lua b/test/box/net.box_fiber_cancel_gh-4834.test.lua > new file mode 100644 > index 000000000..bc0e5af6e > --- /dev/null > +++ b/test/box/net.box_fiber_cancel_gh-4834.test.lua > @@ -0,0 +1,28 @@ > +remote = require 'net.box' > +fiber = require 'fiber' > +test_run = require('test_run').new() > + > +-- #4834: Cancelling fiber doesn't interrupt netbox operations > +function infinite_call() fiber.channel(1):get() end > +box.schema.func.create('infinite_call') > +box.schema.user.grant('guest', 'execute', 'function', 'infinite_call') > + > +error_msg = nil > +test_run:cmd("setopt delimiter ';'") > +function netbox_runner() > + local cn = remote.connect(box.cfg.listen) > + local f = fiber.new(function() > + _, error_msg = pcall(cn.call, cn, 'infinite_call') > + end) > + f:set_joinable(true) > + fiber.yield() > + f:cancel() > + f:join() > + cn:close() > +end; > +test_run:cmd("setopt delimiter ''"); > +netbox_runner() > +error_msg > +box.schema.func.drop('infinite_call') > +infinite_call = nil > +error_msg = nil > > ====================== > >> On 11 Nov 2020, at 00:16, Sergey Ostanevich wrote: >> >> Hi Leonid, thank you for the review! >> >> I put two parts together to handle them in one patch. >> >>> On 4 Nov 2020, at 13:00, Leonid Vasiliev wrote: >>> >>> Hi! Thank you for the patch. >>> See some comments below: >>> >>> 1) The patch changes undocumented behavior, AFAIU. >>> So, I have a question:"Do you plan to backport the patch to >>> tarantool 1.10?". If the answer is "Yes" - I'm comfortable with the >>> changes. But if the answer is "No" - I will object, because in this case >>> both behaviors must be supported in all modules. >>> >> >> Yes, the plan is to push it down to 1.10 >> >>> 2) I think changing the behavior in C doesn't cause much of a problem, >>> because before when you wait without timeout, you don't need to check >>> the return value (it's always 0). But in Lua it will cause the problems, >>> because now throws an error if cancelled and all wait calls should be >>> wrapped to pcall. >> >> Exactly it is the change to Lua and we intentionally make it, since original >> behaviour is wrong. Good things are: fiber.cond has no big presence in the >> /tarantool sourcebase and the cancellation of fiber itself means the fiber >> has to quit. >> >>> >>> On 31.10.2020 19:29, sergos@tarantool.org wrote: >>>> From: Sergey Ostanevich >>>> Hi! >>>> Thanks to Oleg Babin's comment I found there's no need to update any lua >>>> interfaces, since the reason was in C implementation. Also, there is one >>>> place the change is played, so after I fixed it I got complete testing >>>> pass. >>>> Force-pushed branch, v2 patch attached. >>>> Before this patch fiber.cond():wait() just returns for cancelled >>>> fiber. In contrast fiber.channel():get() threw "fiber is >>>> canceled" error. >>>> This patch unify behaviour of channels and condvars and also fixes >>> >>> 3) behaviour -> behavior. >> >> Depends on spellchecker - mine sets it to this British version. >> >>> >>>> related net.box module problem - it was impossible to interrupt >>>> net.box call with fiber.cancel because it used fiber.cond under >>>> the hood. Test cases for both bugs are added. >>>> Closes #4834 >>>> Closes #5013 >>>> Co-authored-by: Oleg Babin >>>> @TarantoolBot document >>>> Title: fiber.cond():wait() throws if fiber is cancelled >>>> Currently fiber.cond():wait() throws an error if waiting fiber is >>>> cancelled like in case with fiber.channel():get(). >>> >>> 4) I don't think it's a good decision adding a comparison with >>> fiber.channel():get() to the documentation. Up to you. >>> >> >> I’d agree to you: let’s just put the new behaviour here. >> >>> 5) Document the changes in module.h. >> >> Done (need to rebuild, obviously). >> >>> >>>> --- >>> >>> 6) Add @ChangeLog. >>> >>>> Github: https://gitlab.com/tarantool/tarantool/-/commits/sergos/gh-5013-fiber-cond >>>> Issue: https://github.com/tarantool/tarantool/issues/5013 >>>> src/box/box.cc | 6 +- >>>> src/lib/core/fiber_cond.c | 1 + >>>> test/app-tap/gh-5013-fiber-cancel.test.lua | 23 +++++++ >>>> test/box/net.box_fiber_cancel_gh-4834.result | 65 +++++++++++++++++++ >>>> .../box/net.box_fiber_cancel_gh-4834.test.lua | 29 +++++++++ >>>> 5 files changed, 120 insertions(+), 4 deletions(-) >>>> create mode 100755 test/app-tap/gh-5013-fiber-cancel.test.lua >>>> create mode 100644 test/box/net.box_fiber_cancel_gh-4834.result >>>> create mode 100644 test/box/net.box_fiber_cancel_gh-4834.test.lua >>> >>>> diff --git a/src/box/box.cc b/src/box/box.cc >>>> index 18568df3b..9e824453d 100644 >>>> --- a/src/box/box.cc >>>> +++ b/src/box/box.cc >>>> @@ -305,10 +305,8 @@ box_wait_ro(bool ro, double timeout) >>>> { >>>> double deadline = ev_monotonic_now(loop()) + timeout; >>>> while (is_box_configured == false || box_is_ro() != ro) { >>>> - if (fiber_cond_wait_deadline(&ro_cond, deadline) != 0) >>>> - return -1; >>>> - if (fiber_is_cancelled()) { >>>> - diag_set(FiberIsCancelled); >>>> + if (fiber_cond_wait_deadline(&ro_cond, deadline) != 0) { >>>> + if (fiber_is_cancelled()) diag_set(FiberIsCancelled); >>> >>> 7) According to https://www.tarantool.io/en/doc/latest/dev_guide/c_style_guide/ >>> it's seems like you are trying to hide something) >>> Use: >>> ``` >>> if (condition) >>> action(); >>> ``` >>> Instead of: >>> ``` >>> if (condition) action(); >>> another_action(); >>> ``` >> >> Fixed. >> >>>> return -1; >>>> } >>>> } >>>> diff --git a/src/lib/core/fiber_cond.c b/src/lib/core/fiber_cond.c >>>> index 904a350d9..0c93c5842 100644 >>>> --- a/src/lib/core/fiber_cond.c >>>> +++ b/src/lib/core/fiber_cond.c >>>> @@ -108,6 +108,10 @@ fiber_cond_wait_timeout(struct fiber_cond *c, double timeout) >>>> diag_set(TimedOut); >>>> return -1; >>>> } >>>> + if (fiber_is_cancelled()) { >>>> + if (diag_is_empty(diag_get())) diag_set(FiberIsCancelled); >>> >>> 8) The same as previously: >>> ``` >>> if (condition) >>> action(); >>> ``` >>> >> >> Fixed. >> >>> 9) Checking diag on empty looks strange to me. I think we should add an >>> error to diag without this check. If we want to save the previous error, >>> I suggest to use the stack diag. >> >> I made this because this interface is used inside the testing machinery, where >> exact cause of e.g. replica disconnection is reported. If we put the diag here >> unconditionally it will break this use of diag. I believe the stack diag also >> is not supported there yet. >> >> >>>> + return -1; >>>> + } >>>> return 0; >>>> } >>>> diff --git a/test/app-tap/gh-5013-fiber-cancel.test.lua b/test/app-tap/gh-5013-fiber-cancel.test.lua >>>> new file mode 100755 >>>> index 000000000..ae805c5bf >>>> --- /dev/null >>>> +++ b/test/app-tap/gh-5013-fiber-cancel.test.lua >>>> @@ -0,0 +1,23 @@ >>>> +#!/usr/bin/env tarantool >>>> + >>>> +local tap = require('tap') >>>> +local fiber = require('fiber') >>>> +local test = tap.test("gh-5013-fiber-cancel") >>>> + >>>> +test:plan(2) >>>> + >>>> +local result = {} >>>> + >>>> +function test_f() >>>> + local cond = fiber.cond() >>>> + local res, err = pcall(cond.wait, cond) >>>> + result.res = res >>>> + result.err = err >>>> +end >>>> + >>>> +local f = fiber.create(test_f) >>>> +f:cancel() >>>> +fiber.yield() >>>> + >>>> +test:ok(result.res == false, tostring(result.res)) >>>> +test:ok(tostring(result.err) == 'fiber is cancelled', tostring(result.err)) >>> >>> 10) Use a user-frendly check message (in both checks). >>> >> Fixed >> >>>> diff --git a/test/box/net.box_fiber_cancel_gh-4834.result b/test/box/net.box_fiber_cancel_gh-4834.result >>>> new file mode 100644 >>>> index 000000000..eab0a5e4d >>>> --- /dev/null >>>> +++ b/test/box/net.box_fiber_cancel_gh-4834.result >>>> @@ -0,0 +1,65 @@ >>>> +-- test-run result file version 2 >>>> +remote = require 'net.box' >>>> + | --- >>>> + | ... >>>> +fiber = require 'fiber' >>>> + | --- >>>> + | ... >>>> +test_run = require('test_run').new() >>>> + | --- >>>> + | ... >>>> + >>>> +-- #4834: Cancelling fiber doesn't interrupt netbox operations >>>> +function infinite_call() fiber.channel(1):get() end >>>> + | --- >>>> + | ... >>>> +box.schema.func.create('infinite_call') >>>> + | --- >>>> + | ... >>>> +box.schema.user.grant('guest', 'execute', 'function', 'infinite_call') >>>> + | --- >>>> + | ... >>>> + >>>> +error_msg = nil >>>> + | --- >>>> + | ... >>>> +test_run:cmd("setopt delimiter ';'") >>>> + | --- >>>> + | - true >>>> + | ... >>>> +function gh4834( > + local cn = remote.connect(box.cfg.listen) >>>> + local f = fiber.new(function() >>>> + _, error_msg = pcall(cn.call, cn, 'infinite_call') >>>> + end) >>>> + f:set_joinable(true) >>>> + fiber.yield() >>>> + f:cancel() >>>> + f:join() >>>> + cn:close() >>>> +end; >>>> + | --- >>>> + | ... >>>> +test_run:cmd("setopt delimiter ''"); >>>> + | --- >>>> + | - true >>>> + | ... >>>> +gh4834() >>>> + | --- >>>> + | ... >>>> +error_msg >>>> + | --- >>>> + | - fiber is cancelled >>>> + | ... >>>> +box.schema.func.drop('infinite_call') >>>> + | --- >>>> + | ... >>>> +infinite_call = nil >>>> + | --- >>>> + | ... >>>> +channel = nil >>>> + | --- >>>> + | ... >>>> +error_msg = nil >>>> + | --- >>>> + | ... >>>> diff --git a/test/box/net.box_fiber_cancel_gh-4834.test.lua b/test/box/net.box_fiber_cancel_gh-4834.test.lua >>>> new file mode 100644 >>>> index 000000000..06fb3ceac >>>> --- /dev/null >>>> +++ b/test/box/net.box_fiber_cancel_gh-4834.test.lua >>>> @@ -0,0 +1,29 @@ >>>> +remote = require 'net.box' >>>> +fiber = require 'fiber' >>>> +test_run = require('test_run').new() >>>> + >>>> +-- #4834: Cancelling fiber doesn't interrupt netbox operations >>>> +function infinite_call() fiber.channel(1):get() end >>>> +box.schema.func.create('infinite_call') >>>> +box.schema.user.grant('guest', 'execute', 'function', 'infinite_call') >>>> + >>>> +error_msg = nil >>>> +test_run:cmd("setopt delimiter ';'") >>>> +function gh4834() >>> >>> 11) I think this is not a self-documenting name for the function. >>> >> Renamed. >> >>>> + local cn = remote.connect(box.cfg.listen) >>>> + local f = fiber.new(function() >>>> + _, error_msg = pcall(cn.call, cn, 'infinite_call') >>>> + end) >>>> + f:set_joinable(true) >>>> + fiber.yield() >>>> + f:cancel() >>>> + f:join() >>>> + cn:close() >>>> +end; >>>> +test_run:cmd("setopt delimiter ''"); >>>> +gh4834() >>>> +error_msg >>>> +box.schema.func.drop('infinite_call') >>>> +infinite_call = nil >>>> +channel = nil >>> >>> 12) You didn't introduce the variable "channel". In addition, I looked >>> at a couple of tests and didn't see them cleaning up variables. But I >>> don't mind. >>> >> Removed this one. In general, clean up prevents flaky. >> >>>> +error_msg = nil >> >> ================= >> >> Before this patch fiber.cond():wait() just returns for cancelled >> fiber. In contrast fiber.channel():get() threw "fiber is >> canceled" error. >> This patch unify behaviour of channels and condvars and also fixes >> related net.box module problem - it was impossible to interrupt >> net.box call with fiber.cancel because it used fiber.cond under >> the hood. Test cases for both bugs are added. >> >> Closes #4834 >> Closes #5013 >> >> Co-authored-by: Oleg Babin >> >> @TarantoolBot document >> Title: fiber.cond():wait() throws if fiber is cancelled >> >> Currently fiber.cond():wait() throws an error if waiting fiber is >> cancelled. >> --- >> >> Github: https://gitlab.com/tarantool/tarantool/-/commits/sergos/gh-5013-fiber-cond >> Issue: https://github.com/tarantool/tarantool/issues/5013 >> >> @Changelog >> * fiber.cond().wait() now throws if fiber is cancelled >> >> src/box/box.cc | 6 +- >> src/lib/core/fiber_cond.c | 4 ++ >> test/app-tap/gh-5013-fiber-cancel.test.lua | 23 +++++++ >> test/box/net.box_fiber_cancel_gh-4834.result | 65 +++++++++++++++++++ >> .../box/net.box_fiber_cancel_gh-4834.test.lua | 29 +++++++++ >> 5 files changed, 123 insertions(+), 4 deletions(-) >> create mode 100755 test/app-tap/gh-5013-fiber-cancel.test.lua >> create mode 100644 test/box/net.box_fiber_cancel_gh-4834.result >> create mode 100644 test/box/net.box_fiber_cancel_gh-4834.test.lua >> >> diff --git a/src/box/box.cc b/src/box/box.cc >> index 18568df3b..9e824453d 100644 >> --- a/src/box/box.cc >> +++ b/src/box/box.cc >> @@ -305,10 +305,8 @@ box_wait_ro(bool ro, double timeout) >> { >> double deadline = ev_monotonic_now(loop()) + timeout; >> while (is_box_configured == false || box_is_ro() != ro) { >> - if (fiber_cond_wait_deadline(&ro_cond, deadline) != 0) >> - return -1; >> - if (fiber_is_cancelled()) { >> - diag_set(FiberIsCancelled); >> + if (fiber_cond_wait_deadline(&ro_cond, deadline) != 0) { >> + if (fiber_is_cancelled()) diag_set(FiberIsCancelled); >> return -1; >> } >> } >> diff --git a/src/lib/core/fiber_cond.c b/src/lib/core/fiber_cond.c >> index 904a350d9..0c93c5842 100644 >> --- a/src/lib/core/fiber_cond.c >> +++ b/src/lib/core/fiber_cond.c >> @@ -108,6 +108,10 @@ fiber_cond_wait_timeout(struct fiber_cond *c, double timeout) >> diag_set(TimedOut); >> return -1; >> } >> + if (fiber_is_cancelled()) { >> + if (diag_is_empty(diag_get())) diag_set(FiberIsCancelled); >> + return -1; >> + } >> return 0; >> } >> >> diff --git a/test/app-tap/gh-5013-fiber-cancel.test.lua b/test/app-tap/gh-5013-fiber-cancel.test.lua >> new file mode 100755 >> index 000000000..34711fb31 >> --- /dev/null >> +++ b/test/app-tap/gh-5013-fiber-cancel.test.lua >> @@ -0,0 +1,23 @@ >> +#!/usr/bin/env tarantool >> + >> +local tap = require('tap') >> +local fiber = require('fiber') >> +local test = tap.test("gh-5013-fiber-cancel") >> + >> +test:plan(2) >> + >> +local result = {} >> + >> +function test_f() >> + local cond = fiber.cond() >> + local res, err = pcall(cond.wait, cond) >> + result.res = res >> + result.err = err >> +end >> + >> +local f = fiber.create(test_f) >> +f:cancel() >> +fiber.yield() >> + >> +test:ok(result.res == false, tostring(result.res)) >> +test:ok(tostring(result.err) == 'fiber is cancelled', tostring(result.err)) >> diff --git a/test/box/net.box_fiber_cancel_gh-4834.result b/test/box/net.box_fiber_cancel_gh-4834.result >> new file mode 100644 >> index 000000000..eab0a5e4d >> --- /dev/null >> +++ b/test/box/net.box_fiber_cancel_gh-4834.result >> @@ -0,0 +1,65 @@ >> +-- test-run result file version 2 >> +remote = require 'net.box' >> + | --- >> + | ... >> +fiber = require 'fiber' >> + | --- >> + | ... >> +test_run = require('test_run').new() >> + | --- >> + | ... >> + >> +-- #4834: Cancelling fiber doesn't interrupt netbox operations >> +function infinite_call() fiber.channel(1):get() end >> + | --- >> + | ... >> +box.schema.func.create('infinite_call') >> + | --- >> + | ... >> +box.schema.user.grant('guest', 'execute', 'function', 'infinite_call') >> + | --- >> + | ... >> + >> +error_msg = nil >> + | --- >> + | ... >> +test_run:cmd("setopt delimiter ';'") >> + | --- >> + | - true >> + | ... >> +function gh4834() >> + local cn = remote.connect(box.cfg.listen) >> + local f = fiber.new(function() >> + _, error_msg = pcall(cn.call, cn, 'infinite_call') >> + end) >> + f:set_joinable(true) >> + fiber.yield() >> + f:cancel() >> + f:join() >> + cn:close() >> +end; >> + | --- >> + | ... >> +test_run:cmd("setopt delimiter ''"); >> + | --- >> + | - true >> + | ... >> +gh4834() >> + | --- >> + | ... >> +error_msg >> + | --- >> + | - fiber is cancelled >> + | ... >> +box.schema.func.drop('infinite_call') >> + | --- >> + | ... >> +infinite_call = nil >> + | --- >> + | ... >> +channel = nil >> + | --- >> + | ... >> +error_msg = nil >> + | --- >> + | ... >> diff --git a/test/box/net.box_fiber_cancel_gh-4834.test.lua b/test/box/net.box_fiber_cancel_gh-4834.test.lua >> new file mode 100644 >> index 000000000..06fb3ceac >> --- /dev/null >> +++ b/test/box/net.box_fiber_cancel_gh-4834.test.lua >> @@ -0,0 +1,29 @@ >> +remote = require 'net.box' >> +fiber = require 'fiber' >> +test_run = require('test_run').new() >> + >> +-- #4834: Cancelling fiber doesn't interrupt netbox operations >> +function infinite_call() fiber.channel(1):get() end >> +box.schema.func.create('infinite_call') >> +box.schema.user.grant('guest', 'execute', 'function', 'infinite_call') >> + >> +error_msg = nil >> +test_run:cmd("setopt delimiter ';'") >> +function gh4834() >> + local cn = remote.connect(box.cfg.listen) >> + local f = fiber.new(function() >> + _, error_msg = pcall(cn.call, cn, 'infinite_call') >> + end) >> + f:set_joinable(true) >> + fiber.yield() >> + f:cancel() >> + f:join() >> + cn:close() >> +end; >> +test_run:cmd("setopt delimiter ''"); >> +gh4834() >> +error_msg >> +box.schema.func.drop('infinite_call') >> +infinite_call = nil >> +channel = nil >> +error_msg = nil >> -- >> 2.29.2 >