[tarantool-patches] Re: [PATCH 08/11] coio: fix double close of a file descriptor
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Dec 5 00:29:26 MSK 2018
On 03/12/2018 19:19, Vladimir Davydov wrote:
> On Fri, Nov 30, 2018 at 06:39:40PM +0300, Vladislav Shpilevoy wrote:
>> coio_service_on_accept is called by evio by an
>> on_accept pointer. If evio obtains not zero from
>> on_accept pointer, it closes accepted socket. But
>> coio_service_on_accept closes it too, when fiber_new
>> fails. It is double close.
>>
>> Note that the bug existed even when on_accept was able
>> to throw.
>> ---
>> src/coio.cc | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/coio.cc b/src/coio.cc
>> index 575bae712..a888a54dd 100644
>> --- a/src/coio.cc
>> +++ b/src/coio.cc
>> @@ -605,9 +605,6 @@ coio_service_on_accept(struct evio_service *evio_service,
>> {
>> struct coio_service *service = (struct coio_service *)
>> evio_service->on_accept_param;
>> - struct ev_io coio;
>> -
>> - coio_create(&coio, fd);
>>
>> /* Set connection name. */
>> char fiber_name[SERVICE_NAME_MAXLEN];
>> @@ -619,9 +616,10 @@ coio_service_on_accept(struct evio_service *evio_service,
>> if (f == NULL) {
>> diag_log();
>> say_error("can't create a handler fiber, dropping client connection");
>> - evio_close(loop(), &coio);
>> return -1;
>> }
>> + struct ev_io coio;
>> + coio_create(&coio, fd);
>> /*
>> * The coio is passed into the created fiber, reset the
>> * libev callback param to point at it.
>
> I see now that you remove an extra close() from coio_service_on_accept
> in a separate patch. This is OK, but then please remove an extra close()
> from iproto_on_accept in the same patch, not in patch 6, where you make
> on_accept exception free.
>
If I left close in iproto_on_accept in the patch making on_accept
nothrow, I would introduce a new bug - double close in iproto_on_accept.
So I can not do it. Even temporary. Patch about nothrow on_accept is
refactoring, this patch is bugfix.
In v2 the patch is left as is.
More information about the Tarantool-patches
mailing list