Re: [Tarantool-patches] [PATCH 2/2] fiber: fiber_join -- don't crash on misuse

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index baf78a130..dd7498dd7 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -615,7 +615,10 @@ fiber_join(struct fiber *fiber)
>  int
>  fiber_join_timeout(struct fiber *fiber, double timeout)
>  {
> -	assert(fiber->flags & FIBER_IS_JOINABLE);
> +	if (!(fiber->flags & FIBER_IS_JOINABLE)) {

1. https://github.com/tarantool/tarantool/wiki/Code-review-procedure#code-style

	In C we don't apply ! operator to non-boolean values. It means, to
	check if an integer is not 0, you use != 0. To check if a pointer is
	not NULL, you use != NULL. The same for ==;

> +		diag_set(IllegalParams, "the fiber is not joinable");
> +		return -1;
> +	}
>  
>  	if (! fiber_is_dead(fiber)) {
>  		bool exceeded = false;
> diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
> index 9c1a23bdd..fbdd82772 100644
> --- a/test/unit/fiber.cc
> +++ b/test/unit/fiber.cc
> @@ -96,6 +96,9 @@ fiber_join_test()
>  	header();
>  
>  	struct fiber *fiber = fiber_new_xc("join", noop_f);
> +	/* gh-6046: crash on attempt to join non joinable */
> +	fiber_set_joinable(fiber, false);
> +	fiber_join(fiber);

2. Would be good to test that it returns -1, and that the diag is not
empty.