From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Mon, 3 Dec 2018 17:58:28 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH 06/11] evio: make on_accept be nothrow
Message-ID: <20181203145828.iholwtaejm2arnib@esperanza>
References: <cover.1543590433.git.v.shpilevoy@tarantool.org>
 <5aa67f292cac37f8b9d2f77b97221b81b1513195.1543590433.git.v.shpilevoy@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5aa67f292cac37f8b9d2f77b97221b81b1513195.1543590433.git.v.shpilevoy@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Fri, Nov 30, 2018 at 06:39:38PM +0300, Vladislav Shpilevoy wrote:
> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index 07ef23cac..dd76e28bd 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -1800,26 +1800,23 @@ iproto_on_accept(struct evio_service * /* service */, int fd,
>  	struct iproto_msg *msg;
>  	struct iproto_connection *con = iproto_connection_new(fd);
>  	if (con == NULL)
> -		goto error_conn;
> +		return -1;
>  	/*
>  	 * Ignore msg allocation failure - the queue size is
>  	 * fixed so there is a limited number of msgs in
>  	 * use, all stored in just a few blocks of the memory pool.
>  	 */
>  	msg = iproto_msg_new(con);
> -	if (msg == NULL)
> -		goto error_msg;
> +	if (msg == NULL) {
> +		mempool_free(&iproto_connection_pool, con);
> +		return -1;
> +	}
>  	cmsg_init(&msg->base, connect_route);
>  	msg->p_ibuf = con->p_ibuf;
>  	msg->wpos = con->wpos;
>  	msg->close_connection = false;
>  	cpipe_push(&tx_pipe, &msg->base);
> -	return;
> -error_msg:
> -	mempool_free(&iproto_connection_pool, con);
> -error_conn:
> -	close(fd);
> -	return;
> +	return 0;

You don't close the file descriptor on error anymore. I guess it's OK,
because evio_service_accept_cb will close it anyway.

> @@ -612,14 +612,12 @@ coio_service_on_accept(struct evio_service *evio_service,
>  		 "%s/%s", evio_service->name, sio_strfaddr(addr, addrlen));
>  
>  	/* Create the worker fiber. */
> -	struct fiber *f;
> -	try {
> -		f = fiber_new_xc(fiber_name, service->handler);
> -	} catch (struct error *e) {
> -		error_log(e);
> +	struct fiber *f = fiber_new(fiber_name, service->handler);
> +	if (f == NULL) {
> +		diag_log();
>  		say_error("can't create a handler fiber, dropping client connection");
>  		evio_close(loop(), &coio);

However, you do close fd here, via evio_close(). Care to remove it?