Re: [tarantool-patches] Re: [PATCH 06/11] evio: make on_accept be nothrow

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

On 03/12/2018 17:58, Vladimir Davydov wrote:
> 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.

Yes, I do not close it since evio_service_accept_cb closes it now. Before
my patch the socket was closed inside iproto_on_accept because it was
exception free. Evio never closed a socket, accepted by iproto_on_accept.
Now a necessity to close a socket is determined by returned value instead of
an exception and I can not leave this close here.

> 
>> @@ -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?
> 

It is a separate bug that I found and fixed in a next commit. This patch
does not fix any bugs. It is pure refactoring.

In v2 the patch is left as is.