From 8a7ff8a31705200e7c62b881301d7ee010508522 Mon Sep 17 00:00:00 2001 From: nick black Date: Sun, 21 Feb 2021 23:04:19 -0500 Subject: [PATCH 1/2] signals: restore them on exit #1357 --- include/notcurses/nckeys.h | 3 +- include/notcurses/notcurses.h | 6 +- src/lib/input.c | 5 +- src/lib/signal.c | 104 ++++++++++++++++++++++++++-------- 4 files changed, 90 insertions(+), 28 deletions(-) diff --git a/include/notcurses/nckeys.h b/include/notcurses/nckeys.h index ba7a41a0b..e97e66d0f 100644 --- a/include/notcurses/nckeys.h +++ b/include/notcurses/nckeys.h @@ -9,7 +9,7 @@ extern "C" { // Special composed key definitions. These values are added to 0x100000. #define NCKEY_INVALID suppuabize(0) -#define NCKEY_RESIZE suppuabize(1) // generated internally in response to SIGWINCH +#define NCKEY_SIGNAL suppuabize(1) // generated internally in response to SIGWINCH / SIGCONT #define NCKEY_UP suppuabize(2) #define NCKEY_RIGHT suppuabize(3) #define NCKEY_DOWN suppuabize(4) @@ -118,6 +118,7 @@ extern "C" { #define NCKEY_SCROLL_UP NCKEY_BUTTON4 #define NCKEY_SCROLL_DOWN NCKEY_BUTTON5 #define NCKEY_RETURN NCKEY_ENTER +#define NCKEY_RESIZE NCKEY_SIGNAL // Just aliases, ma'am, from the 128 characters common to ASCII+UTF8 #define NCKEY_ESC 0x1b diff --git a/include/notcurses/notcurses.h b/include/notcurses/notcurses.h index 9eca6ee98..a6c0e47f3 100644 --- a/include/notcurses/notcurses.h +++ b/include/notcurses/notcurses.h @@ -1010,13 +1010,15 @@ 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); + sigset_t* sigmask, ncinput* ni) + __attribute__ ((nonnull (1))); // Get a file descriptor suitable for input event poll()ing. When this // descriptor becomes available, you can call notcurses_getc_nblock(), // and input ought be ready. This file descriptor is *not* necessarily // the file descriptor associated with stdin (but it might be!). -API int notcurses_inputready_fd(struct notcurses* n); +API int notcurses_inputready_fd(struct notcurses* n) + __attribute__ ((nonnull (1))); // 'ni' may be NULL if the caller is uninterested in event details. If no event // is ready, returns 0. diff --git a/src/lib/input.c b/src/lib/input.c index aa9d9c342..da99c066f 100644 --- a/src/lib/input.c +++ b/src/lib/input.c @@ -14,6 +14,7 @@ static const char32_t NCKEY_CSI = 1; static sig_atomic_t resize_seen; +// called for SIGWINCH and SIGCONT void sigwinch_handler(int signo){ resize_seen = signo; } @@ -335,8 +336,10 @@ block_on_input(int fd, const struct timespec* ts, sigset_t* sigmask){ 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); @@ -402,7 +405,7 @@ handle_input(ncinputlayer* nc, ncinput* ni, int leftmargin, int topmargin, sigse // highest priority is resize notifications, since they don't queue if(resize_seen){ resize_seen = 0; - return NCKEY_RESIZE; + return NCKEY_SIGNAL; } return handle_queued_input(nc, ni, leftmargin, topmargin); } diff --git a/src/lib/signal.c b/src/lib/signal.c index 9392e6c5b..dc325566c 100644 --- a/src/lib/signal.c +++ b/src/lib/signal.c @@ -4,37 +4,85 @@ #include "internal.h" // only one notcurses object can be the target of signal handlers, due to their -// process-wide nature. +// process-wide nature. hold this lock over any of the shared data below. +static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; +// we still need an atomic signal_nc, despite guarding with the mutex, so that +// it can be safely looked up within the signal handler. static void* _Atomic signal_nc = ATOMIC_VAR_INIT(NULL); // ugh -static void (*signal_sa_handler)(int); // stashed signal handler we replaced -static int(*fatal_callback)(void*); // fatal handler callback provided +static bool handling_winch; +static bool handling_fatals; + +// saved signal actions, restored in drop_signals() FIXME make an array +static struct sigaction old_winch; +static struct sigaction old_cont; +static struct sigaction old_term; +static struct sigaction old_int; +static struct sigaction old_ill; +static struct sigaction old_quit; +static struct sigaction old_segv; +static struct sigaction old_abrt; +static struct sigaction old_term; + +static int(*fatal_callback)(void*); // fatal handler callback int drop_signals(void* nc){ - void* old = nc; - if(!atomic_compare_exchange_strong(&signal_nc, &old, NULL)){ - fprintf(stderr, "Couldn't drop signals: %p != %p\n", old, nc); - return -1; + int ret = -1; + void* expected = nc; + pthread_mutex_lock(&lock); + if(atomic_compare_exchange_strong(&signal_nc, &expected, nc)){ + if(handling_winch){ + sigaction(SIGWINCH, &old_winch, NULL); + sigaction(SIGCONT, &old_cont, NULL); + handling_winch = false; + } + if(handling_fatals){ + sigaction(SIGINT, &old_int, NULL); + sigaction(SIGILL, &old_ill, NULL); + sigaction(SIGTERM, &old_term, NULL); + sigaction(SIGSEGV, &old_segv, NULL); + sigaction(SIGABRT, &old_abrt, NULL); + sigaction(SIGQUIT, &old_quit, NULL); + handling_fatals = false; + } + ret = !atomic_compare_exchange_strong(&signal_nc, &expected, NULL); + } + pthread_mutex_unlock(&lock); + if(ret){ + fprintf(stderr, "Couldn't drop signals with %p\n", nc); + } + return ret; +} + +static void +invoke_old(const struct sigaction* old, int signo, siginfo_t* sinfo, void* v){ + if(old->sa_sigaction){ + old->sa_sigaction(signo, sinfo, v); + }else if(old->sa_handler){ + old->sa_handler(signo); } - return 0; } // this wildly unsafe handler will attempt to restore the screen upon receipt -// of SIG{INT, SEGV, ABRT, QUIT, TERM}. godspeed you, black emperor! +// of SIG{ILL, INT, SEGV, ABRT, QUIT, TERM}. godspeed you, black emperor! static void -fatal_handler(int signo){ +fatal_handler(int signo, siginfo_t* siginfo, void* v){ notcurses* nc = atomic_load(&signal_nc); if(nc){ fatal_callback(nc); - if(signal_sa_handler){ - signal_sa_handler(signo); + switch(signo){ + case SIGTERM: invoke_old(&old_term, signo, siginfo, v); break; + case SIGQUIT: invoke_old(&old_quit, signo, siginfo, v); break; + case SIGSEGV: invoke_old(&old_segv, signo, siginfo, v); break; + case SIGINT: invoke_old(&old_int, signo, siginfo, v); break; + case SIGILL: invoke_old(&old_ill, signo, siginfo, v); break; + case SIGABRT: invoke_old(&old_abrt, signo, siginfo, v); break; } - raise(signo); + raise(signo); // FIXME does this invoke twice? hrmmm } } int setup_signals(void* nc, bool no_quit_sigs, bool no_winch_sig, int(*handler)(void*)){ - struct sigaction oldact; void* expected = NULL; struct sigaction sa; @@ -45,36 +93,44 @@ int setup_signals(void* nc, bool no_quit_sigs, bool no_winch_sig, if(!no_winch_sig){ memset(&sa, 0, sizeof(sa)); sa.sa_handler = sigwinch_handler; - if(sigaction(SIGWINCH, &sa, NULL)){ + sigaddset(&sa.sa_mask, SIGWINCH); + sigaddset(&sa.sa_mask, SIGCONT); + int ret = 0; + ret |= sigaction(SIGWINCH, &sa, &old_winch); + ret |= sigaction(SIGCONT, &sa, &old_cont); + if(ret){ atomic_store(&signal_nc, NULL); - fprintf(stderr, "Error installing SIGWINCH handler (%s)\n", + fprintf(stderr, "Error installing term signal handler (%s)\n", strerror(errno)); return -1; } + handling_winch = true; } if(!no_quit_sigs){ memset(&sa, 0, sizeof(sa)); fatal_callback = handler; - sa.sa_handler = fatal_handler; + sa.sa_sigaction = fatal_handler; + sigaddset(&sa.sa_mask, SIGILL); sigaddset(&sa.sa_mask, SIGINT); sigaddset(&sa.sa_mask, SIGQUIT); sigaddset(&sa.sa_mask, SIGSEGV); sigaddset(&sa.sa_mask, SIGABRT); sigaddset(&sa.sa_mask, SIGTERM); - sa.sa_flags = SA_RESETHAND; // don't try twice + sa.sa_flags = SA_SIGINFO | SA_RESETHAND; // don't try fatal signals twice int ret = 0; - ret |= sigaction(SIGINT, &sa, &oldact); - ret |= sigaction(SIGQUIT, &sa, &oldact); - ret |= sigaction(SIGSEGV, &sa, &oldact); - ret |= sigaction(SIGABRT, &sa, &oldact); - ret |= sigaction(SIGTERM, &sa, &oldact); + ret |= sigaction(SIGILL, &sa, &old_ill); + ret |= sigaction(SIGINT, &sa, &old_int); + ret |= sigaction(SIGQUIT, &sa, &old_quit); + ret |= sigaction(SIGSEGV, &sa, &old_segv); + ret |= sigaction(SIGABRT, &sa, &old_abrt); + ret |= sigaction(SIGTERM, &sa, &old_term); if(ret){ atomic_store(&signal_nc, NULL); fprintf(stderr, "Error installing fatal signal handlers (%s)\n", strerror(errno)); return -1; } - signal_sa_handler = oldact.sa_handler; + handling_fatals = true; } return 0; } From 222112054385f88737f09de7c296eb030a021f93 Mon Sep 17 00:00:00 2001 From: nick black Date: Sun, 21 Feb 2021 23:07:57 -0500 Subject: [PATCH 2/2] add SIGILL to documented fatal signals #1357 --- NEWS.md | 3 +++ doc/man/man3/notcurses_direct.3.md | 6 +++--- doc/man/man3/notcurses_init.3.md | 6 +++--- include/notcurses/notcurses.h | 6 +++--- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index a96a820e1..198b9ed71 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,9 @@ This document attempts to list user-visible changes and any major internal rearrangements of Notcurses. +* 2.2.3 (not yet released) + * Add `SIGILL` to the set of fatal signals we handle. + * 2.2.2 (2021-02-18): * `notcurses_stats()` no longer qualifies its `notcurses*` argument with `const`, since it now takes a lock. I'm sorry about that, though on the diff --git a/doc/man/man3/notcurses_direct.3.md b/doc/man/man3/notcurses_direct.3.md index 1ec0b3d50..f9e4ec13f 100644 --- a/doc/man/man3/notcurses_direct.3.md +++ b/doc/man/man3/notcurses_direct.3.md @@ -110,9 +110,9 @@ The following flags are defined: buffering; see **tcgetattr(3)**). * **NCDIRECT_OPTION_NO_QUIT_SIGHANDLERS**: A signal handler will usually be - installed for **SIGINT**, **SIGQUIT**, **SIGSEGV**, **SIGTERM**, and - **SIGABRT**, cleaning up the terminal on such exceptions. With this flag, - the handler will not be installed. + installed for **SIGINT**, **SIGILL**, **SIGQUIT**, **SIGSEGV**, + **SIGTERM**, and **SIGABRT**, cleaning up the terminal on such exceptions. + With this flag, the handler will not be installed. An appropriate **terminfo(5)** entry must exist for the terminal. This entry is usually selected using the value of the **TERM** environment variable (see diff --git a/doc/man/man3/notcurses_init.3.md b/doc/man/man3/notcurses_init.3.md index f7154dbf9..a6654b9a4 100644 --- a/doc/man/man3/notcurses_init.3.md +++ b/doc/man/man3/notcurses_init.3.md @@ -122,9 +122,9 @@ zero. The following flags are defined: input. With this flag, the handler will not be installed. * **NCOPTION_NO_QUIT_SIGHANDLERS**: A signal handler will usually be installed - for **SIGINT**, **SIGQUIT**, **SIGSEGV**, **SIGTERM**, and **SIGABRT**, - cleaning up the terminal on such exceptions. With this flag, the handler - will not be installed. + for **SIGINT**, **SIGILL**, **SIGQUIT**, **SIGSEGV**, **SIGTERM**, and + **SIGABRT**, cleaning up the terminal on such exceptions. With this flag, + the handler will not be installed. * **NCOPTION_SUPPRESS_BANNERS**: Disables the diagnostics and version information printed on startup, and the performance summary on exit. diff --git a/include/notcurses/notcurses.h b/include/notcurses/notcurses.h index a6c0e47f3..cf802a198 100644 --- a/include/notcurses/notcurses.h +++ b/include/notcurses/notcurses.h @@ -836,9 +836,9 @@ typedef enum { // event in the notcurses_getc() queue. Set to inhibit this handler. #define NCOPTION_NO_WINCH_SIGHANDLER 0x0004ull -// We typically install a signal handler for SIG{INT, SEGV, ABRT, QUIT} that -// restores the screen, and then calls the old signal handler. Set to inhibit -// registration of these signal handlers. +// We typically install a signal handler for SIG{INT, ILL, SEGV, ABRT, TERM, +// QUIT} that restores the screen, and then calls the old signal handler. Set +// to inhibit registration of these signal handlers. #define NCOPTION_NO_QUIT_SIGHANDLERS 0x0008ull // NCOPTION_RETAIN_CURSOR was removed in 1.6.18. It ought be repurposed. FIXME.