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

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

Thanks for the answers!

On 11/03/2020 11:43, Timur Safin wrote:
> : -----Original Message-----
> : From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> : Subject: Re: [PATCH] Work-around WSL assert when SO_LINGER is set on unix
> : sockets
> : 
> 
> 
> : 
> : 1. Please, add a subsystem prefix to the commit title. For
> : examples see other commits in the repository.
> 
> Here I need a help, because evidence is not apparent. The last commit to 
> evio.c was not marked with subsystem 
> 
> 	tsafin@M1BOOK6319:~/tarantool$ git log --oneline src/lib/core/evio.c
> 	835d22aa0 (HEAD -> tsafin/gh-4659-wsl-no-linger-assert, master)
> 		Work-around WSL assert when SO_LINGER is set on unix sockets
> 	3f5f59bb5 Move 'core' and 'uuid' libs to src/lib
> 
> Should it be `core` or `coio`?

I think neither of them. It should be 'evio'.

> : > Branch: https://github.com/tarantool/tarantool/tree/tsafin/gh-4659-wsl-
> : no-linger-assert
> : 
> : 2. Please, provide both branch and issue links.
> : 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".
> 
> This is the question - what is the current practice for such 
> simplistic patches? SOP says there is no need to open issue 
> so I'm marking branch with github issue # of which this 
> patch is byproduct. 

I am not sure I understand what is 'patch byproduct'.

If there is no issue, then you can omit issue link. But it certainly
is not right to take some random other issue number into the branch
name. Just omit the issue number everywhere.

    tsafin/wsl-no-linger-assert

instead of

    tsafin/gh-4659-wsl-no-linger-assert

> : > -	/* 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?
> :
> 
> Good question! Never had enough time and motivation to proof this assumption.
> Now you've asked I'll look into linux kernel tcp implementation. Stay ktuned.

The question wasn't really about kernel sources. Rather about
documentation or standards, on which you could rely, when assume,
that SO_LINGER works for some socket types, and does not for
others.

I don't think we can omit SO_LINGER in case it is not guaranteed
that it is no-op for unix sockets.

However, we can omit it for WSL specifically, like Cyrill proposed.
Using cmake to add a new macro, and do nothing when macro says we
compile for WSL.

> : I would rather call sio_setsockopt() always. And ignore an
> : error, if it is EINVAL for AF_UNIX.
> 
> Probably it's less damaging approach. I'll probably use it - but first I need to
> Look around in the kernel.