Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit v3 0/2] Improve CLI signal handling on POSIX.
@ 2026-11-01 23:59 Evgeniy Temirgaleev via Tarantool-patches
  2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd Evgeniy Temirgaleev via Tarantool-patches
  2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 2/2] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
  0 siblings, 2 replies; 9+ messages in thread
From: Evgeniy Temirgaleev via Tarantool-patches @ 2026-11-01 23:59 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

The changes from v2:
* the patch was splitted into two patches;
* other fixes and suggestions from Sergey Bronnikov's letter at 10/26/26 3:59 PM (https://lists.tarantool.org/pipermail/tarantool-patches/2026-October/030876.html) was applied.

The first patch contains test utils refactoring for the tests in the second patch. The second patch is the target 'Improve CLI signal handling on POSIX.' patch.

Mike Pall (1):
  Improve CLI signal handling on POSIX.

Temir Galeev (1):
  test: refactored utils.exec.makecmd

---

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 | 64 +++++++++++++++++++
 .../script.lua                                | 26 ++++++++
 test/tarantool-tests/utils/exec.lua           | 56 ++++++++++++----
 4 files changed, 154 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

-- 
2.49.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd
  2026-11-01 23:59 [Tarantool-patches] [PATCH luajit v3 0/2] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
@ 2026-11-01 23:59 ` Evgeniy Temirgaleev via Tarantool-patches
  2026-11-03 11:55   ` Sergey Kaplun via Tarantool-patches
  2026-11-06 14:37   ` Sergey Bronnikov via Tarantool-patches
  2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 2/2] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
  1 sibling, 2 replies; 9+ messages in thread
From: Evgeniy Temirgaleev via Tarantool-patches @ 2026-11-01 23:59 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

From: Temir Galeev <temir.galeev@bk.ru>

The 'utils.exec.makecmd()' was refactored to allow executing of the
arbitrary commands. Also, the diagnostic messages may be enabled
if the test object is supplied. See the example below.

Initially this patch was prepared for tests in the follows up
'Improve CLI signal handling on POSIX' patch. After the test
simplification the shell script helper was wiped out, but the
refactoring was already done.

How to use new 'utils.exec.makecmd_exec()' to run arbitrary
shell script:

```
local tap = require('tap')
local test = tap.test('example')
local utils = require('utils')
local script = utils.exec.makecmd_exec('/bin/sh', {
  args = {'-c'},
  redirect = '2>&1',
  t = test, -- shows cmd and it's output for diagnostic
})

