[Tarantool-patches] [PATCH luajit] Improve CLI signal handling on POSIX.

[Tarantool-patches] [PATCH luajit] Improve CLI signal handling on POSIX.

[Evgeniy Temirgaleev via Tarantool-patches]

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

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..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
@@ -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
+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
+
+# 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..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.
+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

[Tarantool-patches] [PATCH luajit v2] Improve CLI signal handling on POSIX.

[Evgeniy Temirgaleev via 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

Re: [Tarantool-patches] [PATCH luajit] Improve CLI signal handling on POSIX.

[Sergey Bronnikov via 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 --]