From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6047946970E for ; Thu, 26 Dec 2019 00:06:14 +0300 (MSK) From: Ilya Kosarev Date: Thu, 26 Dec 2019 00:04:07 +0300 Message-Id: <20191225210407.32120-1-i.kosarev@tarantool.org> Subject: [Tarantool-patches] [PATCH v5] test: fix flaky socket test List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org socket.test had a number of flaky problems: - socket readiness expectation - race conditions on socket shutdown in emulation test cases - UDP datagrams losses on mac os - excessive random port searches Now they are solved. We are also using 127.0.0.1 instead of 0.0.0.0 or localhost to prevent wrong connections. Socket test is not fragile anymore. Closes #4426 Closes #4451 Closes #4469 --- Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4426-4451-fix-socket-test Issues: https://github.com/tarantool/tarantool/issues/4426 https://github.com/tarantool/tarantool/issues/4451 https://github.com/tarantool/tarantool/issues/4469 Changes in v2: - reconsidered socket readiness expectation - reduced conditions waiting time Changes in v3: - reconsidered expectations to unify them - simplified randomization Changes in v4: - left infinite timeouts alone - wrapped UDP datagrams awaiting with wait_cond - replaced manual port randomization with zero bind - removed extra wait_cond wrappers Changes in v5: - used 127.0.0.1 instead of 0.0.0.0 or localhost - replaced wait_cond with readableness awaiting for UDP datagrams - renamed WAIT_COND_TIME to TIMEOUT - refactored race conditions on socket shutdown to use channel instead of global flag test/app/socket.result | 120 ++++++++++++++++++++++++++------------- test/app/socket.test.lua | 83 +++++++++++++++------------ test/app/suite.ini | 1 - 3 files changed, 127 insertions(+), 77 deletions(-) diff --git a/test/app/socket.result b/test/app/socket.result index fd299424c96..f3230a2ffc9 100644 --- a/test/app/socket.result +++ b/test/app/socket.result @@ -42,7 +42,7 @@ test_run:cmd("push filter '(error: .builtin/.*[.]lua):[0-9]+' to '\\1'") --- - true ... -WAIT_COND_TIME = 10 +TIMEOUT = 100 --- ... socket('PF_INET', 'SOCK_STREAM', 'tcp121222'); @@ -107,11 +107,11 @@ s:nonblock(true) --- - true ... -s:readable(.1) +s:readable(TIMEOUT) --- - true ... -s:wait(.1) +s:wait(TIMEOUT) --- - RW ... @@ -183,7 +183,7 @@ s:writable(0) --- - true ... -s:wait(.01) +s:wait(TIMEOUT) --- - RW ... @@ -227,11 +227,11 @@ s:syswrite(ffi.cast('const char *', ping), #ping) --- - 6 ... -s:readable(1) +s:readable(TIMEOUT) --- - true ... -s:wait(.01) +s:wait(TIMEOUT) --- - RW ... @@ -308,7 +308,7 @@ sc:nonblock(true) --- - true ... -sc:readable(.5) +sc:readable(TIMEOUT) --- - true ... @@ -451,7 +451,7 @@ sc:sysconnect('127.0.0.1', s:name().port) or errno() == errno.EINPROGRESS --- - true ... -sc:writable(10) +sc:writable(TIMEOUT) --- - true ... @@ -715,7 +715,7 @@ function aexitst(ai, hostnames, port) end; --- ... -aexitst( socket.getaddrinfo('localhost', 'http', { protocol = 'tcp', +aexitst( socket.getaddrinfo('127.0.0.1', 'http', { protocol = 'tcp', type = 'SOCK_STREAM'}), {'127.0.0.1', '::1'}, 80 ); --- - true @@ -830,7 +830,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, world') --- - 12 ... -s:readable(10) +s:readable(TIMEOUT) --- - true ... @@ -842,7 +842,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, world, 2') --- - 15 ... -s:readable(10) +s:readable(TIMEOUT) --- - true ... @@ -898,7 +898,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, World!') --- - 13 ... -s:readable(1) +s:readable(TIMEOUT) --- - true ... @@ -913,7 +913,7 @@ s:sendto(from.host, from.port, 'Hello, hello!') --- - 13 ... -sc:readable(1) +sc:readable(TIMEOUT) --- - true ... @@ -1070,29 +1070,18 @@ s = nil master = socket('PF_INET', 'SOCK_STREAM', 'tcp') --- ... -master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true) +master:bind('127.0.0.1', 0) --- - true ... -port = 32768 + math.random(0, 32767) +port = master:name().port --- ... --- SO_REUSEADDR allows to bind to the same source addr:port twice, --- so listen() can return EADDRINUSE and so we check it within --- wait_cond(). -test_run:cmd("setopt delimiter ';'") +master:listen() --- - true ... -test_run:wait_cond(function() - local ok = master:bind('127.0.0.1', port) - local ok = ok and master:listen() - if not ok then - port = 32768 + math.random(32768) - return false, master:error() - end - return true -end, WAIT_COND_TIME); +test_run:cmd("setopt delimiter ';'") --- - true ... @@ -1322,7 +1311,7 @@ test_run:cmd("setopt delimiter ';'") --- - true ... -server, addr = socket.tcp_server('localhost', 0, { handler = function(s) +server, addr = socket.tcp_server('127.0.0.1', 0, { handler = function(s) s:read(2) s:write('Hello, world') end, name = 'testserv'}); @@ -1632,7 +1621,7 @@ test_run:cmd("setopt delimiter ';'") ... f = fiber.create(function() while true do - local result = socket.getaddrinfo('localhost', '80') + local result = socket.getaddrinfo('127.0.0.1', '80') fiber.sleep(0) end end); @@ -1823,7 +1812,16 @@ test_run:cmd("setopt delimiter ';'") - true ... cfiber = fiber.create(function(sc, rch, wch) - while sc:send(wch:get()) and rch:put(sc:receive("*l")) do end + while true do + local outgoing_data = wch:get() + if outgoing_data == nil then + sc:close() + break + end + sc:send(outgoing_data) + local incoming_data = sc:receive("*l") + rch:put(incoming_data) + end end, sc, rch, wch); --- ... @@ -1936,6 +1934,10 @@ c:receive("*l") --- - ... +rch:get() +--- +- $250 OK +... wch:put("Fu") --- - true @@ -1944,9 +1946,9 @@ c:send("354 Please type your message\n") --- - 29 ... -sc:close() +wch:put(nil) -- EOF --- -- 1 +- true ... c:receive("*l", "Line: ") --- @@ -2155,7 +2157,7 @@ s:close() - 1 ... -- socket.bind / socket.connect -s = socket.bind('0.0.0.0', 0) +s = socket.bind('127.0.0.1', 0) --- ... s @@ -2292,6 +2294,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) --- - 0 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message = receiving_socket:recv() --- ... @@ -2315,6 +2321,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) --- - 0 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message, from = receiving_socket:recvfrom() --- ... @@ -2392,6 +2402,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) --- - 0 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message = receiving_socket:recv(512) --- ... @@ -2415,6 +2429,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) --- - 0 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message, from = receiving_socket:recvfrom(512) --- ... @@ -2452,6 +2470,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message) --- - 1025 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message = receiving_socket:recv() --- ... @@ -2484,6 +2506,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message) --- - 1025 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message, from = receiving_socket:recvfrom() --- ... @@ -2523,6 +2549,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message) --- - 1025 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message = receiving_socket:recv(512) --- ... @@ -2566,6 +2596,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message) --- - 1025 ... +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message, from = receiving_socket:recvfrom(512) --- ... @@ -2643,6 +2677,10 @@ sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errn --- - true ... +listening_socket:readable(TIMEOUT) +--- +- true +... receiving_socket = listening_socket:accept() --- ... @@ -2651,6 +2689,10 @@ sending_socket:write(message) - 513 ... -- case: recvfrom reads first 512 bytes from the message with tcp +receiving_socket:readable(TIMEOUT) +--- +- true +... received_message, from = receiving_socket:recvfrom() --- ... @@ -2747,7 +2789,7 @@ test_run:cmd("setopt delimiter ';'") ... echo_fiber = nil channel = fiber.channel() -server = socket.tcp_server('localhost', 0, { handler = function(s) +server = socket.tcp_server('127.0.0.1', 0, { handler = function(s) echo_fiber = fiber.self() channel:put(true) while true do @@ -2804,7 +2846,7 @@ client:read(5, 5) == 'world' fiber.cancel(echo_fiber) --- ... -client:read(1, 5) == '' +client:read(1, TIMEOUT) == '' --- - true ... @@ -2816,7 +2858,7 @@ test_run:cmd("clear filter") --- - true ... --- case: sicket receive inconsistent behavior +-- case: socket receive inconsistent behavior chan = fiber.channel() --- ... @@ -2826,10 +2868,10 @@ counter = 0 fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end --- ... -srv = socket.tcp_server('0.0.0.0', 8888, fn) +srv = socket.tcp_server('127.0.0.1', 0, fn) --- ... -s = socket.connect('localhost', 8888) +s = socket.connect('127.0.0.1', srv:name().port) --- ... chan:put(5) diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua index c72d41763f4..9873b6a32d4 100644 --- a/test/app/socket.test.lua +++ b/test/app/socket.test.lua @@ -13,7 +13,7 @@ env = require('test_run') test_run = env.new() test_run:cmd("push filter '(error: .builtin/.*[.]lua):[0-9]+' to '\\1'") -WAIT_COND_TIME = 10 +TIMEOUT = 100 socket('PF_INET', 'SOCK_STREAM', 'tcp121222'); @@ -39,8 +39,8 @@ s:nonblock(false) s:nonblock() s:nonblock(true) -s:readable(.1) -s:wait(.1) +s:readable(TIMEOUT) +s:wait(TIMEOUT) socket.iowait(s:fd(), 'RW') socket.iowait(s:fd(), 3) socket.iowait(s:fd(), 'R') @@ -58,7 +58,7 @@ s:errno() > 0 s:error() s:writable(.00000000000001) s:writable(0) -s:wait(.01) +s:wait(TIMEOUT) socket.iowait(nil, nil, -1) socket.iowait(nil, nil, 0.0001) socket.iowait(-1, nil, 0.0001) @@ -75,8 +75,8 @@ ping = msgpack.encode(string.len(ping)) .. ping -- test syswrite with char * s:syswrite(ffi.cast('const char *', ping), #ping) -s:readable(1) -s:wait(.01) +s:readable(TIMEOUT) +s:wait(TIMEOUT) pong = s:sysread() string.len(pong) @@ -100,7 +100,7 @@ sc = socket('PF_INET', 'SOCK_STREAM', 'tcp') sc:nonblock(false) sc:sysconnect('127.0.0.1', s:name().port) sc:nonblock(true) -sc:readable(.5) +sc:readable(TIMEOUT) sc:sysread() string.match(tostring(sc), ', peer') ~= nil #sevres @@ -140,7 +140,7 @@ s:listen(128) sc = socket('PF_INET', 'SOCK_STREAM', 'tcp') sc:sysconnect('127.0.0.1', s:name().port) or errno() == errno.EINPROGRESS -sc:writable(10) +sc:writable(TIMEOUT) sc:write('Hello, world') sa, addr = s:accept() @@ -230,7 +230,7 @@ function aexitst(ai, hostnames, port) end; -aexitst( socket.getaddrinfo('localhost', 'http', { protocol = 'tcp', +aexitst( socket.getaddrinfo('127.0.0.1', 'http', { protocol = 'tcp', type = 'SOCK_STREAM'}), {'127.0.0.1', '::1'}, 80 ); test_run:cmd("setopt delimiter ''"); @@ -266,11 +266,11 @@ s = socket('AF_INET', 'SOCK_DGRAM', 'udp') s:bind('127.0.0.1', 0) sc = socket('AF_INET', 'SOCK_DGRAM', 'udp') sc:sendto('127.0.0.1', s:name().port, 'Hello, world') -s:readable(10) +s:readable(TIMEOUT) s:recv() sc:sendto('127.0.0.1', s:name().port, 'Hello, world, 2') -s:readable(10) +s:readable(TIMEOUT) d, from = s:recvfrom() from.port > 0 from.port = 'Random port' @@ -286,11 +286,11 @@ sc = socket('AF_INET', 'SOCK_DGRAM', 'udp') sc:nonblock(true) sc:sendto('127.0.0.1', s:name().port) sc:sendto('127.0.0.1', s:name().port, 'Hello, World!') -s:readable(1) +s:readable(TIMEOUT) data, from = s:recvfrom(10) data s:sendto(from.host, from.port, 'Hello, hello!') -sc:readable(1) +sc:readable(TIMEOUT) data_r, from_r = sc:recvfrom() data_r from_r.host @@ -342,21 +342,10 @@ s = nil -- random port master = socket('PF_INET', 'SOCK_STREAM', 'tcp') -master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true) -port = 32768 + math.random(0, 32767) --- SO_REUSEADDR allows to bind to the same source addr:port twice, --- so listen() can return EADDRINUSE and so we check it within --- wait_cond(). +master:bind('127.0.0.1', 0) +port = master:name().port +master:listen() test_run:cmd("setopt delimiter ';'") -test_run:wait_cond(function() - local ok = master:bind('127.0.0.1', port) - local ok = ok and master:listen() - if not ok then - port = 32768 + math.random(32768) - return false, master:error() - end - return true -end, WAIT_COND_TIME); function gh361() local s = socket('PF_INET', 'SOCK_STREAM', 'tcp') s:sysconnect('127.0.0.1', port) @@ -434,7 +423,7 @@ server:close() while fio.stat(path) ~= nil do fiber.sleep(0.001) end test_run:cmd("setopt delimiter ';'") -server, addr = socket.tcp_server('localhost', 0, { handler = function(s) +server, addr = socket.tcp_server('127.0.0.1', 0, { handler = function(s) s:read(2) s:write('Hello, world') end, name = 'testserv'}); @@ -546,7 +535,7 @@ socket.getaddrinfo('host', 'port', { flags = 'WRONG' }) == nil and errno() == er test_run:cmd("setopt delimiter ';'") f = fiber.create(function() while true do - local result = socket.getaddrinfo('localhost', '80') + local result = socket.getaddrinfo('127.0.0.1', '80') fiber.sleep(0) end end); @@ -620,7 +609,16 @@ rch, wch = fiber.channel(1), fiber.channel(1) sc = socket.connect(host, port) test_run:cmd("setopt delimiter ';'") cfiber = fiber.create(function(sc, rch, wch) - while sc:send(wch:get()) and rch:put(sc:receive("*l")) do end + while true do + local outgoing_data = wch:get() + if outgoing_data == nil then + sc:close() + break + end + sc:send(outgoing_data) + local incoming_data = sc:receive("*l") + rch:put(incoming_data) + end end, sc, rch, wch); test_run:cmd("setopt delimiter ''"); @@ -651,9 +649,10 @@ rch:get() wch:put("DATA\n") c:receive(4) c:receive("*l") +rch:get() wch:put("Fu") c:send("354 Please type your message\n") -sc:close() +wch:put(nil) -- EOF c:receive("*l", "Line: ") c:receive() c:receive(10) @@ -713,7 +712,7 @@ sc:close() s:close() -- socket.bind / socket.connect -s = socket.bind('0.0.0.0', 0) +s = socket.bind('127.0.0.1', 0) s host, port, family = s:getsockname() sc = socket.connect(host, port) @@ -778,6 +777,7 @@ e == errno.EAGAIN -- expected true -- case: recv, zero datagram sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) +receiving_socket:readable(TIMEOUT) received_message = receiving_socket:recv() e = receiving_socket:errno() received_message == '' -- expected true @@ -786,6 +786,7 @@ e == 0 -- expected true -- case: recvfrom, zero datagram sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) +receiving_socket:readable(TIMEOUT) received_message, from = receiving_socket:recvfrom() e = receiving_socket:errno() received_message == '' -- expected true @@ -812,6 +813,7 @@ e == errno.EAGAIN -- expected true -- case: recv, zero datagram, explicit size sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) +receiving_socket:readable(TIMEOUT) received_message = receiving_socket:recv(512) e = receiving_socket:errno() received_message == '' -- expected true @@ -820,6 +822,7 @@ e == 0 -- expected true -- case: recvfrom, zero datagram, explicit size sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port) +receiving_socket:readable(TIMEOUT) received_message, from = receiving_socket:recvfrom(512) e = receiving_socket:errno() received_message == '' -- expected true @@ -833,6 +836,7 @@ message = string.rep('x', message_len) -- case: recv, non-zero length datagram, the buffer size should be evaluated sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +receiving_socket:readable(TIMEOUT) received_message = receiving_socket:recv() e = receiving_socket:errno() received_message == message -- expected true @@ -844,6 +848,7 @@ e -- case: recvfrom, non-zero length datagram, the buffer size should be -- evaluated sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +receiving_socket:readable(TIMEOUT) received_message, from = receiving_socket:recvfrom() e = receiving_socket:errno() received_message == message -- expected true @@ -856,6 +861,7 @@ e -- case: recv truncates a datagram larger then the buffer of an explicit size sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +receiving_socket:readable(TIMEOUT) received_message = receiving_socket:recv(512) e = receiving_socket:errno() received_message == message:sub(1, 512) -- expected true @@ -872,6 +878,7 @@ message = string.rep('y', message_len) -- case: recvfrom truncates a datagram larger then the buffer of an explicit size sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +receiving_socket:readable(TIMEOUT) received_message, from = receiving_socket:recvfrom(512) e = receiving_socket:errno() received_message == message:sub(1, 512) -- expected true @@ -896,10 +903,12 @@ listening_socket:listen() listening_socket_port = listening_socket:name().port sending_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp') sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errno.EINPROGRESS +listening_socket:readable(TIMEOUT) receiving_socket = listening_socket:accept() sending_socket:write(message) -- case: recvfrom reads first 512 bytes from the message with tcp +receiving_socket:readable(TIMEOUT) received_message, from = receiving_socket:recvfrom() e = receiving_socket:errno() received_message == message:sub(1, 512) -- expected true @@ -931,7 +940,7 @@ listening_socket:close() test_run:cmd("setopt delimiter ';'") echo_fiber = nil channel = fiber.channel() -server = socket.tcp_server('localhost', 0, { handler = function(s) +server = socket.tcp_server('127.0.0.1', 0, { handler = function(s) echo_fiber = fiber.self() channel:put(true) while true do @@ -955,17 +964,17 @@ client:write('world') client:read(5, 5) == 'world' -- cancel fiber fiber.cancel(echo_fiber) -client:read(1, 5) == '' +client:read(1, TIMEOUT) == '' server:close() test_run:cmd("clear filter") --- case: sicket receive inconsistent behavior +-- case: socket receive inconsistent behavior chan = fiber.channel() counter = 0 fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end -srv = socket.tcp_server('0.0.0.0', 8888, fn) -s = socket.connect('localhost', 8888) +srv = socket.tcp_server('127.0.0.1', 0, fn) +s = socket.connect('127.0.0.1', srv:name().port) chan:put(5) chan:put(5) s:receive(5) diff --git a/test/app/suite.ini b/test/app/suite.ini index 79432e29a76..dd802d98cf9 100644 --- a/test/app/suite.ini +++ b/test/app/suite.ini @@ -7,4 +7,3 @@ use_unix_sockets = True use_unix_sockets_iproto = True is_parallel = True pretest_clean = True -fragile = socket.test.lua ; gh-4426 gh-4451 -- 2.17.1