[Tarantool-patches] [PATCH v2] core: handle fiber cancellation for fiber.cond
sergos at tarantool.org
sergos at tarantool.org
Sat Oct 31 19:29:11 MSK 2020
From: Sergey Ostanevich <sergos at tarantool.org>
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
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 <olegrok at tarantool.org>
@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/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..bfa1051f9 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..b0645069e 100644
--- a/src/lib/core/fiber_cond.c
+++ b/src/lib/core/fiber_cond.c
@@ -108,6 +108,7 @@ fiber_cond_wait_timeout(struct fiber_cond *c, double timeout)
diag_set(TimedOut);
return -1;
}
+ if (fiber_is_cancelled()) 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))
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.17.1
More information about the Tarantool-patches
mailing list