Block some signals while writing

Writing a partial escape can easily lock up a terminal. This
is especially relevant when working with bitmaps, as they're
thousands or even millions of times longer than a typical
escape. Immediately before writing, block SIGINT, SIGQUIT, and
SIGTERM in the writing thread, and unblock them upon emerging
(at which point we'll immediately see any queued signal
get delivered). Don't block signals like SIGSEGV that would
seem indicative of actual problems. For this to actually work,
all other threads must also have the signals masked; we thus
now add them to the signal mask of notcurses_getc(), rather
than deleting them. Closes #1416.

NEWS.md

@@ -11,6 +11,14 @@ rearrangements of Notcurses.
   * All functions prefixed with `channel_` have been deprecated in favor of
     versions prefixed with `ncchannel_`, which the former now wrap. The old
     versions will be removed in ABI3.
+  * `SIGINT`, `SIGQUIT`, and `SIGTERM` are now masked for the calling thread
+    when writing starts, and unmasked when writing has ended. This prevents
+    the writing thread from handling these signals in the middle of a write,
+    which could otherwise leave the terminal locked up (if it resulted in
+    aborting an escape sequence). The signal will be delivered when unblocked.
+    For this to work properly, other threads ought also have these signals
+    blocked. `notcurses_getc()` and friends thus no longer drop these signals
+    from the provided `sigset_t`; they are instead added if not present.
 
 * 2.2.8 (2021-04-18)
   * All remaining functions prefixed with `cell_` or `cells_` have been

doc/man/man3/notcurses_input.3.md

