[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