From 9b361ccd64f2653fdaa2e5f9f59adbba5b1fb1c3 Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Thu, 29 Oct 2026 11:03:41 +0100 Subject: [PATCH] THRIFT-6467: Wait the full timeout below one second in the Lua sockets Client: lua socket_wait() and l_socket_create_and_connect() computed their deadline from the millisecond timeout with an integer division by 1000, so a timeout below one second expired at once and the fraction of any longer one was dropped. Both now compute the deadline in fractional seconds. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/build.yml | 5 +- lib/lua/Makefile.am | 4 +- lib/lua/src/luasocket.c | 2 +- lib/lua/src/usocket.c | 2 +- lib/lua/test/test_socket_timeout.lua | 109 +++++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 lib/lua/test/test_socket_timeout.lua diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9346e844f7f..69509bf38cb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1634,8 +1634,9 @@ jobs: run: make -C lib/lua # Generates the types test_recursion_depth.lua needs, then runs the - # scripts in lib/lua/test. Those preload pure-Lua stand-ins for the C - # modules, so it is the cross test below that loads the ones built above. + # scripts in lib/lua/test. All but test_socket_timeout.lua preload + # pure-Lua stand-ins for the C modules; that one and the cross test below + # load the ones built above. - name: Run make check for lua run: make -C lib/lua check diff --git a/lib/lua/Makefile.am b/lib/lua/Makefile.am index 58ab681f609..9bcabaf20ca 100644 --- a/lib/lua/Makefile.am +++ b/lib/lua/Makefile.am @@ -84,7 +84,8 @@ EXTRA_DIST = \ test/test_frame_and_string_limits.lua \ test/test_container_limits.lua \ test/test_json_escapes.lua \ - test/test_socket_write.lua + test/test_socket_write.lua \ + test/test_socket_timeout.lua THRIFT = $(top_builddir)/compiler/cpp/thrift @@ -104,6 +105,7 @@ check-local: test/gen-lua/RecursionDepth_ttypes.lua $(LUA) test/test_container_limits.lua $(LUA) test/test_json_escapes.lua $(LUA) test/test_socket_write.lua + $(LUA) test/test_socket_timeout.lua ./test/test_socket_send clean-local: diff --git a/lib/lua/src/luasocket.c b/lib/lua/src/luasocket.c index 07524ab6a02..cd05d22499b 100644 --- a/lib/lua/src/luasocket.c +++ b/lib/lua/src/luasocket.c @@ -346,7 +346,7 @@ static int l_socket_create_and_connect(lua_State *L) { int timeout = luaL_checknumber(L, 3); // Create and connect loop for timeout milliseconds - end = __gettime() + timeout/1000; + end = __gettime() + timeout / 1000.0; do { // Create and connect the socket err = tcp_create_and_connect(&sock, host, port, timeout); diff --git a/lib/lua/src/usocket.c b/lib/lua/src/usocket.c index 8421d668c9b..2fed2b7ccc1 100644 --- a/lib/lua/src/usocket.c +++ b/lib/lua/src/usocket.c @@ -56,7 +56,7 @@ T_ERRCODE socket_wait(p_socket sock, int mode, int timeout) { return TIMEOUT; } - end = __gettime() + timeout/1000; + end = __gettime() + timeout / 1000.0; do { FD_ZERO(&rfds); FD_ZERO(&wfds); diff --git a/lib/lua/test/test_socket_timeout.lua b/lib/lua/test/test_socket_timeout.lua new file mode 100644 index 00000000000..9f701073389 --- /dev/null +++ b/lib/lua/test/test_socket_timeout.lua @@ -0,0 +1,109 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. +-- +-- A socket timeout is given in milliseconds, and a wait lasts that long also +-- when the timeout is shorter than a second or has a fraction of one: +-- accept() on a server socket waits that long for a connection, and +-- TSocket:open() keeps retrying a refused connection for that long. +-- +-- Unlike the other scripts here, this one opens sockets on 127.0.0.1, with +-- the C socket module that make builds into ../.libs. +-- +-- lua lib/lua/test/test_socket_timeout.lua + +local script_dir = arg[0]:match('(.*[/\\])') or './' +package.path = script_dir .. '../?.lua;' .. package.path +package.cpath = script_dir .. '../.libs/?.so;' .. package.cpath + +require('Thrift') +require('TSocket') + +local failures = 0 + +local function check(condition, message) + if condition then + print('ok - ' .. message) + else + failures = failures + 1 + print('not ok - ' .. message) + end +end + +-- Wall-clock seconds with a fraction. os.time() counts whole seconds, and +-- os.clock() counts processor time, which a blocked wait does not use. +local function now() + local pipe = assert(io.popen('date +%s.%N')) + local text = pipe:read('l') + pipe:close() + local seconds = tonumber(text) + assert(seconds and text:find('%.%d'), + 'date +%s.%N gave no fraction of a second: ' .. tostring(text)) + return seconds +end + +-- How long fn took, then what pcall(fn) returned. +local function timed(fn) + local started = now() + local ok, err = pcall(fn) + return now() - started, ok, err +end + +-- A server socket on 127.0.0.1, at a port the system picks, listening with +-- the given timeout, and that port. The timeout has to be set before listen(). +-- getsockinfo() hands the port through a C short, so it can come back negative. +local function listening(timeout) + local server = TServerSocket:new{host = '127.0.0.1', port = 0} + server:setTimeout(timeout) + server:listen() + return server, math.floor(server:getSocketInfo().port % 65536) +end + +-- accept() with nobody connecting waits out the timeout, then reports it. +for _, case in ipairs({{ms = 500, least = 0.3}, {ms = 1500, least = 1.3}}) do + local server = listening(case.ms) + local took, ok, err = timed(function() return server:accept() end) + server:close() + check(not ok and tostring(err):find('Timeout', 1, true) ~= nil, + string.format('accept() with setTimeout(%d) and no client reports ' .. + 'a timeout: %s', case.ms, tostring(err))) + check(took >= case.least, + string.format('... after waiting at least %.1f s (waited %.3f s)', + case.least, took)) +end + +-- open() retries a refused connection until its timeout has passed. +do + local server, port = listening(1000) + server:close() -- nothing listens at that port any more + local client = TSocket:new{host = '127.0.0.1', port = port} + client:setTimeout(500) + local took, ok, err = timed(function() client:open() end) + client:close() + check(not ok and tostring(err):find('Could not connect', 1, true) ~= nil, + 'open() with setTimeout(500) to a port nobody listens at fails: ' .. + tostring(err)) + check(took >= 0.3, + string.format('... after retrying for at least 0.3 s (took %.3f s)', + took)) +end + +if failures > 0 then + print(failures .. ' failure(s)') + os.exit(1) +end +print('all ok')