Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
       [not found] <bb3eef527b48bdd3aacddc27e0597ceedcb84987.1584371177.git.tsafin@tarantool.org>
@ 2020-03-16 16:53 ` Timur Safin
  2020-03-16 22:35   ` Vladislav Shpilevoy
  2020-03-19 10:27 ` [Tarantool-patches] [PATCH v1.2] " Timur Safin
  1 sibling, 1 reply; 10+ messages in thread
From: Timur Safin @ 2020-03-16 16:53 UTC (permalink / raw)
  To: Cyrill Gorcunov, Vladislav Shpilevoy; +Cc: tarantool-patches

SO_LINGER makes no much sense for unix-sockets, and Microsoft WSL
is returning EINVAL if setsockopts called for SO_LINGER over unix
sockets:

  [004] 2020-03-11 18:42:29.592 [29182] main/102/app sio.c:169 !> SystemError setsockopt(SO_LINGER), called on fd 16, aka
  [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,
  [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,

And it's sort of correct here, but the problem is Linux is simply
silently ignoring it, which passes tests.

After much debates we decided to work-around this case via CMAKE
define.

NB! In a future (April/May 2020), when WSL2 with full Linux kernel
would be released we should disable this check.

Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 cmake/os.cmake      | 13 +++++++++++++
 src/lib/core/evio.c |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/cmake/os.cmake b/cmake/os.cmake
index 0ed554b9b..c6b19f379 100644
--- a/cmake/os.cmake
+++ b/cmake/os.cmake
@@ -11,6 +11,15 @@ if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
 # (see man page for feature_test_macros).
     add_definitions("-D_FILE_OFFSET_BITS=64")
     find_package_message(PLATFORM "Building for Linux" "${CMAKE_SYSTEM_NAME}")
+
+    # There are some subtle differences in Linux kernel calls
+    # implementation under WSL1 (which should go away with WSL2 kernel)
+    # so for a moment we introduce a way to distinguish Linux and
+    # Microsoft/WSL1
+    if (${CMAKE_SYSTEM} MATCHES "Linux-.*-Microsoft")
+        add_definitions("-DTARANTOOL_WSL1_WORKAROUND_ENABLED=1")
+    endif()
+
 elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
     set(TARGET_OS_FREEBSD 1)
     set(TARGET_OS_DEBIAN_FREEBSD 1)
@@ -19,6 +28,7 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
     add_definitions("-D_FILE_OFFSET_BITS=64")
     find_package_message(PLATFORM "Building for Debian/kFreeBSD"
         "${CMAKE_SYSTEM_NAME}")
+
 elseif (${CMAKE_SYSTEM_NAME} STREQUAL "FreeBSD")
     set(TARGET_OS_FREEBSD 1)
     find_package_message(PLATFORM "Building for FreeBSD" "${CMAKE_SYSTEM_NAME}")
@@ -57,9 +67,11 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "FreeBSD")
                         "system libraries is not supported")
     endif()
     unset(REAL_OPENSSL_ROOT_DIR)
+
 elseif (${CMAKE_SYSTEM_NAME} STREQUAL "NetBSD")
     set(TARGET_OS_NETBSD 1)
     find_package_message(PLATFORM "Building for NetBSD" "${CMAKE_SYSTEM_NAME}")
+
 elseif (${CMAKE_SYSTEM_NAME} STREQUAL "Darwin")
     set(TARGET_OS_DARWIN 1)
 
@@ -154,6 +166,7 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "Darwin")
             endif()
         endif()
     endif()
+
 else()
     message (FATAL_ERROR "Unsupported platform -- ${CMAKE_SYSTEM_NAME}")
 endif()
diff --git a/src/lib/core/evio.c b/src/lib/core/evio.c
index 2152c15e6..e1eab44d1 100644
--- a/src/lib/core/evio.c
+++ b/src/lib/core/evio.c
@@ -140,6 +140,7 @@ evio_setsockopt_server(int fd, int family, int type)
 		       &on, sizeof(on)))
 		return -1;
 
+#ifndef TARANTOOL_WSL1_WORKAROUND_ENABLED
 	/* Send all buffered messages on socket before take
 	 * control out from close(2) or shutdown(2). */
 	struct linger linger = { 0, 0 };
@@ -147,6 +148,7 @@ evio_setsockopt_server(int fd, int family, int type)
 	if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER,
 		       &linger, sizeof(linger)))
 		return -1;
+#endif
 	if (type == SOCK_STREAM && family != AF_UNIX &&
 	    evio_setsockopt_keepalive(fd) != 0)
 		return -1;
-- 
2.20.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
  2020-03-16 16:53 ` [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion Timur Safin
@ 2020-03-16 22:35   ` Vladislav Shpilevoy
  2020-03-17 14:40     ` Timur Safin
  0 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-16 22:35 UTC (permalink / raw)
  To: Timur Safin, Cyrill Gorcunov; +Cc: tarantool-patches

Hi! Thanks for the patch!

Please, keep branch link in each new email thread.

On 16/03/2020 17:53, Timur Safin wrote:
> SO_LINGER makes no much sense for unix-sockets, and Microsoft WSL
> is returning EINVAL if setsockopts called for SO_LINGER over unix
> sockets:
> 
>   [004] 2020-03-11 18:42:29.592 [29182] main/102/app sio.c:169 !> SystemError setsockopt(SO_LINGER), called on fd 16, aka
>   [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,
>   [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,
> 
> And it's sort of correct here, but the problem is Linux is simply
> silently ignoring it, which passes tests.
> 
> After much debates we decided to work-around this case via CMAKE
> define.
> 
> NB! In a future (April/May 2020), when WSL2 with full Linux kernel
> would be released we should disable this check.
> 
> Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  cmake/os.cmake      | 13 +++++++++++++
>  src/lib/core/evio.c |  2 ++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/cmake/os.cmake b/cmake/os.cmake
> index 0ed554b9b..c6b19f379 100644
> --- a/cmake/os.cmake
> +++ b/cmake/os.cmake
> @@ -11,6 +11,15 @@ if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
>  # (see man page for feature_test_macros).
>      add_definitions("-D_FILE_OFFSET_BITS=64")
>      find_package_message(PLATFORM "Building for Linux" "${CMAKE_SYSTEM_NAME}")
> +
> +    # There are some subtle differences in Linux kernel calls
> +    # implementation under WSL1 (which should go away with WSL2 kernel)
> +    # so for a moment we introduce a way to distinguish Linux and
> +    # Microsoft/WSL1
> +    if (${CMAKE_SYSTEM} MATCHES "Linux-.*-Microsoft")
> +        add_definitions("-DTARANTOOL_WSL1_WORKAROUND_ENABLED=1")
> +    endif()

To be more consistent I would better call it TARGET_OS_WSL1.
Since we already have TARGET_OS_LINUX, TARGET_OS_FREEBSD,
TARGET_OS_DEBIAN_FREEBSD, TARGET_OS_NETBSD, TARGET_OS_DARWIN.

Besides, it would be consistent with other similar examples
such as fio_filename(), load_cfg(), make_pipe(), and so on. They
use TARGET_OS_* name.

> +
>  elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
>      set(TARGET_OS_FREEBSD 1)
>      set(TARGET_OS_DEBIAN_FREEBSD 1)
> @@ -19,6 +28,7 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
>      add_definitions("-D_FILE_OFFSET_BITS=64")
>      find_package_message(PLATFORM "Building for Debian/kFreeBSD"
>          "${CMAKE_SYSTEM_NAME}")
> +

Please, omit not necessary diff. This and other new empty lines
below.

>  elseif (${CMAKE_SYSTEM_NAME} STREQUAL "FreeBSD")
>      set(TARGET_OS_FREEBSD 1)
>      find_package_message(PLATFORM "Building for FreeBSD" "${CMAKE_SYSTEM_NAME}")
> @@ -57,9 +67,11 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "FreeBSD")
>                          "system libraries is not supported")
>      endif()
>      unset(REAL_OPENSSL_ROOT_DIR)
> +
>  elseif (${CMAKE_SYSTEM_NAME} STREQUAL "NetBSD")
>      set(TARGET_OS_NETBSD 1)
>      find_package_message(PLATFORM "Building for NetBSD" "${CMAKE_SYSTEM_NAME}")
> +
>  elseif (${CMAKE_SYSTEM_NAME} STREQUAL "Darwin")
>      set(TARGET_OS_DARWIN 1)
>  
> @@ -154,6 +166,7 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "Darwin")
>              endif()
>          endif()
>      endif()
> +
>  else()
>      message (FATAL_ERROR "Unsupported platform -- ${CMAKE_SYSTEM_NAME}")
>  endif()

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
  2020-03-16 22:35   ` Vladislav Shpilevoy
@ 2020-03-17 14:40     ` Timur Safin
  2020-03-17 21:27       ` Vladislav Shpilevoy
  0 siblings, 1 reply; 10+ messages in thread
From: Timur Safin @ 2020-03-17 14:40 UTC (permalink / raw)
  To: 'Vladislav Shpilevoy', 'Cyrill Gorcunov'
  Cc: tarantool-patches

Hi there!

: -----Original Message-----
: From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
: Subject: Re: [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
: 
: Hi! Thanks for the patch!
: 
: Please, keep branch link in each new email thread.

Yup, sorry for the omission, I should automate this somehow 
(because at the moment there are too many manual steps which are 
easy to forget or to skip. Do you have any advices/scripts here?)

: > @@ -11,6 +11,15 @@ if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
: >  # (see man page for feature_test_macros).
: >      add_definitions("-D_FILE_OFFSET_BITS=64")
: >      find_package_message(PLATFORM "Building for Linux"
: "${CMAKE_SYSTEM_NAME}")
: > +
: > +    # There are some subtle differences in Linux kernel calls
: > +    # implementation under WSL1 (which should go away with WSL2 kernel)
: > +    # so for a moment we introduce a way to distinguish Linux and
: > +    # Microsoft/WSL1
: > +    if (${CMAKE_SYSTEM} MATCHES "Linux-.*-Microsoft")
: > +        add_definitions("-DTARANTOOL_WSL1_WORKAROUND_ENABLED=1")
: > +    endif()
: 
: To be more consistent I would better call it TARGET_OS_WSL1.
: Since we already have TARGET_OS_LINUX, TARGET_OS_FREEBSD,
: TARGET_OS_DEBIAN_FREEBSD, TARGET_OS_NETBSD, TARGET_OS_DARWIN.
: 

I did consider introducing the special target, but after some 
evaluation I've realized that this is not a new target, but rather 
Linux flavor (with the same abi, binutils, glibc implementation)
but slightly different syscalls implementation. Which difference 
after WSL2 release would go away the end of Spring 2020.


: Besides, it would be consistent with other similar examples
: such as fio_filename(), load_cfg(), make_pipe(), and so on. They
: use TARGET_OS_* name.

And this example of fio_filename() implementation is exactly 
and indication why WSL is still the same Linux - it is requiring the same
exactly TARGET_OS_LINUX defined to be correctly implemented under WSL.


: 
: > +
: >  elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
: >      set(TARGET_OS_FREEBSD 1)
: >      set(TARGET_OS_DEBIAN_FREEBSD 1)
: > @@ -19,6 +28,7 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
: >      add_definitions("-D_FILE_OFFSET_BITS=64")
: >      find_package_message(PLATFORM "Building for Debian/kFreeBSD"
: >          "${CMAKE_SYSTEM_NAME}")
: > +
: 
: Please, omit not necessary diff. This and other new empty lines
: below.
:

Ok, will remove these unnecessary lines added (though they did 
add some extra readability to the cmake scripts we have here :) ) 
 
Regards,
Timur

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
  2020-03-17 14:40     ` Timur Safin