@@ -27,7 +27,7 @@ typedef struct ncinput {
 
 **bool nckey_mouse_p(char32_t ***r***);**
 
-**char32_t notcurses_getc(struct notcurses* ***n***, const struct timespec* ***ts***, sigset_t* ***sigmask***, ncinput* ***ni***);**
+**char32_t notcurses_getc(struct notcurses* ***n***, const struct timespec* ***ts***, const sigset_t* ***sigmask***, ncinput* ***ni***);**
 
 **char32_t notcurses_getc_nblock(struct notcurses* ***n***, ncinput* ***ni***);**
 

include/notcurses/notcurses.h

@@ -1020,7 +1020,7 @@ ncinput_equal_p(const ncinput* n1, const ncinput* n2){
 // be NULL. Returns 0 on a timeout. If an event is processed, the return value
 // is the 'id' field from that event. 'ni' may be NULL.
 API char32_t notcurses_getc(struct notcurses* n, const struct timespec* ts,
-                            sigset_t* sigmask, ncinput* ni)
+                            const sigset_t* sigmask, ncinput* ni)
   __attribute__ ((nonnull (1)));
 
 // Get a file descriptor suitable for input event poll()ing. When this

src/lib/input.c

@@ -322,7 +322,7 @@ handle_getc(ncinputlayer* nc, int kpress, ncinput* ni, int leftmargin, int topma
 // blocks up through ts (infinite with NULL ts), returning number of events
 // (0 on timeout) or -1 on error/interruption.
 static int
-block_on_input(int fd, const struct timespec* ts, sigset_t* sigmask){
+block_on_input(int fd, const struct timespec* ts, const sigset_t* sigmask){
   struct pollfd pfd = {
     .fd = fd,
     .events = POLLIN,
@@ -335,20 +335,20 @@ block_on_input(int fd, const struct timespec* ts, sigset_t* sigmask){
   }else{
     pthread_sigmask(0, NULL, &scratchmask);
   }
-  sigmask = &scratchmask;
-  sigdelset(sigmask, SIGCONT);
-  sigdelset(sigmask, SIGWINCH);
-  sigdelset(sigmask, SIGINT);
-  sigdelset(sigmask, SIGILL);
-  sigdelset(sigmask, SIGQUIT);
-  sigdelset(sigmask, SIGSEGV);
-  sigdelset(sigmask, SIGABRT);
-  sigdelset(sigmask, SIGTERM);
+  sigdelset(&scratchmask, SIGCONT);
+  sigdelset(&scratchmask, SIGWINCH);
+  sigdelset(&scratchmask, SIGILL);
+  sigdelset(&scratchmask, SIGSEGV);
+  sigdelset(&scratchmask, SIGABRT);
+  // now add those which we don't want while writing
+  sigaddset(&scratchmask, SIGINT);
+  sigaddset(&scratchmask, SIGQUIT);
+  sigaddset(&scratchmask, SIGTERM);
 #ifdef POLLRDHUP
   pfd.events |= POLLRDHUP;
 #endif
   int events;
-  while((events = ppoll(&pfd, 1, ts, sigmask)) < 0){
+  while((events = ppoll(&pfd, 1, ts, &scratchmask)) < 0){
     if(events == 0){
       return 0;
     }
@@ -382,7 +382,8 @@ handle_queued_input(ncinputlayer* nc, ncinput* ni, int leftmargin, int topmargin
 }
 
 static char32_t
-handle_input(ncinputlayer* nc, ncinput* ni, int leftmargin, int topmargin, sigset_t* sigmask){
+handle_input(ncinputlayer* nc, ncinput* ni, int leftmargin, int topmargin,
+             const sigset_t* sigmask){
   // we once used getc() here (and kept ttyinfp, a FILE*, instead of the file
   // descriptor ttyinfd), but under tmux, the first time running getc() would
   // (bewilderingly) see a series of terminal reset codes emitted. this has
@@ -412,7 +413,7 @@ handle_input(ncinputlayer* nc, ncinput* ni, int leftmargin, int topmargin, sigse
 
 static char32_t
 handle_ncinput(ncinputlayer* nc, ncinput* ni, int leftmargin, int topmargin,
-               sigset_t* sigmask){
+               const sigset_t* sigmask){
   if(ni){
     memset(ni, 0, sizeof(*ni));
   }
@@ -445,7 +446,7 @@ handle_ncinput(ncinputlayer* nc, ncinput* ni, int leftmargin, int topmargin,
 // helper so we can do counter increment at a single location
 static inline char32_t
 ncinputlayer_prestamp(ncinputlayer* nc, const struct timespec *ts,
-                      sigset_t* sigmask, ncinput* ni, int leftmargin,
+                      const sigset_t* sigmask, ncinput* ni, int leftmargin,
                       int topmargin){
 //fprintf(stderr, "PRESTAMP OCCUPADO: %d\n", nc->inputbuf_occupied);
   if(nc->inputbuf_occupied){
@@ -462,7 +463,7 @@ ncinputlayer_prestamp(ncinputlayer* nc, const struct timespec *ts,
 
 // infp has already been set non-blocking
 char32_t notcurses_getc(notcurses* nc, const struct timespec *ts,
-                        sigset_t* sigmask, ncinput* ni){
+                        const sigset_t* sigmask, ncinput* ni){
   char32_t r = ncinputlayer_prestamp(&nc->input, ts, sigmask, ni,
                                      nc->margin_l, nc->margin_t);
   if(r != (char32_t)-1){

src/lib/internal.h

@@ -603,6 +603,9 @@ typedef struct notcurses {
   int loglevel;
   palette256 palette; // 256-indexed palette can be used instead of/with RGB
   bool palette_damage[NCPALETTESIZE];
+  // we block many signals while writing out a frame, since interrupted escapes
+  // can lock up a terminal. this preserves the signal mask on entry.
+  sigset_t old_blocked_signals;
   unsigned stdio_blocking_save; // was stdio blocking at entry? restore on stop.
 } notcurses;
 
@@ -1578,6 +1581,10 @@ int setup_signals(void* nc, bool no_quit_sigs, bool no_winch_sig,
                   int(*handler)(void*));
 int drop_signals(void* nc);
 
+// block a few signals for the duration of a write to the terminal.
+int block_signals(struct notcurses* nc);
+int unblock_signals(struct notcurses* nc);
+
 void ncvisual_printbanner(const notcurses* nc);
 
 // alpha comes to us 0--255, but we have only 3 alpha values to map them to

src/lib/render.c

@@ -1145,9 +1145,11 @@ raster_and_write(notcurses* nc, ncpile* p, FILE* out){
   }
   int ret = 0;
   //fflush(nc->ttyfp);
+  block_signals(nc);
   if(blocking_write(fileno(nc->ttyfp), nc->rstate.mstream, nc->rstate.mstrsize)){
     ret = -1;
   }
+  unblock_signals(nc);
 //fprintf(stderr, "%lu/%lu %lu/%lu %lu/%lu %d\n", nc->stats.defaultelisions, nc->stats.defaultemissions, nc->stats.fgelisions, nc->stats.fgemissions, nc->stats.bgelisions, nc->stats.bgemissions, ret);
   if(nc->renderfp){
     fprintf(nc->renderfp, "%s\n", nc->rstate.mstream);

src/lib/signal.c

@@ -23,8 +23,29 @@ static struct sigaction old_segv;
 static struct sigaction old_abrt;
 static struct sigaction old_term;
 
+// Signals we block when we start writing out a frame, so as not to be
+// interrupted in media res (interrupting an escape can lock up a terminal).
+// Prepared in setup_signals(), and never changes across our lifetime.
+static sigset_t wblock_signals;
+
 static int(*fatal_callback)(void*); // fatal handler callback
 
+int block_signals(notcurses* nc){
+  if(pthread_sigmask(SIG_BLOCK, &wblock_signals, &nc->old_blocked_signals)){
+    logerror(nc, "Couldn't block signals prior to write (%s)\n", strerror(errno));
+    return -1;
+  }
+  return 0;
+}
+
+int unblock_signals(notcurses* nc){
+  if(pthread_sigmask(SIG_SETMASK, &nc->old_blocked_signals, NULL)){
+    logerror(nc, "Error restoring signal mask after write (%s)\n", strerror(errno));
+    return -1;
+  }
+  return 0;
+}
+
 int drop_signals(void* nc){
   int ret = -1;
   void* expected = nc;
@@ -132,5 +153,10 @@ int setup_signals(void* nc, bool no_quit_sigs, bool no_winch_sig,
     }
     handling_fatals = true;
   }
+  // we don't really want to go blocking SIGSEGV etc while we write, but
+  // we *do* temporarily block user-inititated signals.
+  sigaddset(&wblock_signals, SIGINT);
+  sigaddset(&wblock_signals, SIGTERM);
+  sigaddset(&wblock_signals, SIGQUIT);
   return 0;
 }