[Tarantool-patches] [PATCH V2] socket: fix error while closing socket.tcp_server

[Tarantool-patches] [PATCH V2] socket: fix error while closing socket.tcp_server

[Leonid Vasiliev]

The tcp server starts in a separate fiber.
When server's socket is closed from another fiber,
an exception will be thrown in server's loop from
check_socket function.
A "socket is close" check has been added at server's
fiber and now server's fiber terminates correctly.

Fixes: #4087
---
https://github.com/tarantool/tarantool/issues/4087
https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4087-fix-socket-stuff

@Changelog fix error while closing socket.tcp_server socket(gh-4087)

For a "socket is close" check, fd < 0 is used instead of an additional flag.

 src/lua/socket.lua       | 14 +++++++++++---
 test/app/socket.result   | 14 ++++++++++++++
 test/app/socket.test.lua |  7 ++++++-
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/lua/socket.lua b/src/lua/socket.lua
index a334ad4..bbdef01 100644
--- a/src/lua/socket.lua
+++ b/src/lua/socket.lua
@@ -73,12 +73,17 @@ local gc_socket_t = ffi.metatype(ffi.typeof('struct gc_socket'), {
 
 local socket_mt
 
+local function socket_is_closed(socket)
+    -- The type of 'socket' is not checked because the function
+    -- is internal and the type must be checked outside if needed.
+    return socket._gc_socket.fd < 0
+end
+
 local function check_socket(socket)
     local gc_socket = type(socket) == 'table' and socket._gc_socket
     if ffi.istype(gc_socket_t, gc_socket) then
-        local fd = gc_socket.fd
-        if fd >= 0 then
-            return fd
+        if not socket_is_closed(socket) then
+            return gc_socket.fd
         else
             error("attempt to use closed socket")
         end
@@ -1082,6 +1087,9 @@ local function tcp_server_loop(server, s, addr)
     fiber.name(format("%s/%s:%s", server.name, addr.host, addr.port), {truncate = true})
     log.info("started")
     while socket_readable(s) do
+        if socket_is_closed(s) then
+            break
+        end
         local sc, from = socket_accept(s)
         if sc == nil then
             local errno = s._errno
diff --git a/test/app/socket.result b/test/app/socket.result
index 9f0ea0a..6206848 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -2914,3 +2914,17 @@ srv:close()
 ---
 - true
 ...
+--
+-- gh-4087: fix error while closing socket.tcp_server
+--
+srv = socket.tcp_server('127.0.0.1', 0, function() end)
+---
+...
+srv:close()
+---
+- true
+...
+test_run:wait_log('default', 'stopped', 1024, 0.01)
+---
+- stopped
+...
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index d1fe7ad..a399fca 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -987,4 +987,9 @@ s:settimeout(1)
 s:receive('*a')
 s:close()
 srv:close()
-
+--
+-- gh-4087: fix error while closing socket.tcp_server
+--
+srv = socket.tcp_server('127.0.0.1', 0, function() end)
+srv:close()
+test_run:wait_log('default', 'stopped', 1024, 0.01)
-- 
2.7.4

Re: [Tarantool-patches] [PATCH V2] socket: fix error while closing socket.tcp_server

[Alexander Turenko]

On Tue, May 19, 2020 at 11:05:25PM +0300, Leonid Vasiliev wrote:
> The tcp server starts in a separate fiber.
> When server's socket is closed from another fiber,
> an exception will be thrown in server's loop from
> check_socket function.
> A "socket is close" check has been added at server's
> fiber and now server's fiber terminates correctly.
> 
> Fixes: #4087
> ---
> https://github.com/tarantool/tarantool/issues/4087
> https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4087-fix-socket-stuff
> 
> @Changelog fix error while closing socket.tcp_server socket(gh-4087)
> 
> For a "socket is close" check, fd < 0 is used instead of an additional flag.

LGTM.

Re: [Tarantool-patches] [PATCH V2] socket: fix error while closing socket.tcp_server

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]


LGTM

  
>Пятница, 29 мая 2020, 15:41 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
> 
>On Tue, May 19, 2020 at 11:05:25PM +0300, Leonid Vasiliev wrote:
>> The tcp server starts in a separate fiber.
>> When server's socket is closed from another fiber,
>> an exception will be thrown in server's loop from
>> check_socket function.
>> A "socket is close" check has been added at server's
>> fiber and now server's fiber terminates correctly.
>>
>> Fixes: #4087
>> ---
>>  https://github.com/tarantool/tarantool/issues/4087
>>  https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4087-fix-socket-stuff
>>
>> @Changelog fix error while closing socket.tcp_server socket(gh-4087)
>>
>> For a "socket is close" check, fd < 0 is used instead of an additional flag.
>LGTM. 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 1565 bytes --]

Re: [Tarantool-patches] [PATCH V2] socket: fix error while closing socket.tcp_server

[Kirill Yukhin]

Hello,

On 19 май 23:05, Leonid Vasiliev wrote:
> The tcp server starts in a separate fiber.
> When server's socket is closed from another fiber,
> an exception will be thrown in server's loop from
> check_socket function.
> A "socket is close" check has been added at server's
> fiber and now server's fiber terminates correctly.
> 
> Fixes: #4087

LGTM.

I've checked your patch into 1.10, 2.3, 2.4 and master.

--
Regards, Kirill Yukhin