test:plan(2)
local output
output = script("'echo 1'")
test:is(output, '1')
output = script("'echo 2'")
test:is(output, '23') -- see the on-failure output
test:done(true)
```
---
 test/tarantool-tests/utils/exec.lua | 56 +++++++++++++++++++++++------
 1 file changed, 45 insertions(+), 11 deletions(-)

diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua
index 49f8ef06..b435e084 100644
--- a/test/tarantool-tests/utils/exec.lua
+++ b/test/tarantool-tests/utils/exec.lua
@@ -43,33 +43,67 @@ 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 opts.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)
+  local fname, matches = arg[0]:gsub('%.test%.lua$', '/'..name)
+  assert(matches == 1, 'unexpected test filename')
+  return fname
+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] 9+ messages in thread

* [Tarantool-patches] [PATCH luajit v3 2/2] Improve CLI signal handling on POSIX.
  2026-11-01 23:59 [Tarantool-patches] [PATCH luajit v3 0/2] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
  2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd Evgeniy Temirgaleev via Tarantool-patches
@ 2026-11-01 23:59 ` Evgeniy Temirgaleev via Tarantool-patches
  2026-11-03 12:14   ` Sergey Kaplun via Tarantool-patches
  1 sibling, 1 reply; 9+ messages in thread
From: Evgeniy Temirgaleev via Tarantool-patches @ 2026-11-01 23:59 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

From: Mike Pall <mike>

(cherry picked from commit 423ac2144b0447c925596f4d93e073ce501ed4e1)

An obsolete signal() function in glibc is 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 latter makes certain
system calls, e.g. read() or wait(), restartable across signals [3].

The patch switches from signal() to sigaction() for POSIX systems
to obtain an explicit control of the signal flag set, which must
not contain SA_RESTART flag. (Actually, the empty flag set is
used.)

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
---
 src/luajit.c                                  | 23 +++++--
 ...ket-fix-cli-posix-sigint-handling.test.lua | 64 +++++++++++++++++++
 .../script.lua                                | 26 ++++++++
 3 files changed, 109 insertions(+), 4 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

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..5b235660
--- /dev/null
+++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
@@ -0,0 +1,64 @@
+local tap = require('tap')
+
+-- The test checks that luajit CLI stop hook is executed on
+-- SIGINT on a blocking system call (read(), waitpid(), etc.).
+--
+-- The test runs luajit CLI with a script, which prints it's PID
+-- as a ready marker and blocks on a read() call. After the
+-- child's PID number is read from child's pipe, the test send
+-- SIGINT to the child.
+-- With a patch applied (no SA_RESTART flag for SIGINT handler)
+-- read() call is interrupted and the CLI's Lua stop hook
+-- executed.
+-- Without a patch (SA_RESTART flag is present for SIGINT
+-- handler) read() call is restarted, the test and the child is
+-- blocked on a pipe read until the child is terminated by
+-- SIGALRM. The test reads nothing from the child and fails.
+
+local test = tap.test('lj-noticket-fix-cli-posix-sigint-handling'):skipcond({
+  -- The patched code is not present in Tarantool binary:
+  ["Disabled due to Tarantool uses it's own signal handling"] = _TARANTOOL,
+  ['Disabled on POSIX nonconformant OS'] = jit.os ~= 'BSD' and
+                                           jit.os ~= 'Linux' and
+                                           jit.os ~= 'OSX' and
+                                           jit.os ~= 'POSIX',
+})
+
+local ffi = require('ffi')
+ffi.cdef[[
+  int kill(int pid, int sig);
+]]
+local SIGINT = 2
+
+test:plan(1)
+local utils = require('utils')
+local script = utils.exec.test_data(arg, 'script.lua')
+local cmd = ('exec %s %s 2>&1'):format(utils.exec.luacmd(arg), script)
+local proc = io.popen(cmd)
+
+local MARKER = 'CHILD_PID:'
+local RMARKER = '^'..MARKER..'(%d+)$'
+-- The child prints its PID after the CLI has installed the SIGINT
+-- handler, so it is a readiness marker. Skip possible noise
+-- lines, collect pre-marker output for error reporting.
+local lines_for_diag = {'child output:'}
+local pid, line
+repeat
+  line = proc:read('*l')
+  if line ~= nil then
+    table.insert(lines_for_diag, line)
+    local match = line:match(RMARKER)
+    if match ~= nil then pid = tonumber(match) end
+  end
+until line == nil or pid ~= nil
+if pid == nil then
+  test:diag(table.concat(lines_for_diag, '\n'))
+end
+assert(pid ~= nil, 'the child process must report its PID')
+
+local res = ffi.C.kill(pid, SIGINT)
+assert(res == 0, 'SIGINT delivery failed')
+local output = proc:read('*a')
+proc:close()
+test:like(output, 'interrupted!', 'SIGINT interrupts the blocking call')
+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..d2430a52
--- /dev/null
+++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
@@ -0,0 +1,26 @@
+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. (SIGALRM isn't handled by CLI.)
+ffi.C.alarm(10)
+
+-- The CLI installs the SIGINT handler before the script
+-- starts, so this line is the readiness marker for the test.
+local MARKER = 'CHILD_PID:'
+io.write(MARKER, 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)
-- 
2.49.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd
  2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd Evgeniy Temirgaleev via Tarantool-patches
@ 2026-11-03 11:55   ` Sergey Kaplun via Tarantool-patches
  2026-11-05 15:24     ` Evgeniy Temirgaleev via Tarantool-patches
  2026-11-06 14:37   ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 9+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2026-11-03 11:55 UTC (permalink / raw)
  To: Evgeniy Temirgaleev; +Cc: tarantool-patches

Hi, Evgeniy!
Thanks for the patch!
Please consider my comments below.

On 02.11.26, Evgeniy Temirgaleev wrote:
> From: Temir Galeev <temir.galeev@bk.ru>
> 
> The 'utils.exec.makecmd()' was refactored to allow executing of the
> arbitrary commands. Also, the diagnostic messages may be enabled
> if the test object is supplied. See the example below.
> 
> Initially this patch was prepared for tests in the follows up

Typo: s/follows up/follow up/

> 'Improve CLI signal handling on POSIX' patch. After the test
> simplification the shell script helper was wiped out, but the

Typo: s/simplification/simplification,/

> refactoring was already done.

This refactoring looks OK, but the second patch doesn't use the newly
introduced function. So it is questionable: do we need it right now?
I don't understand what benefits will we have with it (since the testing
of CLI is a unique case, as you can see).

> 
> How to use new 'utils.exec.makecmd_exec()' to run arbitrary
> shell script:
> 
> ```
> local tap = require('tap')
> local test = tap.test('example')
> local utils = require('utils')
> local script = utils.exec.makecmd_exec('/bin/sh', {
>   args = {'-c'},
>   redirect = '2>&1',
>   t = test, -- shows cmd and it's output for diagnostic
> })
> 
> test:plan(2)
> local output
> output = script("'echo 1'")
> test:is(output, '1')
> output = script("'echo 2'")
> test:is(output, '23') -- see the on-failure output
> test:done(true)
> ```
> ---
>  test/tarantool-tests/utils/exec.lua | 56 +++++++++++++++++++++++------
>  1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua
> index 49f8ef06..b435e084 100644
> --- a/test/tarantool-tests/utils/exec.lua
> +++ b/test/tarantool-tests/utils/exec.lua
> @@ -43,33 +43,67 @@ 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 opts.t for diagnostic.
> +function M.makecmd_exec(bin, opts)

This refactoring looks OK, but the second patch doesn't use the newly
introduced function. So it is questionable: do we need it right now?
If not, I suppose the debug adjustment (and default script value) may be
done in the scope of the old `makecmd()` helper.

> +  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,

Maybe name it tap instead?

>    }, {
>      __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

Minor: IMO, `if self.t then` is more readable, since we don't want to check
its type but only its absence. Feel free to ignore.


Minor: s/../ .. /

>        -- 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

Minor: IMO, `if self.t then` is more readable, since we don't want to
check its type but only its absence. Feel free to ignore.
Minor: s/../ .. /

> +      return output
>      end
>    })
>  end
>  
> +-- Makes a file name for running test's data file in
> +-- '%testname%' directory.
> +function M.test_data(arg, name)

Minor: `test_data()` sounds too general. Maybe `scriptname()` instead?

> +  local fname, matches = arg[0]:gsub('%.test%.lua$', '/'..name)
> +  assert(matches == 1, 'unexpected test filename')
> +  return fname
> +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
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v3 2/2] Improve CLI signal handling on POSIX.
  2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 2/2] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
@ 2026-11-03 12:14   ` Sergey Kaplun via Tarantool-patches
  2026-11-08 22:00     ` Evgeniy Temirgaleev via Tarantool-patches
  0 siblings, 1 reply; 9+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2026-11-03 12:14 UTC (permalink / raw)
  To: Evgeniy Temirgaleev; +Cc: tarantool-patches

Envgeniy,
Thanks for the patch!
LGTM, with minor nits below.

On 02.11.26, Evgeniy Temirgaleev wrote:
> From: Mike Pall <mike>
> 
> (cherry picked from commit 423ac2144b0447c925596f4d93e073ce501ed4e1)
> 
> An obsolete signal() function in glibc is implemented as a wrapper for
> POSIX sigaction() call with BSD semantic. In particular, it provides

Typo: s/semantic/semantics/

> an SA_RESTART flag for SIGINT handler [1-2]. The latter makes certain
> system calls, e.g. read() or wait(), restartable across signals [3].
> 
> The patch switches from signal() to sigaction() for POSIX systems
> to obtain an explicit control of the signal flag set, which must
> not contain SA_RESTART flag. (Actually, the empty flag set is
> used.)
> 
> 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
> ---
>  src/luajit.c                                  | 23 +++++--
>  ...ket-fix-cli-posix-sigint-handling.test.lua | 64 +++++++++++++++++++
>  .../script.lua                                | 26 ++++++++
>  3 files changed, 109 insertions(+), 4 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
> 
> diff --git a/src/luajit.c b/src/luajit.c
> index d9d85361..d35bea54 100644
> --- a/src/luajit.c
> +++ b/src/luajit.c

<snipped>

> 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..5b235660
> --- /dev/null
> +++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
> @@ -0,0 +1,64 @@
> +local tap = require('tap')
> +
> +-- The test checks that luajit CLI stop hook is executed on

Typo: s/luajit/the LuaJIT/

> +-- SIGINT on a blocking system call (read(), waitpid(), etc.).
> +--
> +-- The test runs luajit CLI with a script, which prints it's PID

Typo: s/luajit/the LuaJIT/
Typo: s/it's/its/

> +-- as a ready marker and blocks on a read() call. After the
> +-- child's PID number is read from child's pipe, the test send

Typo: s/child's/the child's/
Typo: s/send/sends/

> +-- SIGINT to the child.
> +-- With a patch applied (no SA_RESTART flag for SIGINT handler)
> +-- read() call is interrupted and the CLI's Lua stop hook
> +-- executed.

Typo: s/executed/is executed/

> +-- Without a patch (SA_RESTART flag is present for SIGINT
> +-- handler) read() call is restarted, the test and the child is

Typo: s/is blocked/are blocked/

> +-- blocked on a pipe read until the child is terminated by
> +-- SIGALRM. The test reads nothing from the child and fails.
> +
> +local test = tap.test('lj-noticket-fix-cli-posix-sigint-handling'):skipcond({
> +  -- The patched code is not present in Tarantool binary:
> +  ["Disabled due to Tarantool uses it's own signal handling"] = _TARANTOOL,

Nit: s/"/'/g

> +  ['Disabled on POSIX nonconformant OS'] = jit.os ~= 'BSD' and
> +                                           jit.os ~= 'Linux' and
> +                                           jit.os ~= 'OSX' and
> +                                           jit.os ~= 'POSIX',
> +})
> +
> +local ffi = require('ffi')
> +ffi.cdef[[
> +  int kill(int pid, int sig);
> +]]
> +local SIGINT = 2
> +
> +test:plan(1)
> +local utils = require('utils')
> +local script = utils.exec.test_data(arg, 'script.lua')
> +local cmd = ('exec %s %s 2>&1'):format(utils.exec.luacmd(arg), script)
> +local proc = io.popen(cmd)
> +
> +local MARKER = 'CHILD_PID:'
> +local RMARKER = '^'..MARKER..'(%d+)$'

Typo: s/../ .. /g

> +-- The child prints its PID after the CLI has installed the SIGINT
> +-- handler, so it is a readiness marker. Skip possible noise
> +-- lines, collect pre-marker output for error reporting.
> +local lines_for_diag = {'child output:'}
> +local pid, line
> +repeat
> +  line = proc:read('*l')
> +  if line ~= nil then
> +    table.insert(lines_for_diag, line)
> +    local match = line:match(RMARKER)
> +    if match ~= nil then pid = tonumber(match) end
> +  end
> +until line == nil or pid ~= nil
> +if pid == nil then
> +  test:diag(table.concat(lines_for_diag, '\n'))
> +end
> +assert(pid ~= nil, 'the child process must report its PID')
> +
> +local res = ffi.C.kill(pid, SIGINT)
> +assert(res == 0, 'SIGINT delivery failed')
> +local output = proc:read('*a')
> +proc:close()
> +test:like(output, 'interrupted!', 'SIGINT interrupts the blocking call')
> +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..d2430a52
> --- /dev/null
> +++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
> @@ -0,0 +1,26 @@
> +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. (SIGALRM isn't handled by CLI.)
> +ffi.C.alarm(10)
> +
> +-- The CLI installs the SIGINT handler before the script
> +-- starts, so this line is the readiness marker for the test.
> +local MARKER = 'CHILD_PID:'
> +io.write(MARKER, 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)

Why don't use `io.read()` here? Same semantics but additional
correlation with the comment in sources.

> -- 
> 2.49.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd
  2026-11-03 11:55   ` Sergey Kaplun via Tarantool-patches