@ 2020-03-17 21:27       ` Vladislav Shpilevoy
  2020-03-18  7:12         ` Timur Safin
  0 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-17 21:27 UTC (permalink / raw)
  To: Timur Safin, 'Cyrill Gorcunov'; +Cc: tarantool-patches

Hi!

> : -----Original Message-----
> : From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> : Subject: Re: [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
> : 
> : Hi! Thanks for the patch!
> : 
> : Please, keep branch link in each new email thread.
> 
> Yup, sorry for the omission, I should automate this somehow 
> (because at the moment there are too many manual steps which are 
> easy to forget or to skip. Do you have any advices/scripts here?)

Yeah, I use this:
https://gist.github.com/Gerold103/5471a7ddbeec346c0c845930d5bb9df4
But it is for bash. Don't know whether you use bash on Windows.

> : > @@ -11,6 +11,15 @@ if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
> : >  # (see man page for feature_test_macros).
> : >      add_definitions("-D_FILE_OFFSET_BITS=64")
> : >      find_package_message(PLATFORM "Building for Linux"
> : "${CMAKE_SYSTEM_NAME}")
> : > +
> : > +    # There are some subtle differences in Linux kernel calls
> : > +    # implementation under WSL1 (which should go away with WSL2 kernel)
> : > +    # so for a moment we introduce a way to distinguish Linux and
> : > +    # Microsoft/WSL1
> : > +    if (${CMAKE_SYSTEM} MATCHES "Linux-.*-Microsoft")
> : > +        add_definitions("-DTARANTOOL_WSL1_WORKAROUND_ENABLED=1")
> : > +    endif()
> : 
> : To be more consistent I would better call it TARGET_OS_WSL1.
> : Since we already have TARGET_OS_LINUX, TARGET_OS_FREEBSD,
> : TARGET_OS_DEBIAN_FREEBSD, TARGET_OS_NETBSD, TARGET_OS_DARWIN.
> : 
> 
> I did consider introducing the special target, but after some 
> evaluation I've realized that this is not a new target, but rather 
> Linux flavor (with the same abi, binutils, glibc implementation)
> but slightly different syscalls implementation. Which difference 
> after WSL2 release would go away the end of Spring 2020.
> 
> 
> : Besides, it would be consistent with other similar examples
> : such as fio_filename(), load_cfg(), make_pipe(), and so on. They
> : use TARGET_OS_* name.
> 
> And this example of fio_filename() implementation is exactly 
> and indication why WSL is still the same Linux - it is requiring the same
> exactly TARGET_OS_LINUX defined to be correctly implemented under WSL.

As you wish.

> : 
> : > +
> : >  elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
> : >      set(TARGET_OS_FREEBSD 1)
> : >      set(TARGET_OS_DEBIAN_FREEBSD 1)
> : > @@ -19,6 +28,7 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
> : >      add_definitions("-D_FILE_OFFSET_BITS=64")
> : >      find_package_message(PLATFORM "Building for Debian/kFreeBSD"
> : >          "${CMAKE_SYSTEM_NAME}")
> : > +
> : 
> : Please, omit not necessary diff. This and other new empty lines
> : below.
> :
> 
> Ok, will remove these unnecessary lines added (though they did 
> add some extra readability to the cmake scripts we have here :) ) 

Perhaps, but readability is a subjective thing usually. However
more important is that such unnecessary diff tends to pollute git
blame, and pad out patch size making it harder to review.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
  2020-03-17 21:27       ` Vladislav Shpilevoy
@ 2020-03-18  7:12         ` Timur Safin
  0 siblings, 0 replies; 10+ messages in thread
From: Timur Safin @ 2020-03-18  7:12 UTC (permalink / raw)
  To: 'Vladislav Shpilevoy', 'Cyrill Gorcunov'
  Cc: tarantool-patches



: -----Original Message-----
: From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
: Subject: Re: [PATCH v1.1] evio: workaround for wsl1 so_linger assertion
: 
: > Yup, sorry for the omission, I should automate this somehow
: > (because at the moment there are too many manual steps which are
: > easy to forget or to skip. Do you have any advices/scripts here?)
: 
: Yeah, I use this:
: https://gist.github.com/Gerold103/5471a7ddbeec346c0c845930d5bb9df4
: But it is for bash. Don't know whether you use bash on Windows.

That's quite nice. I'll try to automate it even further (i.e. there is no 
much need to provide manual branch information, and the number of patches
to pick - all them could be automatically calculated), but it's very good
start.  

: > :
: > : > +
: > : >  elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
: > : >      set(TARGET_OS_FREEBSD 1)
: > : >      set(TARGET_OS_DEBIAN_FREEBSD 1)
: > : > @@ -19,6 +28,7 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
: > : >      add_definitions("-D_FILE_OFFSET_BITS=64")
: > : >      find_package_message(PLATFORM "Building for Debian/kFreeBSD"
: > : >          "${CMAKE_SYSTEM_NAME}")
: > : > +
: > :
: > : Please, omit not necessary diff. This and other new empty lines
: > : below.
: > :
: >
: > Ok, will remove these unnecessary lines added (though they did
: > add some extra readability to the cmake scripts we have here :) )
: 
: Perhaps, but readability is a subjective thing usually. However
: more important is that such unnecessary diff tends to pollute git
: blame, and pad out patch size making it harder to review.

