[tarantool-patches] Re: [PATCH] Fix fiber_join() hang in case fiber_cancel() was called

[Vladimir Davydov]

On Tue, Feb 05, 2019 at 09:28:01AM +0300, Serge Petrenko wrote:
> 
> 
> > 4 февр. 2019 г., в 20:44, Vladimir Davydov <vdavydov.dev at gmail.com> написал(а):
> > 
> >> 
> >> diff --git a/src/fiber.c b/src/fiber.c
> >> index 6f3d0ab78..4dc6b3c5a 100644
> >> --- a/src/fiber.c
> >> +++ b/src/fiber.c
> >> @@ -392,9 +392,17 @@ fiber_join(struct fiber *fiber)
> >> 	assert(fiber->flags & FIBER_IS_JOINABLE);
> >> 
> >> 	if (! fiber_is_dead(fiber)) {
> >> -		rlist_add_tail_entry(&fiber->wake, fiber(), state);
> >> 
> >> 		do {
> >> +			/*
> >> +			 * In case fiber is cancelled during yield
> >> +			 * it will be removed from wake queue by a
> >> +			 * wakeup following the cancel.
> >> +			 * Having multiple entries for the same fiber
> >> +			 * doesn't hurt, since wakeup is executed only
> >> +			 * once per fiber.
> >> +			 */
> >> +			rlist_add_tail_entry(&fiber->wake, fiber(), state);
> > 
> > I don't quite like the idea that cancelling a fiber that is joining
> > another fiber will have no effect until the other fiber has exited.
> > Can't we break the loop if fiber_is_cancelled()?
> 
> We can do that. But then we have to set FIBER_IS_JOINABLE to false
> for the joined fiber so that it executes fiber_recycle().

Why should we? IMO the user should be free to kill a fiber executing
fiber_join. If that happens, the joinable fiber shouldn't be collected
until another fiber joins it successfully. This would be consistent with
pthread_join behavior.

> Otherwise it will leak.  Is it ok?