terminal-unix: move all processing out of sighandler

commit fa9e1f06f tried to move signal unsafe operations out of
signal handlers but mistakenly introduced a race. before,
sigtstop would process the following in order:

0. do_deactivate_getch2();
1. raise(SIGTSTP)

that commit moved 0 out of the signal handler (due to it being
unsafe) but kept 1 in there. this may mess up the ordering of
these operations. this commit moves everything out of the
handler so that things happen in proper order.

since things are now moved out of the handler, SA_RESETHAND is
no longer being applied to SIGTSTP. since that can result in
races if multiple signals are delivered faster than we can
respond to them.

1 files changed, 16 insertions, 23 deletions

diff --git a/osdep/terminal-unix.c b/osdep/terminal-unix.c
index cb92dfc664..9b22ac26c3 100644
--- a/osdep/terminal-unix.c
+++ b/osdep/terminal-unix.c
@@ -324,7 +324,7 @@ static int setsigaction(int signo, void (*handler) (int),
     struct sigaction sa;
     sa.sa_handler = handler;
 
-    if(do_mask)
+    if (do_mask)
         sigfillset(&sa.sa_mask);
     else
         sigemptyset(&sa.sa_mask);
@@ -354,26 +354,11 @@ static int death_pipe[2] = {-1, -1};
 enum { PIPE_STOP, PIPE_CONT };
 static int stop_cont_pipe[2] = {-1, -1};
 
-static void stop_sighandler(int signum)
+static void stop_cont_sighandler(int signum)
 {
     int saved_errno = errno;
-    (void)write(stop_cont_pipe[1], &(char){PIPE_STOP}, 1);
-    (void)write(STDERR_FILENO, TERM_ESC_RESTORE_CURSOR,
-                sizeof(TERM_ESC_RESTORE_CURSOR) - 1);
-    errno = saved_errno;
-
-    // note: for this signal, we use SA_RESETHAND but do NOT mask signals
-    // so this will invoke the default handler
-    raise(SIGTSTP);
-}
-
-static void continue_sighandler(int signum)
-{
-    int saved_errno = errno;
-    // SA_RESETHAND has reset SIGTSTP, so we need to restore it here
-    setsigaction(SIGTSTP, stop_sighandler, SA_RESETHAND, false);
-
-    (void)write(stop_cont_pipe[1], &(char){PIPE_CONT}, 1);
+    char sig = signum == SIGCONT ? PIPE_CONT : PIPE_STOP;
+    (void)write(stop_cont_pipe[1], &sig, 1);
     errno = saved_errno;
 }
 
@@ -437,10 +422,18 @@ static MP_THREAD_VOID terminal_thread(void *ptr)
         if (fds[1].revents & POLLIN) {
             int8_t c = -1;
             (void)read(stop_cont_pipe[0], &c, 1);
-            if (c == PIPE_STOP)
+            if (c == PIPE_STOP) {
                 do_deactivate_getch2();
-            else if (c == PIPE_CONT)
+                (void)write(STDERR_FILENO, TERM_ESC_RESTORE_CURSOR,
+                            sizeof(TERM_ESC_RESTORE_CURSOR) - 1);
+                // trying to reset SIGTSTP handler to default and raise it will
+                // result in a race and there's no other way to invoke the
+                // default handler. so just invoke SIGSTOP since it's
+                // effectively the same thing.
+                raise(SIGSTOP);
+            } else if (c == PIPE_CONT) {
                 getch2_poll();
+            }
         }
         if (fds[2].revents) {
             int retval = read(tty_in, &buf.b[buf.len], BUF_LEN - buf.len);
@@ -566,8 +559,8 @@ void terminal_init(void)
     tcgetattr(tty_in, &tio_orig);
 
     // handlers to fix terminal settings
-    setsigaction(SIGCONT, continue_sighandler, 0, true);
-    setsigaction(SIGTSTP, stop_sighandler, SA_RESETHAND, false);
+    setsigaction(SIGCONT, stop_cont_sighandler, 0, true);
+    setsigaction(SIGTSTP, stop_cont_sighandler, 0, true);
     setsigaction(SIGTTIN, SIG_IGN, 0, true);
     setsigaction(SIGTTOU, SIG_IGN, 0, true);