Re: [Tarantool-patches] [PATCH v7 3/3] iproto: move greeting from tx thread to iproto

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

Did you even test it?

I used exactly the same test as in my last email and I still get
"too many open files".

See 2 comments below.

> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index f7330af21d..b48a774c92 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -1484,8 +1544,16 @@ static inline struct iproto_msg *
>  tx_accept_msg(struct cmsg *m)
>  {
>  	struct iproto_msg *msg = (struct iproto_msg *) m;
> -	tx_accept_wpos(msg->connection, &msg->wpos);
> -	tx_fiber_init(msg->connection->session, msg->header.sync);
> +	struct iproto_connection *con = msg->connection;
> +	if (con->state != IPROTO_CONNECTION_ALIVE) {

1. Connection state can only be changed and read by iproto thread.
The variable is not protected anyhow, so you can't simply read/write
it in 2 threads.

> +		/*
> +		 * Connection might be closed from iproto already.
> +		 * No action required in this case.
> +		 */
> +		return msg;

2. Why do you return a message instead of NULL here, if the
connection is already dead?

I ask you to spend more time on finishing the patch. If you will
rush to "fix" it without thinking it through, the review will
never end, and it will waste time both yours and mine.