@ 2026-11-05 15:24     ` Evgeniy Temirgaleev via Tarantool-patches
  0 siblings, 0 replies; 9+ messages in thread
From: Evgeniy Temirgaleev via Tarantool-patches @ 2026-11-05 15:24 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Hi, Sergey! Thanks for review!

Please, see my answers. Fixes are made and force pushed.

On 11/3/26 2:55 PM, Sergey Kaplun wrote:
> Hi, Evgeniy!
> Thanks for the patch!
> Please consider my comments below.
> 
> On 02.11.26, Evgeniy Temirgaleev wrote:
>> From: Temir Galeev <temir.galeev@bk.ru>
>>
>> The 'utils.exec.makecmd()' was refactored to allow executing of the
>> arbitrary commands. Also, the diagnostic messages may be enabled
>> if the test object is supplied. See the example below.
>>
>> Initially this patch was prepared for tests in the follows up
> 
> Typo: s/follows up/follow up/
> 
Fixed.

>> 'Improve CLI signal handling on POSIX' patch. After the test
>> simplification the shell script helper was wiped out, but the
> 
> Typo: s/simplification/simplification,/
> 
Fixed.

>> refactoring was already done.
> 
> This refactoring looks OK, but the second patch doesn't use the newly
> introduced function. So it is questionable: do we need it right now?
> I don't understand what benefits will we have with it (since the testing
> of CLI is a unique case, as you can see).
> 
The other tests uses the 'utils.exec.makecmd()' func. The refactoring 
makes it's code more clear to understand and in consequence more easy to 
use.

We can postpone the export of 'makecmd_exec()' function until it will be 
really need, but the refactoring is a part of work which is wasteful to 
reject due to it is done already.

>>
>> How to use new 'utils.exec.makecmd_exec()' to run arbitrary
>> shell script:
>>
>> ```
>> local tap = require('tap')
>> local test = tap.test('example')
>> local utils = require('utils')
>> local script = utils.exec.makecmd_exec('/bin/sh', {
>>    args = {'-c'},
>>    redirect = '2>&1',
>>    t = test, -- shows cmd and it's output for diagnostic
>> })
>>
>> test:plan(2)
>> local output
>> output = script("'echo 1'")
>> test:is(output, '1')
>> output = script("'echo 2'")
>> test:is(output, '23') -- see the on-failure output
>> test:done(true)
>> ```
>> ---
>>   test/tarantool-tests/utils/exec.lua | 56 +++++++++++++++++++++++------
>>   1 file changed, 45 insertions(+), 11 deletions(-)
>>
>> diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua
>> index 49f8ef06..b435e084 100644
>> --- a/test/tarantool-tests/utils/exec.lua
>> +++ b/test/tarantool-tests/utils/exec.lua
>> @@ -43,33 +43,67 @@ 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 opts.t for diagnostic.
>> +function M.makecmd_exec(bin, opts)
> 
> This refactoring looks OK, but the second patch doesn't use the newly
> introduced function. So it is questionable: do we need it right now?
> If not, I suppose the debug adjustment (and default script value) may be
> done in the scope of the old `makecmd()` helper.
> 
The same.

>> +  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,
> 
> Maybe name it tap instead?
> 
Renamed.

>>     }, {
>>       __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
> 
> Minor: IMO, `if self.t then` is more readable, since we don't want to check
> its type but only its absence. Feel free to ignore.
> 
> 
> Minor: s/../ .. /
> 
I try to follow Tarantool's "Lua style guide" [1], where implicit 
casting to bool is prohibited.

1. 
https://www.tarantool.io/en/doc/latest/contributing/lua_style_guide/#idioms-and-patterns

Dots: fixed.

>>         -- 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
> 
> Minor: IMO, `if self.t then` is more readable, since we don't want to
> check its type but only its absence. Feel free to ignore.
> Minor: s/../ .. /
> 
The same.

>> +      return output
>>       end
>>     })
>>   end
>>   
>> +-- Makes a file name for running test's data file in
>> +-- '%testname%' directory.
>> +function M.test_data(arg, name)
> 
> Minor: `test_data()` sounds too general. Maybe `scriptname()` instead?
> 
When we need to store some other input data for the test, will we rename 
or will we make one more function?

>> +  local fname, matches = arg[0]:gsub('%.test%.lua$', '/'..name)
>> +  assert(matches == 1, 'unexpected test filename')
>> +  return fname
>> +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
>>
> 

---

test: refactored utils.exec.makecmd
...
Initially this patch was prepared for tests in the follow up
'Improve CLI signal handling on POSIX' patch. After the test
simplification, the shell script helper was wiped out, but the
refactoring was already done.
...

---

diff --git a/test/tarantool-tests/utils/exec.lua 
b/test/tarantool-tests/utils/exec.lua
index b435e084..b202d384 100644
--- a/test/tarantool-tests/utils/exec.lua
+++ b/test/tarantool-tests/utils/exec.lua
@@ -50,7 +50,7 @@ end
  -- 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 opts.t for diagnostic.
+-- Pass tap.test() object as opts.tap for diagnostic.
  function M.makecmd_exec(bin, opts)
    assert(type(bin) == 'string')
    assert(opts == nil or type(opts) == 'table')
@@ -61,7 +61,7 @@ function M.makecmd_exec(bin, opts)
      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,
+    tap = opts and opts.tap,
    }, {
      __call = function(self, ...)
        -- This line just makes the command for <io.popen> by the
@@ -73,12 +73,12 @@ function M.makecmd_exec(bin, opts)
        --    the resulting command.
        local cmd = ('<ENV> <BIN> <REDIRECT> 
<PERMARGS>'):gsub('%<(%w+)>', self)
                    .. (' %s'):rep(select('#', ...)):format(...)
-      if self.t ~= nil then self.t:diag('cmd: '..cmd) end
+      if self.tap ~= nil then self.tap:diag('cmd: ' .. cmd) end
        -- Trim both leading and trailing whitespace from the output
        -- produced by the child process.
        local output = io.popen(cmd):
          read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
-      if self.t ~= nil then self.t:diag('output: '..output) end
+      if self.tap ~= nil then self.tap:diag('output: ' .. output) end
        return output
      end
    })
@@ -102,7 +102,7 @@ function M.makecmd(arg, opts)
      env = opts and opts.env or nil,
      redirect = opts and opts.redirect or nil,
      args = {script},
-    t = opts and opts.t or nil,
+    tap = opts and opts.tap or nil,
    })
  end

-- 
--
Best regards,
Evgeniy Temirgaleev


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd
  2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd Evgeniy Temirgaleev via Tarantool-patches
  2026-11-03 11:55   ` Sergey Kaplun via Tarantool-patches
