From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A706F469719 for ; Wed, 11 Mar 2020 13:43:17 +0300 (MSK) From: "Timur Safin" References: <045901d5f6e7$bc2d47a0$3487d6e0$@tarantool.org> <1e1a11dc-cce3-603d-69f0-3dabf371d59a@tarantool.org> In-Reply-To: <1e1a11dc-cce3-603d-69f0-3dabf371d59a@tarantool.org> Date: Wed, 11 Mar 2020 13:43:16 +0300 Message-ID: <06c001d5f791$df13e800$9d3bb800$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: ru Subject: Re: [Tarantool-patches] [PATCH] Work-around WSL assert when SO_LINGER is set on unix sockets List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Vladislav Shpilevoy' , tarantool-patches@dev.tarantool.org, 'Kirill Yukhin' : -----Original Message----- : From: Vladislav Shpilevoy : Subject: Re: [PATCH] Work-around WSL assert when SO_LINGER is set on = unix : sockets :=20 :=20 : 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=20 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`? : > Branch: = https://github.com/tarantool/tarantool/tree/tsafin/gh-4659-wsl- : no-linger-assert :=20 : 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=20 simplistic patches? SOP says there is no need to open issue=20 so I'm marking branch with github issue # of which this=20 patch is byproduct.=20 :: 3. I see, that on the branch your commit message is just empty. : Seems like you didn't push the latest message. Yup, I didn't fully realize that such single-patch patchsets=20 should contain patch commit messages and not that separate messages=20 which I'd put to the cover message of patchset. Will amend. =20 :=20 : > --- :=20 : 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. Yes, thanks for the correction, this is side-effect of an approach used above. =20 : > - /* Send all buffered messages on socket before take : > - * control out from close(2) or shutdown(2). */ : > - struct linger linger =3D { 0, 0 }; : > + if (family !=3D AF_UNIX) { :=20 : 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. =20 =20 : 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. :=20 : 7. It is worth adding a comment why SO_LINGER is workarounded : somehow. Indeed. Will do . :=20 : > + /* Send all buffered messages on socket before : > + * take control out from close(2) or shutdown(2). */ : > + struct linger linger =3D { 0, 0 }; : > + : > + if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER, : > + &linger, sizeof(linger))) : > + return -1; : > + } Timur