* [Tarantool-patches] Отзыв: [PATCH] evio: workaround for wsl1 so_linger assertion
@ 2020-03-16 15:31 Тимур Сафин
0 siblings, 0 replies; 5+ messages in thread
From: Тимур Сафин @ 2020-03-16 15:31 UTC (permalink / raw)
To: Vladislav Shpilevoy, Cyrill Gorcunov; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 113 bytes --]
оНКЭГНБЮРЕКЭ рХЛСП яЮТХМ УНРЕК АШ НРНГБЮРЭ ЯННАЫЕМХЕ, "[PATCH] evio:
workaround for wsl1 so_linger assertion".
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 1697 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Tarantool-patches] [PATCH] evio: workaround for wsl1 so_linger assertion
@ 2020-03-16 15:25 Timur Safin
2020-03-16 15:32 ` Cyrill Gorcunov
0 siblings, 1 reply; 5+ messages in thread
From: Timur Safin @ 2020-03-16 15:25 UTC (permalink / raw)
To: Vladislav Shpilevoy, Cyrill Gorcunov; +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.
---
This patch replaces prior one tsafin/gh-4659-wsl-no-linger-assert because original
approach considered unsafe, and we rather want to workaround it via CMake instead.
Branch: https://github.com/tarantool/tarantool/tree/tsafin/wsl1-no-linger-assert
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..ee8870d21 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_WSL_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..6793236da 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_WSL_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] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] evio: workaround for wsl1 so_linger assertion
2020-03-16 15:25 [Tarantool-patches] " Timur Safin
@ 2020-03-16 15:32 ` Cyrill Gorcunov
2020-03-16 16:04 ` Timur Safin
0 siblings, 1 reply; 5+ messages in thread
From: Cyrill Gorcunov @ 2020-03-16 15:32 UTC (permalink / raw)
To: Timur Safin; +Cc: Cyrill Gorcunov, Vladislav Shpilevoy, tarantool-patches
On Mon, Mar 16, 2020 at 06:25:22PM +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.
> ---
>
> This patch replaces prior one tsafin/gh-4659-wsl-no-linger-assert because original
> approach considered unsafe, and we rather want to workaround it via CMake instead.
>
>
> Branch: https://github.com/tarantool/tarantool/tree/tsafin/wsl1-no-linger-assert
>
> 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..ee8870d21 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_WSL_WORKAROUND_ENABLED=1")
> + endif()
> +
Great! Thanks a huge, Timur! Since you mentioned WSL2 maybe we should
name it -DTARANTOOL_WSL_1 ? I don't insist though.
Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] evio: workaround for wsl1 so_linger assertion
2020-03-16 15:32 ` Cyrill Gorcunov
@ 2020-03-16 16:04 ` Timur Safin
2020-03-16 16:06 ` Cyrill Gorcunov
0 siblings, 1 reply; 5+ messages in thread
From: Timur Safin @ 2020-03-16 16:04 UTC (permalink / raw)
To: 'Cyrill Gorcunov'
Cc: 'Cyrill Gorcunov', 'Vladislav Shpilevoy',
tarantool-patches
: -----Original Message-----
: From: Cyrill Gorcunov <gorcunov@gmail.com>
: Sent: Monday, March 16, 2020 6:32 PM
: To: Timur Safin <tsafin@tarantool.org>
: Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>; Cyrill Gorcunov
: <gorcunov@tarantool.org>; tarantool-patches@dev.tarantool.org
: Subject: Re: [Tarantool-patches] [PATCH] evio: workaround for wsl1
: so_linger assertion
:
: > +
: > + # 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_WSL_WORKAROUND_ENABLED=1")
: > + endif()
: > +
:
: Great! Thanks a huge, Timur! Since you mentioned WSL2 maybe we should
: name it -DTARANTOOL_WSL_1 ? I don't insist though.
:
: Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>
Agreed, will rename it shortly to *WSL1_WORKAROUND*
Timur
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] evio: workaround for wsl1 so_linger assertion
2020-03-16 16:04 ` Timur Safin
@ 2020-03-16 16:06 ` Cyrill Gorcunov
0 siblings, 0 replies; 5+ messages in thread
From: Cyrill Gorcunov @ 2020-03-16 16:06 UTC (permalink / raw)
To: Timur Safin
Cc: 'Cyrill Gorcunov', 'Vladislav Shpilevoy',
tarantool-patches
On Mon, Mar 16, 2020 at 07:04:52PM +0300, Timur Safin wrote:
> :
> : Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>
>
> Agreed, will rename it shortly to *WSL1_WORKAROUND*
Thanks. You can preserve my Ack tnen.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-16 16:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-16 15:31 [Tarantool-patches] Отзыв: [PATCH] evio: workaround for wsl1 so_linger assertion Тимур Сафин
-- strict thread matches above, loose matches on Subject: below --
2020-03-16 15:25 [Tarantool-patches] " Timur Safin
2020-03-16 15:32 ` Cyrill Gorcunov
2020-03-16 16:04 ` Timur Safin
2020-03-16 16:06 ` Cyrill Gorcunov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox