From e7a5120603f362364953315d31aa529342263e0b Mon Sep 17 00:00:00 2001 From: Oleg Babin Date: Sun, 6 Sep 2020 08:43:25 +0300 Subject: [PATCH] core: properly 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 @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(). --- src/lib/core/fiber_cond.c | 4 ++ test/app/gh-5013-fiber_cond-cancel.result | 46 +++++++++++++ test/app/gh-5013-fiber_cond-cancel.test.lua | 17 +++++ test/box/net.box_fiber_cancel_gh-4834.result | 65 +++++++++++++++++++ .../box/net.box_fiber_cancel_gh-4834.test.lua | 29 +++++++++ 5 files changed, 161 insertions(+) create mode 100644 test/app/gh-5013-fiber_cond-cancel.result create mode 100644 test/app/gh-5013-fiber_cond-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/lib/core/fiber_cond.c b/src/lib/core/fiber_cond.c index 904a350d9ddf..71bb2d04d696 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()) { + diag_set(FiberIsCancelled); + return -1; + } return 0; } diff --git a/test/app/gh-5013-fiber_cond-cancel.result b/test/app/gh-5013-fiber_cond-cancel.result new file mode 100644 index 000000000000..611fc2d6801d --- /dev/null +++ b/test/app/gh-5013-fiber_cond-cancel.result @@ -0,0 +1,46 @@ +-- test-run result file version 2 +fiber = require('fiber') + | --- + | ... +test_run = require('test_run').new() + | --- + | ... + +err = nil + | --- + | ... + +test_run:cmd("setopt delimiter ';'") + | --- + | - true + | ... +function test() + _, err = pcall(function() fiber.cond():wait() end) +end; + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +f = fiber.new(test) + | --- + | ... +fiber.yield() + | --- + | ... +f:cancel() + | --- + | ... +fiber.yield() + | --- + | ... +f + | --- + | - the fiber is dead + | ... +err + | --- + | - fiber is cancelled + | ... diff --git a/test/app/gh-5013-fiber_cond-cancel.test.lua b/test/app/gh-5013-fiber_cond-cancel.test.lua new file mode 100644 index 000000000000..0934fb58a474 --- /dev/null +++ b/test/app/gh-5013-fiber_cond-cancel.test.lua @@ -0,0 +1,17 @@ +fiber = require('fiber') +test_run = require('test_run').new() + +err = nil + +test_run:cmd("setopt delimiter ';'") +function test() + _, err = pcall(function() fiber.cond():wait() end) +end; +test_run:cmd("setopt delimiter ''"); + +f = fiber.new(test) +fiber.yield() +f:cancel() +fiber.yield() +f +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 000000000000..eab0a5e4d0b9 --- /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 000000000000..06fb3ceac661 --- /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