Re: [Tarantool-patches] [PATCH] Work-around WSL assert when SO_LINGER is set on unix sockets

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

Hi! Thanks for the patch!

See 7 comments below.

1. Please, add a subsystem prefix to the commit title. For
examples see other commits in the repository.

On 10/03/2020 15:25, Timur Safin wrote:
> Using SO_LINGER over unix sockets makes no much sense, though
> it's harmless on Linux. The problem is, it breaks majority of 
> tests under Windows/WSL with assertion, because setsockopt()
> would return EINVAL in that case under WSL.> 
> This is known WSL issue and reported here 
> https://github.com/microsoft/WSL/issues/3992
> 
> So we filter out SO_LINGER if evio_setsockopt_server is being
> called with AF_UNIX family.
>  
> Branch: https://github.com/tarantool/tarantool/tree/tsafin/gh-4659-wsl-no-linger-assert

2. Please, provide both branch and issue links.

3. I see, that on the branch your commit message is just empty.
Seems like you didn't push the latest message.

4. The patch has nothing to do with gh-4659:
https://github.com/tarantool/tarantool/issues/4659
"sql: raise an error in case space features HASH index".

> ---

5. Links should be below this marker '---'. Some people apply
patches from emails, and when you write links above '---', you
make them part of the commit message.

>  src/lib/core/evio.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/src/lib/core/evio.c b/src/lib/core/evio.c
> index 2152c15e6..06aa11ce4 100644
> --- a/src/lib/core/evio.c
> +++ b/src/lib/core/evio.c
> @@ -140,13 +140,16 @@ evio_setsockopt_server(int fd, int family, int type)
>  		       &on, sizeof(on)))
>  		return -1;
>  
> -	/* Send all buffered messages on socket before take
> -	 * control out from close(2) or shutdown(2). */
> -	struct linger linger = { 0, 0 };
> +	if (family != AF_UNIX) {

6. Is there any proof that it is no-op on Linux for AF_UNIX?

I would rather call sio_setsockopt() always. And ignore an
error, if it is EINVAL for AF_UNIX.

7. It is worth adding a comment why SO_LINGER is workarounded
somehow.

> +		/* Send all buffered messages on socket before
> +		 * take control out from close(2) or shutdown(2). */
> +		struct linger linger = { 0, 0 };
> +
> +		if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER,
> +				&linger, sizeof(linger)))
> +			return -1;
> +	}
>  
> -	if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER,
> -		       &linger, sizeof(linger)))
> -		return -1;
>  	if (type == SOCK_STREAM && family != AF_UNIX &&
>  	    evio_setsockopt_keepalive(fd) != 0)
>  		return -1;
>