Re: [Tarantool-patches] [PATCH] socket: fix error while closing socket.tcp_server

[lvasiliev <lvasiliev@tarantool.org>]

Hi! Thank you for the review.

On 06.05.2020 18:38, Alexander Turenko wrote:
>> @@ -1082,6 +1085,9 @@ local function tcp_server_loop(server, s, addr)
>>       fiber.name(format("%s/%s:%s", server.name, addr.host, addr.port), {truncate = true})
>>       log.info("started")
>>       while socket_readable(s) do
>> +        if s._server_socket_closed then
>> +            break
>> +        end
> 
> Can we use the same check `<s>._gc_socket.fd >= 0` as in check_socket()
> (we can wrap it to a function like socket_is_closed())?
> 
> It seems that using the same way for determining whether a socket is
> closed owuld simplify the code a bit.
> 
> WBR, Alexander Turenko.
> 
If I understand correctly, in this case we must not only check whether
the socket is closed or not, but also check whether it was closed
without errors by checking errno. In fact, we come to my first patchset,
where I disable throwing an exception from the check_socket(). Disabling
throwing an exception is the right way because now our API doesn't
correspond to the documentation (I will create a ticket for this). But
fixing a socket check leads to big changes in the code that are not
related to the problem and will not pass the review. I can wrap the flag
check in a method, but then all sockets should have a flag and it's look
like overkill (this will duplicate the functionality of the
check_socket() function after fix).
In addition, we don't check EV_ERROR and it seems to me that this can
also lead to the fact that the check you proposed may be insufficient.