@ 2026-11-06 14:37   ` Sergey Bronnikov via Tarantool-patches
  2026-11-09  2:16     ` Evgeniy Temirgaleev via Tarantool-patches
  1 sibling, 1 reply; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2026-11-06 14:37 UTC (permalink / raw)
  To: Evgeniy Temirgaleev, Sergey Kaplun; +Cc: tarantool-patches

Hi,

please see my comment below.

 >  test: refactored utils.exec.makecmd

Tarantool's developers guide asks for an imperative title; "refactored" 
is past tense.
Would `test: refactor utils.exec.makecmd` be better?

Sergey

On 11/2/26 02:59, Evgeniy Temirgaleev wrote:
> From: Temir Galeev <temir.galeev@bk.ru>
>
> The 'utils.exec.makecmd()' was refactored to allow executing of the
> arbitrary commands. Also, the diagnostic messages may be enabled
s/allow executing of the arbitrary commands/allow executing arbitrary 
commands/
> if the test object is supplied. See the example below.
>
> Initially this patch was prepared for tests in the follows up
> 'Improve CLI signal handling on POSIX' patch. After the test
> simplification the shell script helper was wiped out, but the
> refactoring was already done.
  The new `test_data()` helper and the changed `makecmd()` object fields are
   not mentioned. Nothing in the tree reads the removed `LUABIN`/`SCRIPT`
   (checked), so the change is safe, but it is an interface change worth a
   line in the message.
>
> How to use new 'utils.exec.makecmd_exec()' to run arbitrary
> shell script:
s/arbitrary/an arbitrary/
>
> ```
> local tap = require('tap')
> local test = tap.test('example')
> local utils = require('utils')
> local script = utils.exec.makecmd_exec('/bin/sh', {
>    args = {'-c'},
>    redirect = '2>&1',
>    t = test, -- shows cmd and it's output for diagnostic
> })
typo: s/t =/tap =/
>
> test:plan(2)
> local output
> output = script("'echo 1'")
> test:is(output, '1')
> output = script("'echo 2'")
> test:is(output, '23') -- see the on-failure output
> test:done(true)
> ```

Usually we don't put code examples to the commit messages.


> ---
>   test/tarantool-tests/utils/exec.lua | 56 +++++++++++++++++++++++------
>   1 file changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua
> index 49f8ef06..b435e084 100644
> --- a/test/tarantool-tests/utils/exec.lua
> +++ b/test/tarantool-tests/utils/exec.lua
> @@ -43,33 +43,67 @@ 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 opts.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)),
`PERMARGS` already starts each argument with a space while the template also
has a space before `<PERMARGS>`: `makecmd()` now builds ` luabin   script`
(three spaces) instead of the old two. Harmless for the shell, but the
command is printed in the diagnostics now, so the extra whitespace is
user-visible. Would letting one place own the separator -- e.g.
`PERMARGS = table.concat(perm_args, ' ')` with the template's space kept --
read simpler and produce a clean command line?
> +    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
`tap.diag()` treats its first argument as a format string. Passing
`'cmd: ' .. cmd` / `'output: ' .. output` means any `%` in the command or in
the child output either throws or is interpreted as a conversion. Minimal
reproduction:


luajit -e 'local e = dofile("./test/tarantool-tests/utils/exec.lua")
   local t = {diag = function(_, fmt, ...) return fmt:format(...) end}
   e.makecmd_exec("/bin/echo", {args = {"%"}, tap = t})("100%")'
luajit: ./test/tarantool-tests/utils/exec.lua:76: invalid option '% 100' 
to 'format'
stack traceback:
         [C]: in function 'diag'
         ./test/tarantool-tests/utils/exec.lua:76: in function 
<./test/tarantool-tests/utils/exec.lua:66>
         (command line):3: in main chunk
         [C]: at 0x560209573120


A test that prints `100%`, or a call like `script("'echo 100%'")`, will make
the utility error out instead of printing the diagnostic. `tap.lua` itself
uses the safe form (`test:diag("%s", test.name)`); would
`self.tap:diag('%s', 'cmd: ' .. cmd)` and
`self.tap:diag('%s', 'output: ' .. output)` fix it?


Also, I don't like mixing tap helpers with code helpers. I would replace 
diagnostics by assertions.

>         -- 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)
> +  local fname, matches = arg[0]:gsub('%.test%.lua$', '/'..name)
> +  assert(matches == 1, 'unexpected test filename')
> +  return fname
> +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

test/tarantool-tests/lj-962-stack-overflow-report.test.lua

Now that `utils.exec.test_data()` exists, this test still computes the
script path manually with `arg[0]:gsub('%.test%.lua$', '/script.lua')`.
Would switching it to `test_data(arg, 'script.lua')` keep the script-file
naming convention in one place? This is the only remaining manual 
occurrence.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v3 2/2] Improve CLI signal handling on POSIX.
  2026-11-03 12:14   ` Sergey Kaplun via Tarantool-patches
@ 2026-11-08 22:00     ` Evgeniy Temirgaleev via Tarantool-patches
  0 siblings, 0 replies; 9+ messages in thread
From: Evgeniy Temirgaleev via Tarantool-patches @ 2026-11-08 22:00 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey, thanks for review!

Please, see my answers below. Fixes are force pushed to the branch.

On 11/3/26 3:14 PM, Sergey Kaplun wrote:
> Envgeniy,
> Thanks for the patch!
> LGTM, with minor nits below.
> 
> On 02.11.26, Evgeniy Temirgaleev wrote:
>> From: Mike Pall <mike>
>>
>> (cherry picked from commit 423ac2144b0447c925596f4d93e073ce501ed4e1)
>>
>> An obsolete signal() function in glibc is implemented as a wrapper for
>> POSIX sigaction() call with BSD semantic. In particular, it provides
> 
> Typo: s/semantic/semantics/
> 
Fixed.

>> an SA_RESTART flag for SIGINT handler [1-2]. The latter makes certain
>> system calls, e.g. read() or wait(), restartable across signals [3].
>>
>> The patch switches from signal() to sigaction() for POSIX systems
>> to obtain an explicit control of the signal flag set, which must
>> not contain SA_RESTART flag. (Actually, the empty flag set is
>> used.)
>>
>> 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
>> ---
>>   src/luajit.c                                  | 23 +++++--
>>   ...ket-fix-cli-posix-sigint-handling.test.lua | 64 +++++++++++++++++++
>>   .../script.lua                                | 26 ++++++++
>>   3 files changed, 109 insertions(+), 4 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
>>
>> diff --git a/src/luajit.c b/src/luajit.c
>> index d9d85361..d35bea54 100644
>> --- a/src/luajit.c
>> +++ b/src/luajit.c
> 
> <snipped>
> 
>> 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..5b235660
>> --- /dev/null
>> +++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling.test.lua
>> @@ -0,0 +1,64 @@
>> +local tap = require('tap')
>> +
>> +-- The test checks that luajit CLI stop hook is executed on
> 
> Typo: s/luajit/the LuaJIT/
> 
Fixed

>> +-- SIGINT on a blocking system call (read(), waitpid(), etc.).
>> +--
>> +-- The test runs luajit CLI with a script, which prints it's PID
> 
> Typo: s/luajit/the LuaJIT/
> Typo: s/it's/its/
> 
Fixed

>> +-- as a ready marker and blocks on a read() call. After the
>> +-- child's PID number is read from child's pipe, the test send
> 
> Typo: s/child's/the child's/
> Typo: s/send/sends/
> 
Fixed

>> +-- SIGINT to the child.
>> +-- With a patch applied (no SA_RESTART flag for SIGINT handler)
>> +-- read() call is interrupted and the CLI's Lua stop hook
>> +-- executed.
> 
> Typo: s/executed/is executed/
>
Fixed


>> +-- Without a patch (SA_RESTART flag is present for SIGINT
>> +-- handler) read() call is restarted, the test and the child is
> 
> Typo: s/is blocked/are blocked/
> 
Fixed

>> +-- blocked on a pipe read until the child is terminated by
>> +-- SIGALRM. The test reads nothing from the child and fails.
>> +
>> +local test = tap.test('lj-noticket-fix-cli-posix-sigint-handling'):skipcond({
>> +  -- The patched code is not present in Tarantool binary:
>> +  ["Disabled due to Tarantool uses it's own signal handling"] = _TARANTOOL,
> 
> Nit: s/"/'/g
> 
Here is a single quite inside a string.

>> +  ['Disabled on POSIX nonconformant OS'] = jit.os ~= 'BSD' and
>> +                                           jit.os ~= 'Linux' and
>> +                                           jit.os ~= 'OSX' and
>> +                                           jit.os ~= 'POSIX',
>> +})
>> +
>> +local ffi = require('ffi')
>> +ffi.cdef[[
>> +  int kill(int pid, int sig);
>> +]]
>> +local SIGINT = 2
>> +
>> +test:plan(1)
>> +local utils = require('utils')
>> +local script = utils.exec.test_data(arg, 'script.lua')
>> +local cmd = ('exec %s %s 2>&1'):format(utils.exec.luacmd(arg), script)
>> +local proc = io.popen(cmd)
>> +
>> +local MARKER = 'CHILD_PID:'
>> +local RMARKER = '^'..MARKER..'(%d+)$'
> 
> Typo: s/../ .. /g
> 
Fixed.

>> +-- The child prints its PID after the CLI has installed the SIGINT
>> +-- handler, so it is a readiness marker. Skip possible noise
>> +-- lines, collect pre-marker output for error reporting.
>> +local lines_for_diag = {'child output:'}
>> +local pid, line
>> +repeat
>> +  line = proc:read('*l')
>> +  if line ~= nil then
>> +    table.insert(lines_for_diag, line)
>> +    local match = line:match(RMARKER)
>> +    if match ~= nil then pid = tonumber(match) end
>> +  end
>> +until line == nil or pid ~= nil
>> +if pid == nil then
>> +  test:diag(table.concat(lines_for_diag, '\n'))
>> +end
>> +assert(pid ~= nil, 'the child process must report its PID')
>> +
>> +local res = ffi.C.kill(pid, SIGINT)
>> +assert(res == 0, 'SIGINT delivery failed')
>> +local output = proc:read('*a')
>> +proc:close()
>> +test:like(output, 'interrupted!', 'SIGINT interrupts the blocking call')
>> +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..d2430a52
>> --- /dev/null
>> +++ b/test/tarantool-tests/lj-noticket-fix-cli-posix-sigint-handling/script.lua
>> @@ -0,0 +1,26 @@
>> +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. (SIGALRM isn't handled by CLI.)
>> +ffi.C.alarm(10)
>> +
>> +-- The CLI installs the SIGINT handler before the script
>> +-- starts, so this line is the readiness marker for the test.
>> +local MARKER = 'CHILD_PID:'
>> +io.write(MARKER, 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)
> 
> Why don't use `io.read()` here? Same semantics but additional
> correlation with the comment in sources.
> 
`io.read()` may not block in non console environment. Added a comment.

>> -- 
>> 2.49.0
>>
> 

---
Commit msg:

...
An obsolete signal() function in glibc is implemented as a wrapper for
POSIX sigaction() call with BSD semantics. In particular, it provides
an SA_RESTART flag for SIGINT handler [1-2]. The latter makes certain
system calls, e.g. read() or wait(), restartable across signals [3].
...

---

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
index 5b235660..e080b7f3 100644
--- 
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
@@ -1,17 +1,17 @@
  local tap = require('tap')

--- The test checks that luajit CLI stop hook is executed on
+-- The test checks that LuaJIT CLI stop hook is executed on
  -- SIGINT on a blocking system call (read(), waitpid(), etc.).
  --
--- The test runs luajit CLI with a script, which prints it's PID
+-- The test runs LuaJIT CLI with a script, which prints its PID
  -- as a ready marker and blocks on a read() call. After the
--- child's PID number is read from child's pipe, the test send
--- SIGINT to the child.
+-- the child's PID number is read from child's pipe, the test
+-- sends SIGINT to the child.
  -- With a patch applied (no SA_RESTART flag for SIGINT handler)
  -- read() call is interrupted and the CLI's Lua stop hook
--- executed.
+-- is executed.
  -- Without a patch (SA_RESTART flag is present for SIGINT
--- handler) read() call is restarted, the test and the child is
+-- handler) read() call is restarted, the test and the child are
  -- blocked on a pipe read until the child is terminated by
  -- SIGALRM. The test reads nothing from the child and fails.

@@ -37,7 +37,7 @@ local cmd = ('exec %s %s 
2>&1'):format(utils.exec.luacmd(arg), script)
  local proc = io.popen(cmd)

  local MARKER = 'CHILD_PID:'
-local RMARKER = '^'..MARKER..'(%d+)$'
+local RMARKER = '^' .. MARKER .. '(%d+)$'
  -- The child prints its PID after the CLI has installed the SIGINT
  -- handler, so it is a readiness marker. Skip possible noise
  -- lines, collect pre-marker output for error reporting.
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
index d2430a52..fab558e7 100644
--- 
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
@@ -1,15 +1,10 @@
  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);
  ]]
+
+-- The script runs via 'popen()' with closed pipe read-end,
+-- and the stdin is not guaranteed to be connected somewhere.
+-- Reading from it may result in EOF.
+-- So, establish the new pipe, which read-end would block.
  local fds = ffi.new('int[2]')
  assert(ffi.C.pipe(fds) == 0)

  -- Fail instead of hanging forever if SIGINT does not break
  -- the call. (SIGALRM isn't handled by CLI.)
  ffi.C.alarm(10)

-- 
--
Best regards,
Evgeniy Temirgaleev


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd
  2026-11-06 14:37   ` Sergey Bronnikov via Tarantool-patches
@ 2026-11-09  2:16     ` Evgeniy Temirgaleev via Tarantool-patches
  0 siblings, 0 replies; 9+ messages in thread
From: Evgeniy Temirgaleev via Tarantool-patches @ 2026-11-09  2:16 UTC (permalink / raw)
  To: Sergey Bronnikov, Sergey Kaplun; +Cc: tarantool-patches

Thanks for the comments! Please see my answers and fixes.

The fixes was force pushed to the branch.

New changes:
- two tests refactored to use utils.exec;
- one of them uses new utils.exec.makecmd_exec() -> example removed from 
commit message;
- args type checks in the 'exec' module slightly refactored.

On 11/6/26 5:37 PM, Sergey Bronnikov wrote:
> Hi,
> 
> please see my comment below.
> 
>  >  test: refactored utils.exec.makecmd
> 
> Tarantool's developers guide asks for an imperative title; "refactored" 
> is past tense.
> Would `test: refactor utils.exec.makecmd` be better?
> 
Fixed.

> Sergey
> 
> On 11/2/26 02:59, Evgeniy Temirgaleev wrote:
>> From: Temir Galeev <temir.galeev@bk.ru>
>>
>> The 'utils.exec.makecmd()' was refactored to allow executing of the
>> arbitrary commands. Also, the diagnostic messages may be enabled
> s/allow executing of the arbitrary commands/allow executing arbitrary 
> commands/
Rewritten.

>> if the test object is supplied. See the example below.
>>
>> Initially this patch was prepared for tests in the follows up
>> 'Improve CLI signal handling on POSIX' patch. After the test
>> simplification the shell script helper was wiped out, but the
>> refactoring was already done.
>   The new `test_data()` helper and the changed `makecmd()` object fields 
> are
>    not mentioned. Nothing in the tree reads the removed `LUABIN`/`SCRIPT`
>    (checked), so the change is safe, but it is an interface change worth a
>    line in the message.
Rewritten.

>>
>> How to use new 'utils.exec.makecmd_exec()' to run arbitrary
>> shell script:
> s/arbitrary/an arbitrary/
Removed.

>>
>> ```
>> local tap = require('tap')
>> local test = tap.test('example')
>> local utils = require('utils')
>> local script = utils.exec.makecmd_exec('/bin/sh', {
>>    args = {'-c'},
>>    redirect = '2>&1',
>>    t = test, -- shows cmd and it's output for diagnostic
>> })
> typo: s/t =/tap =/
Removed.

>>
>> test:plan(2)
>> local output
>> output = script("'echo 1'")
>> test:is(output, '1')
>> output = script("'echo 2'")
>> test:is(output, '23') -- see the on-failure output
>> test:done(true)
>> ```
> 
> Usually we don't put code examples to the commit messages.
> 
> 
Yes, it was an exceptional case. Removed due to the example appears in 
the code (refactored 
'test/tarantool-tests/lj-586-debug-non-string-error.test.lua' test).

>> ---
>>   test/tarantool-tests/utils/exec.lua | 56 +++++++++++++++++++++++------
>>   1 file changed, 45 insertions(+), 11 deletions(-)
>>
>> diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool- 
>> tests/utils/exec.lua
>> index 49f8ef06..b435e084 100644
>> --- a/test/tarantool-tests/utils/exec.lua
>> +++ b/test/tarantool-tests/utils/exec.lua
>> @@ -43,33 +43,67 @@ 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 opts.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)),
> `PERMARGS` already starts each argument with a space while the template 
> also
> has a space before `<PERMARGS>`: `makecmd()` now builds ` luabin   script`
> (three spaces) instead of the old two. Harmless for the shell, but the
> command is printed in the diagnostics now, so the extra whitespace is
> user-visible. Would letting one place own the separator -- e.g.
> `PERMARGS = table.concat(perm_args, ' ')` with the template's space kept --
> read simpler and produce a clean command line?
Yes! Done.

