From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 C2CB2469719 for ; Wed, 28 Oct 2020 19:22:16 +0300 (MSK) From: sergos@tarantool.org Date: Wed, 28 Oct 2020 19:22:12 +0300 Message-Id: <20201028162212.1550-1-sergos@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] lua: handle fiber cancellation for fiber.cond List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: alexander.turenko@tarantool.org From: Sergey Ostanevich 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 like in case with fiber.channel():get(). --- Github: https://gitlab.com/tarantool/tarantool/-/commits/sergos/gh-5013-fiber-cond Issue: https://github.com/tarantool/tarantool/issues/5013 src/lua/fiber_cond.c | 3 +- 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 +++++++++ 4 files changed, 118 insertions(+), 2 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/lua/fiber_cond.c b/src/lua/fiber_cond.c index 617226969..fdb2ab7fe 100644 --- a/src/lua/fiber_cond.c +++ b/src/lua/fiber_cond.c @@ -95,8 +95,7 @@ luaT_fiber_cond_wait(struct lua_State *L) } } rc = fiber_cond_wait_timeout(e, timeout); - if (rc != 0) - luaL_testcancel(L); + luaL_testcancel(L); lua_pushboolean(L, rc == 0); return 1; } 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)) 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.24.3 (Apple Git-128)