Ok, ok, will get rid of them in next iteration.

Regards,
Timur

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Tarantool-patches] [PATCH v1.2] evio: workaround for wsl1 so_linger assertion
       [not found] <bb3eef527b48bdd3aacddc27e0597ceedcb84987.1584371177.git.tsafin@tarantool.org>
  2020-03-16 16:53 ` [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion Timur Safin
@ 2020-03-19 10:27 ` Timur Safin
  2020-03-19 10:52   ` Cyrill Gorcunov
  2020-03-22 19:21   ` Vladislav Shpilevoy
  1 sibling, 2 replies; 10+ messages in thread
From: Timur Safin @ 2020-03-19 10:27 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches, Cyrill Gorcunov

SO_LINGER makes no much sense for unix-sockets, and Microsoft WSL
is returning EINVAL if setsockopts called for SO_LINGER over unix
sockets:

  [004] 2020-03-11 18:42:29.592 [29182] main/102/app sio.c:169 !> SystemError setsockopt(SO_LINGER), called on fd 16, aka
  [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,
  [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,

And it's sort of correct here, but the problem is Linux is simply
silently ignoring it, which passes tests.

After much debates we decided to work-around this case via CMAKE
define.

NB! In a future (April/May 2020), when WSL2 with full Linux kernel
would be released we should disable this check.

Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

This patch replaces prior one tsafin/gh-4659-wsl-no-linger-assert, and tsafin/wsl1-no-linger-assert
because original approach considered unsafe, and we rather want to workaround it via CMake instead.

Patch 1.2 - got rid of pure stylistic cmake changes. 


Branch: https://github.com/tarantool/tarantool/tree/tsafin/wsl1-no-linger-assert-2

 cmake/os.cmake      | 9 +++++++++
 src/lib/core/evio.c | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/cmake/os.cmake b/cmake/os.cmake
index 0ed554b9b..cf6eac10d 100644
--- a/cmake/os.cmake
+++ b/cmake/os.cmake
@@ -11,6 +11,15 @@ if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
 # (see man page for feature_test_macros).
     add_definitions("-D_FILE_OFFSET_BITS=64")
     find_package_message(PLATFORM "Building for Linux" "${CMAKE_SYSTEM_NAME}")
+
+    # There are some subtle differences in Linux kernel calls
+    # implementation under WSL1 (which should go away with WSL2 kernel)
+    # so for a moment we introduce a way to distinguish Linux and
+    # Microsoft/WSL1
+    if (${CMAKE_SYSTEM} MATCHES "Linux-.*-Microsoft")
+        add_definitions("-DTARANTOOL_WSL1_WORKAROUND_ENABLED=1")
+    endif()
+
 elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD")
     set(TARGET_OS_FREEBSD 1)
     set(TARGET_OS_DEBIAN_FREEBSD 1)
diff --git a/src/lib/core/evio.c b/src/lib/core/evio.c
index 2152c15e6..e1eab44d1 100644
--- a/src/lib/core/evio.c
+++ b/src/lib/core/evio.c
@@ -140,6 +140,7 @@ evio_setsockopt_server(int fd, int family, int type)
 		       &on, sizeof(on)))
 		return -1;
 