>> +    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
> `tap.diag()` treats its first argument as a format string. Passing
> `'cmd: ' .. cmd` / `'output: ' .. output` means any `%` in the command 
> or in
> the child output either throws or is interpreted as a conversion. Minimal
> reproduction:
> 
> 
> luajit -e 'local e = dofile("./test/tarantool-tests/utils/exec.lua")
>    local t = {diag = function(_, fmt, ...) return fmt:format(...) end}
>    e.makecmd_exec("/bin/echo", {args = {"%"}, tap = t})("100%")'
> luajit: ./test/tarantool-tests/utils/exec.lua:76: invalid option '% 100' 
> to 'format'
> stack traceback:
>          [C]: in function 'diag'
>          ./test/tarantool-tests/utils/exec.lua:76: in function <./test/ 
> tarantool-tests/utils/exec.lua:66>
>          (command line):3: in main chunk
>          [C]: at 0x560209573120
> 
> 
> A test that prints `100%`, or a call like `script("'echo 100%'")`, will 
> make
> the utility error out instead of printing the diagnostic. `tap.lua` itself
> uses the safe form (`test:diag("%s", test.name)`); would
> `self.tap:diag('%s', 'cmd: ' .. cmd)` and
> `self.tap:diag('%s', 'output: ' .. output)` fix it?
Fixed in new `makecmd_tap_trace()` helper.

> 
> Also, I don't like mixing tap helpers with code helpers.
It's a point. Made a common tracer plus `makecmd_tap_trace()` helper to 
redirect the data to tap output.

> I would replace diagnostics by assertions.
> 
Assertions may clog the test code, and it seems they are not helpful for 
any case. I think, they may be omitted for generated commands for test 
purposes (we expect the `makecmd_exec()` and the test code is made 
good). But the generated commands may be helpful in the log for the new 
reader, which tries to understand the tests. Or the reader must have a 
possibility to enable such log lines easy for discovery purposes.

>>         -- 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)
>> +  local fname, matches = arg[0]:gsub('%.test%.lua$', '/'..name)
>> +  assert(matches == 1, 'unexpected test filename')
>> +  return fname
>> +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
> 
> test/tarantool-tests/lj-962-stack-overflow-report.test.lua
> 
> Now that `utils.exec.test_data()` exists, this test still computes the
> script path manually with `arg[0]:gsub('%.test%.lua$', '/script.lua')`.
> Would switching it to `test_data(arg, 'script.lua')` keep the script-file
> naming convention in one place? This is the only remaining manual 
> occurrence.
Refactored this test to use `makecmd()` function.

Also refactored 
'test/tarantool-tests/lj-586-debug-non-string-error.test.lua' to use 
common `makecmd_exec()` function.

---

Rewritten commit message:

test: refactor utils.exec.makecmd

The 'utils.exec.makecmd()' was refactored. The common
'utils.exec.makecmd_exec()' added to allow executing arbitrary
commands and the 'makecmd()' is implemented on top of it.
The command object's 'LUABIN' field renamed to 'BIN', the
'SCRIPT' field with script filename replaced with 'PERMARGS'
field, containing all command arguments, which are passed on
every call, and the 'trace' callback option is added for
additional processing of the generated command and it
output.

Also added the 'makecmd_tap_trace()' and 'test_data()' helpers.

Test [1] was refactored to use 'makecmd()' as usual.
Test [2] was refactored to use common 'makecmd_exec()'.

1. test/tarantool-tests/lj-962-stack-overflow-report.test.lua
2. test/tarantool-tests/lj-586-debug-non-string-error.test.lua

Initially the patch for exec utils was prepared for tests in
the follow up 'Improve CLI signal handling on POSIX' patch.
After it's test simplification, the shell script helper was
wiped out, but the refactoring was already done.

---

diff --git a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua 
b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
index 67869708..570ea484 100644
--- a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
+++ b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
@@ -6,20 +6,18 @@ test:plan(1)

  -- XXX: This is quite an exotic test case in that point of view
  -- that testing the debug interactive interface always ends with
--- sending commands to another instance via stdin. However, the
--- module with test helpers lacks the suitable routine.
--- `utils.makecmd()` doesn't fit for this, since `debug.debug()`
--- captures `io.stdin` and waits at `fgets()` in debug busy loop.
--- As it's already mentioned, such tests are not usual, so there
--- is no need to introduce a new helper to utils module (at least
--- for now). Hence, we use `utils.luacmd()` function to obtain the
--- interpreter command, run `debug.debug()` with it, send two
--- commands via pipeline to the launched process, capture its
--- stdout and stderr and test the resulting output against the
--- expected one.
+-- sending commands to another instance via stdin.
+-- Since `debug.debug()` captures `io.stdin` and waits at
+-- `fgets()` in debug busy loop, we use `utils.exec.luacmd()`
+-- function to obtain the interpreter command, run
+-- `debug.debug()` with it, send two commands from the
+-- `utils.exec.test_data('commands.lua')` file redirecting it to
+-- the process stdin, capture its stdout and stderr and test the
+-- resulting output against the expected one.
  --
  -- Debugger prompt.
  local ldb = 'lua_debug> '
