From 4c836a1dd1708f439f3e0e36de729a3ffc33a63e Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Fri, 12 Feb 2021 05:43:04 -0700 Subject: [PATCH] rktio: always set signal mask to empty after fork Saving and restoring the signal-mask state does not work right, since rktio itself may block SIGCHLD in some cases, and it doesn't seem useful/right to preserve the mask after fork. --- .../tests/racket/subprocess.rktl | 5 +++- .../tests/racket/unix_check.c | 29 +++++++++++++++---- racket/src/rktio/rktio.h | 14 +++++---- racket/src/rktio/rktio_process.c | 4 +++ racket/src/rktio/rktio_signal.c | 20 ++++--------- 5 files changed, 45 insertions(+), 27 deletions(-) diff --git a/pkgs/racket-test-core/tests/racket/subprocess.rktl b/pkgs/racket-test-core/tests/racket/subprocess.rktl index fbfc3118ac..43daf4493a 100644 --- a/pkgs/racket-test-core/tests/racket/subprocess.rktl +++ b/pkgs/racket-test-core/tests/racket/subprocess.rktl @@ -595,7 +595,10 @@ exe (path->complete-path "unix_check.c" (or (current-load-relative-directory) (current-directory))))) - (test #t 'subprocess-state (system* exe))) + (test #t 'subprocess-state (let ([o (open-output-bytes)]) + (or (parameterize ([current-output-port o]) + (system* exe)) + (get-output-bytes o))))) (delete-directory/files dir))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/pkgs/racket-test-core/tests/racket/unix_check.c b/pkgs/racket-test-core/tests/racket/unix_check.c index fd75443ac9..2b0734c3e5 100644 --- a/pkgs/racket-test-core/tests/racket/unix_check.c +++ b/pkgs/racket-test-core/tests/racket/unix_check.c @@ -1,5 +1,6 @@ #include #include +#include /* Make sure a child process is started with normal properties, such as no blocked signals */ @@ -11,10 +12,12 @@ int main() int i; /* SIGPROF tends to be near the end of the range of signal IDs */ - for (i = 1; i < SIGPROF; i++) { + for (i = 1; i <= SIGPROF; i++) { if (sigaction(i, NULL, &sa) == 0) { - if (sa.sa_handler != SIG_DFL) + if (sa.sa_handler != SIG_DFL) { + printf("handler %d: %p\n", i, (void *)sa.sa_handler); return 1; + } } else { /* sigaction sometimes isn't allowed at all on SIGKILL, for example, so ignore that failure */ @@ -24,8 +27,22 @@ int main() sigemptyset(&set); sigprocmask(SIG_BLOCK, &set, &old_set); - return (sigismember(&old_set, SIGCHLD) - || sigismember(&old_set, SIGINT) - || sigismember(&old_set, SIGHUP) - || sigismember(&old_set, SIGQUIT)); + if (sigismember(&old_set, SIGCHLD)) { + printf("masked: SIGCHLD\n"); + return 1; + } + if (sigismember(&old_set, SIGINT)) { + printf("masked: SIGINT\n"); + return 1; + } + if (sigismember(&old_set, SIGHUP)) { + printf("masked: SIGHUP\n"); + return 1; + } + if (sigismember(&old_set, SIGQUIT)) { + printf("masked: SIGQUIT\n"); + return 1; + } + + return 0; } diff --git a/racket/src/rktio/rktio.h b/racket/src/rktio/rktio.h index 2474bf973b..955ad997d5 100644 --- a/racket/src/rktio/rktio.h +++ b/racket/src/rktio/rktio.h @@ -1004,7 +1004,9 @@ RKTIO_EXTERN void rktio_install_os_signal_handler(rktio_t *rktio); Ctl-C on Windows) to signal the handle of `rktio` and also records the signal for reporting via `rktio_poll_os_signal`. Only one `rktio` can be registered this way at a time. This function must - not be called in two threads at the same time. */ + not be called in two threads at the same time; more generally, it + can only be called when `rktio_will_modify_os_signal_handler` + can be called for SIGINT, etc. */ RKTIO_EXTERN_NOERR int rktio_poll_os_signal(rktio_t *rktio); /* Returns one of the following, not counting the last one: */ @@ -1021,11 +1023,11 @@ RKTIO_EXTERN void rktio_will_modify_os_signal_handler(int sig_id); about to be modified within the process but outside of rktio, where `sig_id` is a signal identifier --- such as SIGINT or SIGTERM. This notification allows rktio to record the current signal disposition - so that it can be restored after forking a new Unix process. - Signal registrations should happen only before multiple threads use - rktio, and registration of the signal can happen before any - `rktio_init` call. On the first `rktio_will_modify_os_signal_handler` - call, the signal mask is also recorded to be restored in a fork. */ + so that it can be restored after forking a new Unix process. Signal + registrations should happen only before multiple threads use rktio, + and registration of the signal can happen before any `rktio_init` + call. After a signal is registered, trying to re-register it after + threads start is harmless. */ /*************************************************/ /* Time and date */ diff --git a/racket/src/rktio/rktio_process.c b/racket/src/rktio/rktio_process.c index ae94892922..89202436c0 100644 --- a/racket/src/rktio/rktio_process.c +++ b/racket/src/rktio/rktio_process.c @@ -982,6 +982,10 @@ int rktio_process_init(rktio_t *rktio) it's a per-thread setting on Linux, and we want SIGCHLD blocked everywhere. */ block_sigchld(); + /* We'll set a handler later (possibly after other threads start), + so register SIGCHLD now: */ + rktio_will_modify_os_signal_handler(SIGCHLD); + centralized_start_child_signal_handler(); #endif diff --git a/racket/src/rktio/rktio_signal.c b/racket/src/rktio/rktio_signal.c index 271659d30c..c4a6105b6c 100644 --- a/racket/src/rktio/rktio_signal.c +++ b/racket/src/rktio/rktio_signal.c @@ -127,18 +127,10 @@ typedef struct signal_handler_saved_disposition { } signal_handler_saved_disposition; static signal_handler_saved_disposition *saved_dispositions; -#ifdef RKTIO_SYSTEM_UNIX -static sigset_t initial_procmask; -#endif void rktio_will_modify_os_signal_handler(int sig_id) { signal_handler_saved_disposition *saved; -#ifdef RKTIO_SYSTEM_UNIX - if (saved_dispositions == NULL) - sigprocmask(SIG_SETMASK, NULL, &initial_procmask); -#endif - for (saved = saved_dispositions; saved; saved = saved->next) if (saved->sig_id == sig_id) return; @@ -156,13 +148,13 @@ void rktio_will_modify_os_signal_handler(int sig_id) { #ifdef RKTIO_SYSTEM_UNIX /* called in a child thread after `fork */ void rktio_restore_modified_signal_handlers() { - if (saved_dispositions) { - signal_handler_saved_disposition *saved; + signal_handler_saved_disposition *saved; + sigset_t set; - for (saved = saved_dispositions; saved; saved = saved->next) - sigaction(saved->sig_id, &saved->sa, NULL); + for (saved = saved_dispositions; saved; saved = saved->next) + sigaction(saved->sig_id, &saved->sa, NULL); - sigprocmask(SIG_SETMASK, &initial_procmask, NULL); - } + sigemptyset(&set); + sigprocmask(SIG_SETMASK, &set, NULL); } #endif