+#ifndef TARANTOOL_WSL1_WORKAROUND_ENABLED
 	/* Send all buffered messages on socket before take
 	 * control out from close(2) or shutdown(2). */
 	struct linger linger = { 0, 0 };
@@ -147,6 +148,7 @@ evio_setsockopt_server(int fd, int family, int type)
 	if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER,
 		       &linger, sizeof(linger)))
 		return -1;
+#endif
 	if (type == SOCK_STREAM && family != AF_UNIX &&
 	    evio_setsockopt_keepalive(fd) != 0)
 		return -1;
-- 
2.20.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.2] evio: workaround for wsl1 so_linger assertion
  2020-03-19 10:27 ` [Tarantool-patches] [PATCH v1.2] " Timur Safin
@ 2020-03-19 10:52   ` Cyrill Gorcunov
  2020-03-19 11:07     ` Timur Safin
  2020-03-22 19:21   ` Vladislav Shpilevoy
  1 sibling, 1 reply; 10+ messages in thread
From: Cyrill Gorcunov @ 2020-03-19 10:52 UTC (permalink / raw)
  To: Timur Safin; +Cc: tarantool-patches, Vladislav Shpilevoy, Cyrill Gorcunov

On Thu, Mar 19, 2020 at 01:27:08PM +0300, Timur Safin wrote:
> SO_LINGER makes no much sense for unix-sockets, and Microsoft WSL
> is returning EINVAL if setsockopts called for SO_LINGER over unix
> sockets:
> 
>   [004] 2020-03-11 18:42:29.592 [29182] main/102/app sio.c:169 !> SystemError setsockopt(SO_LINGER), called on fd 16, aka
>   [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,
>   [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize storage: setsockopt(SO_LINGER), called on fd 16,
> 
> And it's sort of correct here, but the problem is Linux is simply
> silently ignoring it, which passes tests.
> 
> After much debates we decided to work-around this case via CMAKE
> define.
> 
> NB! In a future (April/May 2020), when WSL2 with full Linux kernel
> would be released we should disable this check.

Hardly. This will break backward compatibility. Or we will have
to _require_ WSL2 to build tarantool.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.2] evio: workaround for wsl1 so_linger assertion
  2020-03-19 10:52   ` Cyrill Gorcunov
@ 2020-03-19 11:07     ` Timur Safin
  0 siblings, 0 replies; 10+ messages in thread
From: Timur Safin @ 2020-03-19 11:07 UTC (permalink / raw)
  To: 'Cyrill Gorcunov'
  Cc: tarantool-patches, 'Vladislav Shpilevoy',
	'Cyrill Gorcunov'



: -----Original Message-----
: From: Cyrill Gorcunov <gorcunov@gmail.com>
: Sent: Thursday, March 19, 2020 1:52 PM
: To: Timur Safin <tsafin@tarantool.org>
: Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>; tarantool-
: patches@dev.tarantool.org; Cyrill Gorcunov <gorcunov@tarantool.org>
: Subject: Re: [Tarantool-patches] [PATCH v1.2] evio: workaround for wsl1
: so_linger assertion
: 
: On Thu, Mar 19, 2020 at 01:27:08PM +0300, Timur Safin wrote:
: > SO_LINGER makes no much sense for unix-sockets, and Microsoft WSL
: > is returning EINVAL if setsockopts called for SO_LINGER over unix
: > sockets:
: >
: >   [004] 2020-03-11 18:42:29.592 [29182] main/102/app sio.c:169 !>
: SystemError setsockopt(SO_LINGER), called on fd 16, aka
: >   [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize
: storage: setsockopt(SO_LINGER), called on fd 16,
: >   [004] 2020-03-11 18:42:29.592 [29182] main/102/app F> can't initialize
: storage: setsockopt(SO_LINGER), called on fd 16,
: >
: > And it's sort of correct here, but the problem is Linux is simply
: > silently ignoring it, which passes tests.
: >
: > After much debates we decided to work-around this case via CMAKE
: > define.
: >
: > NB! In a future (April/May 2020), when WSL2 with full Linux kernel
: > would be released we should disable this check.
: 
: Hardly. This will break backward compatibility. Or we will have
: to _require_ WSL2 to build tarantool.

I'll find the proper way to distinguish WSL1/WSL2 by then. No problem.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.2] evio: workaround for wsl1 so_linger assertion
  2020-03-19 10:27 ` [Tarantool-patches] [PATCH v1.2] " Timur Safin
  2020-03-19 10:52   ` Cyrill Gorcunov
@ 2020-03-22 19:21   ` Vladislav Shpilevoy
  2020-03-23 23:12     ` Vladislav Shpilevoy
  1 sibling, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-22 19:21 UTC (permalink / raw)
  To: Timur Safin; +Cc: tarantool-patches, Cyrill Gorcunov

Hi! Thanks for the patch!

LGTM.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Tarantool-patches] [PATCH v1.2] evio: workaround for wsl1 so_linger assertion
  2020-03-22 19:21   ` Vladislav Shpilevoy
@ 2020-03-23 23:12     ` Vladislav Shpilevoy
  0 siblings, 0 replies; 10+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-23 23:12 UTC (permalink / raw)
  To: Timur Safin; +Cc: tarantool-patches, Cyrill Gorcunov

I pushed the patch to master, 2.3, 2.2, 1.10.

On 22/03/2020 20:21, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> LGTM.
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-03-23 23:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <bb3eef527b48bdd3aacddc27e0597ceedcb84987.1584371177.git.tsafin@tarantool.org>
2020-03-16 16:53 ` [Tarantool-patches] [PATCH v1.1] evio: workaround for wsl1 so_linger assertion Timur Safin
2020-03-16 22:35   ` Vladislav Shpilevoy
2020-03-17 14:40     ` Timur Safin
2020-03-17 21:27       ` Vladislav Shpilevoy
2020-03-18  7:12         ` Timur Safin
2020-03-19 10:27 ` [Tarantool-patches] [PATCH v1.2] " Timur Safin
2020-03-19 10:52   ` Cyrill Gorcunov
2020-03-19 11:07     ` Timur Safin
2020-03-22 19:21   ` Vladislav Shpilevoy
2020-03-23 23:12     ` Vladislav Shpilevoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox