[tarantool-patches] Re: [PATCH 06/11] evio: make on_accept be nothrow
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Dec 5 00:29:14 MSK 2018
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.
More information about the Tarantool-patches
mailing list