From: Serge Petrenko <sergepetrenko@tarantool.org> To: vdavydov.dev@gmail.com Cc: tarantool-patches@freelists.org, Serge Petrenko <sergepetrenko@tarantool.org> Subject: [PATCH v2] Fix fiber_join() hang in case fiber_cancel() was called Date: Tue, 5 Feb 2019 18:01:11 +0300 [thread overview] Message-ID: <20190205150111.63224-1-sergepetrenko@tarantool.org> (raw) In case a fiber joining another fiber gets cancelled, it stays suspended forever and never finishes joining. This happens because fiber_cancel() wakes the fiber and removes it from all execution queues. Fix this by handling possible cancellation in fiber_join(). Closes #3948 --- Changes in v2: - rewrote the test completely. - instead of continuing to join if the fiber is cancelled make the fiber to be joined non-joinable and exit. This solution was discussed verbally. - revert comment changes for fiber_yield(). It really isn't a cancellation point. src/fiber.c | 25 ++++++++++++++++--------- src/lua/fiber.c | 11 ++++++++++- test/app/fiber.result | 33 +++++++++++++++++++++++++++++++++ test/app/fiber.test.lua | 17 +++++++++++++++++ 4 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/fiber.c b/src/fiber.c index 6f3d0ab78..7e9a3d38d 100644 --- a/src/fiber.c +++ b/src/fiber.c @@ -396,18 +396,25 @@ fiber_join(struct fiber *fiber) do { fiber_yield(); + if (fiber_is_cancelled()) + break; } while (! fiber_is_dead(fiber)); } - - /* Move exception to the caller */ - int ret = fiber->f_ret; - if (ret != 0) { - assert(!diag_is_empty(&fiber->diag)); - diag_move(&fiber->diag, &fiber()->diag); + if (! fiber_is_dead(fiber)) { + fiber_set_joinable(fiber, false); + diag_set(FiberIsCancelled); + return -1; + } else { + /* Move exception to the caller */ + int ret = fiber->f_ret; + if (ret != 0) { + assert(!diag_is_empty(&fiber->diag)); + diag_move(&fiber->diag, &fiber()->diag); + } + /* The fiber is already dead. */ + fiber_recycle(fiber); + return ret; } - /* The fiber is already dead. */ - fiber_recycle(fiber); - return ret; } /** diff --git a/src/lua/fiber.c b/src/lua/fiber.c index 8b17d6475..c655c5258 100644 --- a/src/lua/fiber.c +++ b/src/lua/fiber.c @@ -33,6 +33,7 @@ #include <fiber.h> #include "lua/utils.h" #include "backtrace.h" +#include "exception.h" #include <lua.h> #include <lauxlib.h> @@ -676,10 +677,18 @@ lbox_fiber_join(struct lua_State *L) { struct fiber *fiber = lbox_checkfiber(L, 1); struct lua_State *child_L = fiber->storage.lua.stack; - fiber_join(fiber); + int f_ret = fiber_join(fiber); struct error *e = NULL; int num_ret = 0; int coro_ref = 0; + + if (f_ret != 0 && diag_last_error(&fiber()->diag)->type == + &type_FiberIsCancelled) { + /* The fiber was cancelled before we joined. */ + luaL_testcancel(L); + } + luaL_testcancel(L); + if (child_L != NULL) { coro_ref = 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 = nil l1 = 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 = fiber.create(another_func) + fib:set_joinable(true) + fib:join() +end; +--- +... +f = fiber.create(func) +f:cancel() +while f:status() ~= 'dead' do fiber.sleep(0.01) end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +f = nil +--- +... +func = nil +--- +... +another_func = nil +--- +... -- cleanup test_run:cmd("clear filter") --- diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua index 2762047e4..98bb8f3e1 100644 --- a/test/app/fiber.test.lua +++ b/test/app/fiber.test.lua @@ -602,6 +602,23 @@ f = nil l = nil l1 = nil +-- gh-3948 fiber.join() blocks if fiber is cancelled. +function another_func() fiber.yield() end +test_run:cmd("setopt delimiter ';'") +function func() + local fib = fiber.create(another_func) + fib:set_joinable(true) + fib:join() +end; +f = fiber.create(func) +f:cancel() +while f:status() ~= 'dead' do fiber.sleep(0.01) end; + +test_run:cmd("setopt delimiter ''"); +f = nil +func = nil +another_func = nil + -- cleanup test_run:cmd("clear filter") -- 2.17.2 (Apple Git-113)
next reply other threads:[~2019-02-05 15:01 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-05 15:01 Serge Petrenko [this message] 2019-02-06 9:55 ` Vladimir Davydov 2019-02-06 12:58 ` [tarantool-patches] " Serge Petrenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190205150111.63224-1-sergepetrenko@tarantool.org \ --to=sergepetrenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH v2] Fix fiber_join() hang in case fiber_cancel() was called' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox