From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Georgy Kirichenko Subject: [PATCH v3] socket: fix polling in case of spurious wakeup Date: Wed, 10 Oct 2018 13:13:04 +0300 Message-Id: In-Reply-To: <20181005125400.vcj4mbbrfrfpr736@esperanza> References: <20181005125400.vcj4mbbrfrfpr736@esperanza> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com, Georgy Kirichenko List-ID: 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