* [Tarantool-patches] [PATCH luajit v2] Improve CLI signal handling on POSIX.
2026-10-21 17:42 [Tarantool-patches] [PATCH luajit] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
@ 2026-10-21 23:23 ` Evgeniy Temirgaleev via Tarantool-patches
2026-10-26 12:59 ` [Tarantool-patches] [PATCH luajit] " Sergey Bronnikov via Tarantool-patches
1 sibling, 0 replies; 3+ messages in thread
From: Evgeniy Temirgaleev via Tarantool-patches @ 2026-10-21 23:23 UTC (permalink / raw)
To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches
Hi, all. Sorry, I missed some lint fixes.
Fixes is force pushed to the branch:
- lint;
- Also tuned wait time in 'script.sh' from 1.5s to 3s. It seems, the
valgrind/sanitaizers tests flaks.
Please, see v2:
---
From: Mike Pall <mike>
(cherry picked from commit 423ac2144b0447c925596f4d93e073ce501ed4e1)
An obsolete signal() function in glibc implemented as a wrapper for
POSIX sigaction() call with BSD semantic. In particular, it provides
an SA_RESTART flag for SIGINT handler [1-2]. The last makes certain
system calls, e.g. read() or wait(), restartable across signals [3].
The patch switches from signal() to sigaction() for POSIX systems.
1. https://man7.org/linux/man-pages/man2/signal.2.html
2. https://ftp.gnu.org/gnu/glibc/glibc-2.44.tar.xz
(sysdeps/posix/signal.c, line 44)
3. https://man7.org/linux/man-pages/man2/sigaction.2.html
Temir Galeev:
* added the description and the test for the problem
Part of tarantool/tarantool#13442
---
Branch:
https://github.com/tarantool/luajit/tree/tmr_g/lj-noticket-improve-cli-sig-handling-2
src/luajit.c | 23 +++++--
...ket-fix-cli-posix-sigint-handling.test.lua | 17 +++++
.../script.lua | 25 +++++++
.../script.sh | 28 ++++++++
test/tarantool-tests/utils/exec.lua | 66 +++++++++++++++----
5 files changed, 144 insertions(+), 15 deletions(-)
create mode 100644
test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
create mode 100644
test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
create mode 100644
test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh
diff --git a/src/luajit.c b/src/luajit.c
index d9d85361..d35bea54 100644
--- a/src/luajit.c
+++ b/src/luajit.c
@@ -35,6 +35,21 @@
#if !LJ_TARGET_CONSOLE
#include <signal.h>
+
+#if LJ_TARGET_POSIX
+/* Improve signal handling on POSIX. Try CTRL-C on: luajit -e
'io.read()' */
+static void signal_set(int sig, void (*h)(int))
+{
+ struct sigaction sa;
+ memset(&sa, 0, sizeof(sa));
+ sa.sa_handler = h;
+ sigemptyset(&sa.sa_mask);
+ sigaction(sig, &sa, NULL);
+}
+#else
+#define signal_set signal
+#endif
+
#endif
static lua_State *globalL = NULL;
@@ -54,8 +69,8 @@ static void lstop(lua_State *L, lua_Debug *ar)
static void laction(int i)
{
- signal(i, SIG_DFL); /* if another SIGINT happens before lstop,
- terminate process (default action) */
+ /* Terminate process if another SIGINT happens (double CTRL-C). */
+ signal_set(i, SIG_DFL);
lua_sethook(globalL, lstop, LUA_MASKCALL | LUA_MASKRET |
LUA_MASKCOUNT, 1);
}
#endif
@@ -129,11 +144,11 @@ static int docall(lua_State *L, int narg, int clear)
lua_pushcfunction(L, traceback); /* push traceback function */
lua_insert(L, base); /* put it under chunk and args */
#if !LJ_TARGET_CONSOLE
- signal(SIGINT, laction);
+ signal_set(SIGINT, laction);
#endif
status = lua_pcall(L, narg, (clear ? 0 : LUA_MULTRET), base);
#if !LJ_TARGET_CONSOLE
- signal(SIGINT, SIG_DFL);
+ signal_set(SIGINT, SIG_DFL);
#endif
lua_remove(L, base); /* remove traceback function */
/* force a complete garbage collection in case of errors */
diff --git
a/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
new file mode 100644
index 00000000..4ea316d8
--- /dev/null
+++
b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
@@ -0,0 +1,17 @@
+local tap = require('tap')
+local test =
tap.test('lj-noticket-fix-cli-posix-sigint-handling'):skipcond({
+ ['Check LuaJIT CLI signal handling'] = _TARANTOOL,
+ ['Disabled on POSIX nonconformant OS'] = jit.os ~= 'BSD' and
+ jit.os ~= 'Linux' and
+ jit.os ~= 'OSX' and
+ jit.os ~= 'POSIX',
+})
+
+test:plan(1)
+local utils = require('utils')
+local script = utils.exec.makecmd_bash(arg, {t = test})
+local output = script(
+ utils.exec.luabin(arg),
+ utils.exec.test_data(arg, 'script.lua'))
+test:is(output, 'OK')
+test:done(true)
diff --git
a/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
new file mode 100644
index 00000000..c8e6b70b
--- /dev/null
+++
b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
@@ -0,0 +1,25 @@
+local ffi = require("ffi")
+
+ffi.cdef[[
+ int fork(void);
+ unsigned int sleep(unsigned int seconds);
+ int waitpid(int pid, int *status, int options);
+]]
+
+local pid = ffi.C.fork()
+
+if pid == 0 then
+ -- child.
+ ffi.C.sleep(10)
+elseif pid > 0 then
+ -- parent.
+ local status = ffi.new("int[1]")
+ -- wait() system call: it restarts after signal is handled
+ -- if SA_RESTART flag is set for the handler in sigaction()
+ -- or if old signal() is used and it's OS specific
+ -- implementation is BSD-aware.
+ ffi.C.waitpid(pid, status, 0)
+else
+ -- fork() failed.
+ os.exit(1)
+end
diff --git
a/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh
b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh
new file mode 100644
index 00000000..17d8241a
--- /dev/null
+++
b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh
@@ -0,0 +1,28 @@
+LUA_BIN=$1
+SCRIPT_TO_RUN=$2
+$LUA_BIN $SCRIPT_TO_RUN 2>/dev/null &
+pid=$!
+
+# Wait for process is started and running.
+sleep 3
+if ! kill -0 "$pid" 2>/dev/null; then
+ echo 'ERR'
+ exit 1
+fi
+
+# Send SIGINT and wait for termination.
+kill -INT "$pid" 2>/dev/null
+sleep 3
+
+# Terminated? OK, signal handled -- 'sigaction() behavior'.
+if ! kill -0 "$pid" 2>/dev/null; then
+ wait "$pid" 2>/dev/null
+ echo 'OK'
+ exit 0
+fi
+
+# Not terminated -- 'signal() BSD-behavior'.
+kill -KILL "$pid" 2>/dev/null
+wait "$pid" 2>/dev/null
+echo 'FAIL'
+exit 0
diff --git a/test/tarantool-tests/utils/exec.lua
b/test/tarantool-tests/utils/exec.lua
index 49f8ef06..80ca87fc 100644
--- a/test/tarantool-tests/utils/exec.lua
+++ b/test/tarantool-tests/utils/exec.lua
@@ -43,33 +43,77 @@ local function makeenv(tabenv)
return table.concat(flatenv, ' ')
end
--- <makecmd> creates a command that runs %testname%/script.lua by
--- <LUAJIT_TEST_BINARY> with the given environment, launch options
--- and CLI arguments. The function yields an object (i.e. table)
--- with the aforementioned parameters. To launch the command just
--- call the object.
-function M.makecmd(arg, opts)
+-- <makecmd_exec> creates a command that runs given binary
+-- with the given environment, launch options and zero or
+-- more permanent arguments.
+-- The function yields an object (i.e. table) with the
+-- aforementioned parameters. To launch the command just call
+-- the object. Additional arguments which may vary from call
+-- to call are the `__call` args.
+-- Pass tap.test() object as opt.t for diagnostic.
+function M.makecmd_exec(bin, opts)
+ assert(type(bin) == 'string')
+ assert(opts == nil or type(opts) == 'table')
+ local perm_args = opts and opts.args or {}
+ assert(type(perm_args) == 'table')
return setmetatable({
- LUABIN = M.luacmd(arg),
- SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$',
'/script.lua'),
+ BIN = bin,
ENV = opts and makeenv(opts.env) or '',
REDIRECT = opts and opts.redirect or '',
+ PERMARGS = (' %s'):rep(#perm_args):format(unpack(perm_args)),
+ t = opts and opts.t,
}, {
__call = function(self, ...)
-- This line just makes the command for <io.popen> by the
-- following steps:
-- 1. Replace the placeholders with the corresponding values
- -- given to the command constructor (e.g. script, env).
+ -- given to the command constructor (e.g. bin, env).
-- 2. Join all CLI arguments given to the __call metamethod.
-- 3. Concatenate the results of step 1 and step 2 to obtain
-- the resulting command.
- local cmd = ('<ENV> <LUABIN> <REDIRECT>
<SCRIPT>'):gsub('%<(%w+)>', self)
+ local cmd = ('<ENV> <BIN> <REDIRECT>
<PERMARGS>'):gsub('%<(%w+)>', self)
.. (' %s'):rep(select('#', ...)):format(...)
+ if self.t ~= nil then self.t:diag('cmd: '..cmd) end
-- Trim both leading and trailing whitespace from the output
-- produced by the child process.
- return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
+ local output = io.popen(cmd):
+ read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
+ if self.t ~= nil then self.t:diag('output: '..output) end
+ return output
end
})
end
+-- Makes a file name for running test's data file in
+-- '%testname%' directory.
+function M.test_data(arg, name)
+ return arg[0]:gsub('%.test%.lua$', '/'..name)
+end
+
+-- <makecmd_bash> creates a command that runs %testname%/script.sh
+-- with the given environment, launch options and CLI arguments.
+function M.makecmd_bash(arg, opts)
+ local script = opts and opts.script or M.test_data(arg, 'script.sh')
+ return M.makecmd_exec('/bin/bash', {
+ env = opts and opts.env or nil,
+ redirect = opts and opts.redirect or nil,
+ args = {script},
+ t = opts and opts.t or nil,
+ })
+end
+
+-- <makecmd> creates a command that runs %testname%/script.lua by
+-- <LUAJIT_TEST_BINARY> with the given environment, launch options
+-- and CLI arguments.
+function M.makecmd(arg, opts)
+ local luabin = M.luacmd(arg)
+ local script = opts and opts.script or M.test_data(arg, 'script.lua')
+ return M.makecmd_exec(luabin, {
+ env = opts and opts.env or nil,
+ redirect = opts and opts.redirect or nil,
+ args = {script},
+ t = opts and opts.t or nil,
+ })
+end
+
return M
--
2.49.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Tarantool-patches] [PATCH luajit] Improve CLI signal handling on POSIX.
2026-10-21 17:42 [Tarantool-patches] [PATCH luajit] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
2026-10-21 23:23 ` [Tarantool-patches] [PATCH luajit v2] " Evgeniy Temirgaleev via Tarantool-patches
@ 2026-10-26 12:59 ` Sergey Bronnikov via Tarantool-patches
1 sibling, 0 replies; 3+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2026-10-26 12:59 UTC (permalink / raw)
To: Evgeniy Temirgaleev, Sergey Kaplun; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 15580 bytes --]
Hi, Evgeniy,
thanks for the patch! See my comments below.
Sergey
On 10/21/26 20:42, Evgeniy Temirgaleev wrote:
> From: Mike Pall <mike>
>
> (cherry picked from commit 423ac2144b0447c925596f4d93e073ce501ed4e1)
>
> An obsolete signal() function in glibc implemented as a wrapper for
s/implemented/is implemented/
> POSIX sigaction() call with BSD semantic. In particular, it provides
> an SA_RESTART flag for SIGINT handler [1-2]. The last makes certain
s/last/latter/
> system calls, e.g. read() or wait(), restartable across signals [3].
>
> The patch switches from signal() to sigaction() for POSIX systems.
>
> 1.https://man7.org/linux/man-pages/man2/signal.2.html
> 2.https://ftp.gnu.org/gnu/glibc/glibc-2.44.tar.xz (sysdeps/posix/signal.c, line 44)
isn't better to replace it with a link to a file in repository?
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/posix/signal.c;h=7a80338fd6270b2b0e713c32c98c66fb284bd196;hb=HEAD
> 3.https://man7.org/linux/man-pages/man2/sigaction.2.html
>
> Temir Galeev:
> * added the description and the test for the problem
Please consider replacing the shell scripts and helpers with an
implementation that uses FFI C
and libc functions. The draft of the script.lua is below:
local ffi = require('ffi')
ffi.cdef[[
int pipe(int *fds);
int getpid(void);
long read(int fd, void *buf, size_t count);
unsigned int alarm(unsigned int seconds);
]]
local fds = ffi.new('int[2]')
assert(ffi.C.pipe(fds) == 0)
-- Fail instead of hanging forever if SIGINT does not break the call.
ffi.C.alarm(5)
-- The CLI installs the SIGINT handler before the script starts, so this
-- line is the readiness marker for the test.
io.write(ffi.C.getpid(), '\n')
io.flush()
-- Block in read(2) until SIGINT. Without SA_RESTART it returns EINTR and
-- the CLI runs the Lua hook, which raises "interrupted!".
ffi.C.read(fds[0], ffi.new('char[1]'), 1)
and
test.lua:
test:plan(1)
local ffi = require('ffi')
ffi.cdef[[
int kill(int pid, int sig);
]]
local SIGINT = 2
local utils = require('utils')
local script = arg[0]:gsub('%.test%.lua$', '/script.lua')
local cmd = ('exec %s %s 2>&1'):format(utils.exec.luacmd(arg), script)
local proc = io.popen(cmd)
-- The child prints its PID after the CLI has installed the SIGINT
-- handler, so it is a readiness marker. Skip possible noise lines.
local pid
for _ = 1, 10 do
local line = proc:read('*l')
if line == nil then break end
pid = line:match('^%d+$') and tonumber(line)
if pid ~= nil then break end
end
assert(pid ~= nil, 'the child process must report its PID')
ffi.C.kill(pid, SIGINT)
local output = proc:read('*a')
proc:close()
test:like(output, 'interrupted!', 'SIGINT interrupts the blocking call')
test:done(true)
>
> Part of tarantool/tarantool#13442
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/tmr_g/lj-noticket-improve-cli-sig-handling-2
>
> src/luajit.c | 23 +++++--
> ...ket-fix-cli-posix-sigint-handling.test.lua | 17 +++++
> .../script.lua | 25 ++++++++
> .../script.sh | 28 +++++++++
> test/tarantool-tests/utils/exec.lua | 63 +++++++++++++++----
> 5 files changed, 141 insertions(+), 15 deletions(-)
> create mode 100644 test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
> create mode 100644 test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
> create mode 100644 test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh
The commit mixes three things: the C change, its test, and a generic
refactoring
of `utils/exec.lua` (new `makecmd_exec`/`makecmd_bash`/`test_data`, with
`makecmd` rewritten on top of them). I would split the existing patch to
two patches.
>
> diff --git a/src/luajit.c b/src/luajit.c
> index d9d85361..d35bea54 100644
> --- a/src/luajit.c
> +++ b/src/luajit.c
> @@ -35,6 +35,21 @@
>
> #if !LJ_TARGET_CONSOLE
> #include <signal.h>
> +
> +#if LJ_TARGET_POSIX
> +/* Improve signal handling on POSIX. Try CTRL-C on: luajit -e 'io.read()' */
> +static void signal_set(int sig, void (*h)(int))
> +{
> + struct sigaction sa;
> + memset(&sa, 0, sizeof(sa));
`signal_set()` drops `SA_RESTART` only implicitly: `memset(&sa, 0,
sizeof(sa))`
zeroes `sa_flags` and nothing is assigned to the field afterwards. That is
exactly what the patch needs, but a reader (or a future "sync with glibc"
change) may see the zero flags as an omission and restore `SA_RESTART`,
silently reverting the fix. It might be worth explicitly noting this in
the commit message.
> + sa.sa_handler = h;
> + sigemptyset(&sa.sa_mask);
> + sigaction(sig, &sa, NULL);
> +}
> +#else
> +#define signal_set signal
> +#endif
> +
> #endif
>
> static lua_State *globalL = NULL;
> @@ -54,8 +69,8 @@ static void lstop(lua_State *L, lua_Debug *ar)
>
> static void laction(int i)
> {
> - signal(i, SIG_DFL); /* if another SIGINT happens before lstop,
> - terminate process (default action) */
> + /* Terminate process if another SIGINT happens (double CTRL-C). */
> + signal_set(i, SIG_DFL);
> lua_sethook(globalL, lstop, LUA_MASKCALL | LUA_MASKRET | LUA_MASKCOUNT, 1);
> }
> #endif
> @@ -129,11 +144,11 @@ static int docall(lua_State *L, int narg, int clear)
> lua_pushcfunction(L, traceback); /* push traceback function */
> lua_insert(L, base); /* put it under chunk and args */
> #if !LJ_TARGET_CONSOLE
> - signal(SIGINT, laction);
> + signal_set(SIGINT, laction);
> #endif
> status = lua_pcall(L, narg, (clear ? 0 : LUA_MULTRET), base);
> #if !LJ_TARGET_CONSOLE
> - signal(SIGINT, SIG_DFL);
> + signal_set(SIGINT, SIG_DFL);
> #endif
> lua_remove(L, base); /* remove traceback function */
> /* force a complete garbage collection in case of errors */
> diff --git a/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
> new file mode 100644
> index 00000000..4ea316d8
> --- /dev/null
> +++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
> @@ -0,0 +1,17 @@
> +local tap = require('tap')
> +local test = tap.test('lj-noticket-fix-cli-posix-sigint-handling'):skipcond({
> + ['Check LuaJIT CLI signal handling'] = _TARANTOOL,
The skip reason `['Check LuaJIT CLI signal handling'] = _TARANTOOL`
describes
what the test checks, not why it is skipped under Tarantool.
> + ['Disabled on POSIX nonconformant OS'] = jit.os ~= 'BSD' and
> + jit.os ~= 'Linux' and
> + jit.os ~= 'OSX' and
> + jit.os ~= 'POSIX',
the condition can be inverted and contain only two items:
if jit.os == 'Windows' or jit.os == 'Other'
Feel free to ignore.
> +})
> +
It would be great to describe, at least briefly, what the test is
intended to do and what it checks.
> +test:plan(1)
> +local utils = require('utils')
> +local script = utils.exec.makecmd_bash(arg, {t = test})
> +local output = script(
> + utils.exec.luabin(arg),
> + utils.exec.test_data(arg, 'script.lua'))
test_data returns two values and I think it is bad idea to pass both
values to script()
> +test:is(output, 'OK')
test description is missed
> +test:done(true)
> diff --git a/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
> new file mode 100644
> index 00000000..0e6f4078
> --- /dev/null
> +++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
> @@ -0,0 +1,25 @@
> +local ffi = require("ffi")
> +
> +ffi.cdef[[
> + int fork(void);
> + unsigned int sleep(unsigned int seconds);
> + int waitpid(int pid, int *status, int options);
> +]]
> +
> +local pid = ffi.C.fork()
> +
> +if pid == 0 then
> + -- child.
> + ffi.C.sleep(10)
> +elseif pid > 0 then
> + -- parent.
> + local status = ffi.new("int[1]")
> + -- wait() system call: it restarts after signal is handled
> + -- if SA_RESTART flag is set for the handler in sigaction()
> + -- or if old signal() is used and it's OS specific implementation
> + -- is BSD-aware.
> + ffi.C.waitpid(pid, status, 0)
> +else
> + -- fork() failed.
> + os.exit(1)
> +end
> diff --git a/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh
> new file mode 100644
> index 00000000..7fa3c019
> --- /dev/null
> +++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.sh
please check script.sh with shellcheck and fix all found warnings
> @@ -0,0 +1,28 @@
> +LUA_BIN=$1
> +SCRIPT_TO_RUN=$2
> +$LUA_BIN $SCRIPT_TO_RUN 2>/dev/null &
> +pid=$!
> +
> +# Wait for proccess is started and running.
> +sleep 1.5
Using a delay in seconds is a poor method of synchronization. The same
is below.
With these sleeps the test will be a good candidate for flaky.
Also, "1.5" is an outdated value, 3 seconds is used in the branch.
> +if ! kill -0 "$pid" 2>/dev/null; then
> + echo 'ERR'
> + exit 1
> +fi
> +
> +# Send SIGINT and wait for termination.
> +kill -INT "$pid" 2>/dev/null
> +sleep 1.5
The test spends ~10 seconds of pure waiting for a single assertion, and
it is
not only the two `sleep 3`:
- The forked child process inherits the `io.popen()` pipe and keeps its
write
end open until its `sleep(10)` ends, so the harness's `read('*all')`
waits
for the full 10 seconds even when the check at line 18 already says
"terminated". Measured: `ctest -R lj-noticket...` takes 10.01 s, while
`script.sh` alone (stdout to a terminal) returns in 6.00 s.
- The two fixed sleeps make the test sensitive to a loaded CI in both
directions: too short a startup wait gives a false `ERR`, too short a
wait
after SIGINT gives a false `FAIL`.
Would it be more robust to (a) have `script.lua` print a readiness
marker (e.g.
right before `waitpid()`) and let the shell wait for that line instead of
`sleep 3`, (b) poll `kill -0` in a short loop instead of the second
`sleep 3`,
and (c) make the child not hold the pipe (`ffi.C.close(1)` in the child, or
kill/reap it from the parent after the checks)?
> +
> +# Terminated? OK, signal handled -- 'sigaction() behavior'.
> +if ! kill -0 "$pid" 2>/dev/null; then
> + wait "$pid" 2>/dev/null
> + echo 'OK'
> + exit 0
> +fi
> +
> +# Not terminated -- 'signal() BSD-behavior'.
> +kill -KILL "$pid" 2>/dev/null
> +wait "$pid" 2>/dev/null
> +echo 'FAIL'
> +exit 0
The child process created by `script.lua` outlives the parent in both
branches:
on success (line 21) the parent LuaJIT exits and leaves it orphaned for
the rest
of the 10 s sleep; on failure (line 25) only the parent is SIGKILLed. So a
failed run leaves a stray LuaJIT process behind. Could the parent
kill/reap the
child before exiting (or `script.sh` signal the process group) so the test
leaves nothing behind?
> diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua
> index 49f8ef06..f19405a2 100644
> --- a/test/tarantool-tests/utils/exec.lua
> +++ b/test/tarantool-tests/utils/exec.lua
> @@ -43,33 +43,74 @@ local function makeenv(tabenv)
> return table.concat(flatenv, ' ')
> end
>
> --- <makecmd> creates a command that runs %testname%/script.lua by
> --- <LUAJIT_TEST_BINARY> with the given environment, launch options
> --- and CLI arguments. The function yields an object (i.e. table)
> --- with the aforementioned parameters. To launch the command just
> --- call the object.
> -function M.makecmd(arg, opts)
> +-- <makecmd_exec> creates a command that runs given binary with the
> +-- given environment, launch options and zero or more permanent
> +-- arguments.
> +-- The function yields an object (i.e. table) with the aforementioned
> +-- parameters. To launch the command just call the object. Additional
> +-- arguments which may vary from call to call are the `__call` args.
> +-- Pass tap.test() object as opt.t for diagnostic.
s/opt/opts/
> +function M.makecmd_exec(bin, opts)
> + assert(type(bin) == 'string')
> + assert(opts == nil or type(opts) == 'table')
> + local perm_args = opts and opts.args or {}
> + assert(type(perm_args) == 'table')
> return setmetatable({
> - LUABIN = M.luacmd(arg),
> - SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),
> + BIN = bin,
> ENV = opts and makeenv(opts.env) or '',
> REDIRECT = opts and opts.redirect or '',
> + PERMARGS = (' %s'):rep(#perm_args):format(unpack(perm_args)),
> + t = opts and opts.t,
> }, {
> __call = function(self, ...)
> -- This line just makes the command for <io.popen> by the
> -- following steps:
> -- 1. Replace the placeholders with the corresponding values
> - -- given to the command constructor (e.g. script, env).
> + -- given to the command constructor (e.g. bin, env).
> -- 2. Join all CLI arguments given to the __call metamethod.
> -- 3. Concatenate the results of step 1 and step 2 to obtain
> -- the resulting command.
> - local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
> + local cmd = ('<ENV> <BIN> <REDIRECT> <PERMARGS>'):gsub('%<(%w+)>', self)
> .. (' %s'):rep(select('#', ...)):format(...)
> + if self.t ~= nil then self.t:diag('cmd: '..cmd) end
> -- Trim both leading and trailing whitespace from the output
> -- produced by the child process.
> - return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> + local output = io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> + if self.t ~= nil then self.t:diag('output: '..output) end
> + return output
> end
> })
> end
>
> +-- Makes a file name for running test's data file in '%testname%' directory.
> +function M.test_data(arg, name)
> + return arg[0]:gsub('%.test%.lua$', '/'..name)
> +end
> +
> +-- <makecmd_bash> creates a command that runs %testname%/script.sh
> +-- with the given environment, launch options and CLI arguments.
> +function M.makecmd_bash(arg, opts)
> + local script = opts and opts.script or M.test_data(arg, 'script.sh')
> + return M.makecmd_exec('/bin/bash', {
`makecmd_bash()` hardcodes `/bin/bash`, while the test explicitly enables
itself on `jit.os == 'BSD'` and on `'POSIX'`. On FreeBSD, bash is not
part of
the base system (and lives in `/usr/local/bin` when installed), so the test
would produce an empty output and fail there. Since `script.sh` uses
only POSIX
sh (`$!`, `kill`, `wait`, `&`), would `/bin/sh` (and a rename to
`makecmd_sh`)
be enough? The CI matrix is Linux/macOS only, so this is not a CI breaker
today, but the skip condition promises BSD support.
> + env = opts and opts.env or nil,
> + redirect = opts and opts.redirect or nil,
> + args = {script},
> + t = opts and opts.t or nil,
> + })
> +end
> +
> +-- <makecmd> creates a command that runs %testname%/script.lua by
> +-- <LUAJIT_TEST_BINARY> with the given environment, launch options
> +-- and CLI arguments.
> +function M.makecmd(arg, opts)
> + local luabin = M.luacmd(arg)
> + local script = opts and opts.script or M.test_data(arg, 'script.lua')
> + return M.makecmd_exec(luabin, {
> + env = opts and opts.env or nil,
> + redirect = opts and opts.redirect or nil,
> + args = {script},
> + t = opts and opts.t or nil,
> + })
> +end
> +
> return M
[-- Attachment #2: Type: text/html, Size: 21716 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread