[tarantool-patches] Re: [PATCH] Fix lua socket polling in case of a spurious wakeup

  • From: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx, Georgy Kirichenko <georgy@xxxxxxxxxxxxx>
  • Date: Wed, 18 Jul 2018 20:06:01 +0300

Thanks for the fixes!

Please, use [PATCH v2] prefix when sending second
version of a patch.

See 2 comments below.

On 18/07/2018 19:46, Georgy Kirichenko wrote:

socket_writable/socket_readable may return before timeout is exceeded
with the false status in cause of a spurious wakeup and this should not be
treated as an EOF or an error.

Fixed #3344
---
Issue: https://github.com/tarantool/tarantool/issues/3344
Branch:
https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-3344-socket-io-spurios-wakeup
  src/lua/socket.lua       | 57 ++++++++++++++++----------------
  test/app/socket.result   | 70 ++++++++++++++++++++++++++++++++++++++++
  test/app/socket.test.lua | 29 +++++++++++++++++
  3 files changed, 129 insertions(+), 27 deletions(-)

diff --git a/src/lua/socket.lua b/src/lua/socket.lua
index 06306eae2..6ebcea055 100644
--- a/src/lua/socket.lua
+++ b/src/lua/socket.lua
@@ -337,11 +337,15 @@ local function do_wait(self, what, timeout)
  end

1. The function 'do_wait' now sets _errno to timeout
even if it was actually not timeout but spurious wakeup.
I think, we should not set any errors in such a case.
Actually, do_wait is called from only three functions,
so we can set _errno in them when the timeout really
occurs.

local function socket_readable(self, timeout)
-    return do_wait(self, 1, timeout) ~= 0
+    local result = do_wait(self, 1, timeout) ~= 0

2. Function socket_readable now can return even if the
socket actually is not readable because of spurious
wakeup. It can lead to block on read() as I understand,
if we are trying to read non-readable thing. And breaks
logic of this function.

I think, that here we should check do_wait result to
actually contain 'R'. If not 'R' and the timeout still
is not depleted, we should call do_wait again.

+    fiber.testcancel()
+    return result
  end
local function socket_writable(self, timeout)
-    return do_wait(self, 2, timeout) ~= 0
+    local result = do_wait(self, 2, timeout) ~= 0
+    fiber.testcancel()
+    return result
  end
local function socket_wait(self, timeout)

Other related posts:

  1. Home
  2. tarantool-patches
  3. Archive
  4. 07-2018