[PATCH v3] socket: fix polling in case of spurious wakeup

Georgy Kirichenko georgy at tarantool.org
Wed Oct 10 13:13:04 MSK 2018


socket_writable/socket_readable handles socket.iowait spurious wakeup
until event is happened or timeout is exceeded.

Closes: #3344
---
https://github.com/tarantool/tarantool/issues/3344
https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-3344-socket-io-spurios-wakeup

Changes in v3:
 - Fixes according to Vova's review

Changes in v2:
 - Fixes according to Vlad's review

 src/lua/socket.lua       | 46 +++++++++++-----------------
 test/app/socket.result   | 66 ++++++++++++++++++++++++++++++++++++++++
 test/app/socket.test.lua | 28 +++++++++++++++++
 3 files changed, 112 insertions(+), 28 deletions(-)

diff --git a/src/lua/socket.lua b/src/lua/socket.lua
index 8f5e2926b..7f9b40fb1 100644
--- a/src/lua/socket.lua
+++ b/src/lua/socket.lua
@@ -332,12 +332,17 @@ local function do_wait(self, what, timeout)
     self._errno = nil
     timeout = timeout or TIMEOUT_INFINITY
 
-    local res = internal.iowait(fd, what, timeout)
-    if res == 0 then
-        self._errno = boxerrno.ETIMEDOUT
-        return 0
-    end
-    return res
+    repeat
+        local started = fiber.clock()
+        local events = internal.iowait(fd, what, timeout)
+        fiber.testcancel()
+        if events ~= 0 and events ~= '' then
+            return events
+        end
+        timeout = timeout - (fiber.clock() - started)
+    until timeout < 0
+    self._errno = boxerrno.ETIMEDOUT
+    return 0
 end
 
 local function socket_readable(self, timeout)
@@ -666,9 +671,8 @@ local function read(self, limit, timeout, check, ...)
         return data
     end
 
-    while timeout > 0 do
-        local started = fiber.clock()
-
+    local deadline = fiber.clock() + timeout
+    repeat
         assert(rbuf:size() < limit)
         local to_read = math.min(limit - rbuf:size(), buffer.READAHEAD)
         local data = rbuf:reserve(to_read)
@@ -692,16 +696,7 @@ local function read(self, limit, timeout, check, ...)
         elseif not errno_is_transient[self._errno] then
             return nil
         end
-
-        if not socket_readable(self, timeout) then
-            return nil
-        end
-        if timeout <= 0 then
-            break
-        end
-        timeout = timeout - ( fiber.clock() - started )
-    end
-    self._errno = boxerrno.ETIMEDOUT
+    until not socket_readable(self, deadline - fiber.clock())
     return nil
 end
 
@@ -739,8 +734,8 @@ local function socket_write(self, octets, timeout)
         return 0
     end
 
-    local started = fiber.clock()
-    while true do
+    local deadline = fiber.clock() + timeout
+    repeat
         local written = syswrite(self, p, e - p)
         if written == 0 then
             return p - s -- eof
@@ -754,11 +749,8 @@ local function socket_write(self, octets, timeout)
             return nil
         end
 
-        timeout = timeout - (fiber.clock() - started)
-        if timeout <= 0 or not socket_writable(self, timeout) then
-            break
-        end
-    end
+    until not socket_writable(self, deadline - fiber.clock())
+    return nil
 end
 
 local function socket_send(self, octets, flags)
@@ -999,8 +991,6 @@ local function socket_tcp_connect(s, address, port, timeout)
     -- conditions appart SO_ERROR must be consulted (man connect).
     if socket_writable(s, timeout) then
         s._errno = socket_getsockopt(s, 'SOL_SOCKET', 'SO_ERROR')
-    else
-        s._errno = boxerrno.ETIMEDOUT
     end
     if s._errno ~= 0 then
         return nil
diff --git a/test/app/socket.result b/test/app/socket.result
index 729c1eb3f..2f002a37e 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -2779,6 +2779,72 @@ listening_socket:close()
 ---
 - true
 ...
+--gh-3344 connection should not fail is there is a spurious wakeup for io fiber
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+echo_fiber = nil
+server = socket.tcp_server('localhost', 0, { handler = function(s)
+    echo_fiber = fiber.self()
+    while true do
+        local b = s:read(1, 0.1)
+        if b ~= nil then
+            s:write(b)
+        end
+    end
+end, name = 'echoserv'});
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+addr = server:name()
+---
+...
+client = socket.tcp_connect(addr.host, addr.port)
+---
+...
+echo_fiber ~= nil
+---
+- true
+...
+client:write('hello')
+---
+- 5
+...
+client:read(5, 0.1) == 'hello'
+---
+- true
+...
+-- send spurious wakeup
+fiber.wakeup(echo_fiber)
+---
+...
+fiber.sleep(0)
+---
+...
+client:write('world')
+---
+- 5
+...
+client:read(5, 0.1) == 'world'
+---
+- true
+...
+-- cancel fiber
+fiber.cancel(echo_fiber)
+---
+...
+client:read(1, 0.1) == ''
+---
+- true
+...
+server:close()
+---
+- true
+...
 test_run:cmd("clear filter")
 ---
 - true
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index 9b896522b..c0b001449 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -939,4 +939,32 @@ receiving_socket:close()
 sending_socket:close()
 listening_socket:close()
 
+--gh-3344 connection should not fail is there is a spurious wakeup for io fiber
+test_run:cmd("setopt delimiter ';'")
+echo_fiber = nil
+server = socket.tcp_server('localhost', 0, { handler = function(s)
+    echo_fiber = fiber.self()
+    while true do
+        local b = s:read(1, 0.1)
+        if b ~= nil then
+            s:write(b)
+        end
+    end
+end, name = 'echoserv'});
+test_run:cmd("setopt delimiter ''");
+addr = server:name()
+client = socket.tcp_connect(addr.host, addr.port)
+echo_fiber ~= nil
+client:write('hello')
+client:read(5, 0.1) == 'hello'
+-- send spurious wakeup
+fiber.wakeup(echo_fiber)
+fiber.sleep(0)
+client:write('world')
+client:read(5, 0.1) == 'world'
+-- cancel fiber
+fiber.cancel(echo_fiber)
+client:read(1, 0.1) == ''
+server:close()
+
 test_run:cmd("clear filter")
-- 
2.19.1




More information about the Tarantool-patches mailing list