[tarantool-patches] Re: [PATCH] CBUS: fix cbus_unpair behavior

[Vladislav Shpilevoy]

Hi! Thanks for the patch! See 3 comments below.

1. We don't normally use uppercase in commit titles. Please, use 'cbus',
not 'CBUS'.

On 10/09/2019 16:17, Georgy Kirichenko wrote:
> Each side should close outgoing cpipe while cbus unpair processing.

2. Why? I didn't understand it. Please, explain in more details.

As I see in the code, cbus_pair() does cpipe_create(dest_pipe) in
the caller thread, and cbus_pair_perform() does cpipe_create(msg->src_pipe)
in the target thread.

On the other hand cbus_unpair() does cpipe_destroy(dest_pipe) and
cbus_unpair_perform() does cpipe_destroy(msg->src_pipe). Looks
symmetrical. What was wrong here?

> 
> Fixes: #4484
> ---
> Branch: https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-4484-cbus-unpair-cpipe-destroy
> Issue: https://github.com/tarantool/tarantool/issues/4484
>  src/lib/core/cbus.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lib/core/cbus.c b/src/lib/core/cbus.c
> index b3b1280e7..78f3e5341 100644
> --- a/src/lib/core/cbus.c
> +++ b/src/lib/core/cbus.c
> @@ -547,6 +547,7 @@ struct cbus_unpair_msg {
>  	void (*unpair_cb)(void *);
>  	void *unpair_arg;
>  	struct cpipe *src_pipe;
> +	struct cpipe *dest_pipe;

3. Please, use 'dst'. When we have 'src', for
destination we conventionally use 'dst'.

>  	bool complete;
>  	struct fiber_cond cond;
>  };