+-- The magic number is passed via MAGIC environment variable.
  local magic = 42
  local luabin = utils.exec.luacmd(arg)
  local stderr = {
@@ -37,11 +35,13 @@ local stderr = {
  -- errors are yielded before the magic given to `print()` command
  local expected = table.concat(stderr)..magic

-local cmd = ([[cat <<EOF | %s -e 'debug.debug()' 2>&1
-error({})
-print(%d)
-EOF]]):format(luabin, magic)
-
-local res = io.popen(cmd):read('*all'):gsub('%s+$', '')
+local dbg_commands_file = utils.exec.test_data(arg, 'commands.lua')
+local command = utils.exec.makecmd_exec(luabin, {
+  env = {MAGIC = magic},
+  redirect = ('2>&1 <%s'):format(dbg_commands_file),
+  args = {'-e', "'debug.debug()'"},
+  trace = utils.exec.makecmd_tap_trace(test),
+})
+local res = command()
  test:ok(res == expected, 'handle non-string error in debug.debug()')
  test:done(true)
diff --git 
a/test/tarantool-tests/lj-586-debug-non-string-error/commands.lua 
b/test/tarantool-tests/lj-586-debug-non-string-error/commands.lua
new file mode 100644
index 00000000..a27aaee7
--- /dev/null
+++ b/test/tarantool-tests/lj-586-debug-non-string-error/commands.lua
@@ -0,0 +1,2 @@
+error({})
+print(tonumber(os.getenv('MAGIC')))
diff --git a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua 
b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
index 848f4ead..14ffea10 100644
--- a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
+++ b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
@@ -4,8 +4,8 @@ local tap = require('tap')
  local test = tap.test('lj-962-stack-overflow-report')
  test:plan(1)

-local LUABIN = require('utils').exec.luabin(arg)
-local SCRIPT = arg[0]:gsub('%.test%.lua$', '/script.lua')
-local output = io.popen(LUABIN .. ' 2>&1 ' .. SCRIPT, 'r'):read('*all')
+local utils = require('utils')
+local script = utils.exec.makecmd(arg, {redirect = '2>&1'})
+local output = script()
  test:like(output, 'stack overflow', 'stack overflow reported correctly')
  test:done(true)
diff --git a/test/tarantool-tests/utils/exec.lua 
b/test/tarantool-tests/utils/exec.lua
index b202d384..1b79bd63 100644
--- a/test/tarantool-tests/utils/exec.lua
+++ b/test/tarantool-tests/utils/exec.lua
@@ -22,6 +22,7 @@ end

  local function makeenv(tabenv)
    if tabenv == nil then return '' end
+  assert(type(tabenv) == 'table')
    local flatenv = {}
    for var, value in pairs(tabenv) do
      if var == 'LD_PRELOAD' then
@@ -50,18 +51,23 @@ end
  -- 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 opts.tap for diagnostic.
+-- Pass opts.trace function(event, value) for additional
+-- command/output processing.
  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 {}
+  opts = opts or {}
+  assert(type(opts) == 'table')
+  local redirect = opts.redirect or ''
+  assert(type(redirect) == 'string')
+  local perm_args = opts.args or {}
    assert(type(perm_args) == 'table')
+  assert(opts.trace == nil or type(opts.trace) == 'function')
    return setmetatable({
      BIN = bin,
-    ENV = opts and makeenv(opts.env) or '',
-    REDIRECT = opts and opts.redirect or '',
-    PERMARGS = (' %s'):rep(#perm_args):format(unpack(perm_args)),
-    tap = opts and opts.tap,
+    ENV = makeenv(opts.env),
+    REDIRECT = redirect,
+    PERMARGS = table.concat(perm_args, ' '),
+    trace = opts.trace,
    }, {
      __call = function(self, ...)
        -- This line just makes the command for <io.popen> by the
@@ -73,17 +79,23 @@ function M.makecmd_exec(bin, opts)
        --    the resulting command.
        local cmd = ('<ENV> <BIN> <REDIRECT> 
<PERMARGS>'):gsub('%<(%w+)>', self)
                    .. (' %s'):rep(select('#', ...)):format(...)
-      if self.tap ~= nil then self.tap:diag('cmd: ' .. cmd) end
+      if self.trace ~= nil then self.trace('cmd', cmd) end
        -- Trim both leading and trailing whitespace from the output
        -- produced by the child process.
        local output = io.popen(cmd):
          read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
-      if self.tap ~= nil then self.tap:diag('output: ' .. output) end
+      if self.trace ~= nil then self.trace('output', output) end
        return output
      end
    })
  end

+-- Make a trace function for `makecmd_exec` script, which produces
+-- tap test diagnostic messages with the command and it's output.
+function M.makecmd_tap_trace(tap)
+  return function(ev, val) tap:diag('%s: %s', ev, val) end
+end
+
  -- Makes a file name for running test's data file in
  -- '%testname%' directory.
  function M.test_data(arg, name)
@@ -102,7 +114,7 @@ function M.makecmd(arg, opts)
      env = opts and opts.env or nil,
      redirect = opts and opts.redirect or nil,
      args = {script},
-    tap = opts and opts.tap or nil,
+    trace = opts and opts.trace or nil,
    })
  end

-- 
--
Best regards,
Evgeniy Temirgaleev


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-11-09  2:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-01 23:59 [Tarantool-patches] [PATCH luajit v3 0/2] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 1/2] test: refactored utils.exec.makecmd Evgeniy Temirgaleev via Tarantool-patches
2026-11-03 11:55   ` Sergey Kaplun via Tarantool-patches
2026-11-05 15:24     ` Evgeniy Temirgaleev via Tarantool-patches
2026-11-06 14:37   ` Sergey Bronnikov via Tarantool-patches
2026-11-09  2:16     ` Evgeniy Temirgaleev via Tarantool-patches
2026-11-01 23:59 ` [Tarantool-patches] [PATCH luajit v3 2/2] Improve CLI signal handling on POSIX Evgeniy Temirgaleev via Tarantool-patches
2026-11-03 12:14   ` Sergey Kaplun via Tarantool-patches
2026-11-08 22:00     ` Evgeniy Temirgaleev via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox