From: Arran Cudbard-Bell Date: Wed, 4 Jul 2018 20:06:27 +0000 (-0400) Subject: Use central thread creation function that prevents signals being delivered to pthreads X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=712efca481cf7080cb45f75bd1080d8470dabf2a;p=thirdparty%2Ffreeradius-server.git Use central thread creation function that prevents signals being delivered to pthreads --- diff --git a/src/lib/io/schedule.c b/src/lib/io/schedule.c index 0b356a8b85b..86f74b8b275 100644 --- a/src/lib/io/schedule.c +++ b/src/lib/io/schedule.c @@ -303,16 +303,67 @@ fail: } +/** Creates a new thread using our standard set of options + * + * New threads are: + * - Joinable, i.e. you can call pthread_join on them to confirm they've exited + * - Immune to catchable signals. + * + * @param[out] thread handled that was created by pthread_create. + * @param[in] func entry point for the thread. + * @param[in] arg Argument to pass to func. + * @return + * - 0 on success. + * - -1 on failure. + */ +int fr_schedule_pthread_create(pthread_t *thread, void *(*func)(void *), void *arg) +{ + pthread_attr_t attr; + sigset_t sig_mask; /* signals to block */ + int ret; + + /* + * Set the thread to wait around after it's exited + * so it can be joined. This is more of a useful + * mechanism for the parent to determine if all + * the threads have exited so it can continue with + * a graceful shutdown. + */ + pthread_attr_init(&attr); + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); + + /* + * We generally use kqueue to signal threads so we + * need to mask all the signals to prevent them from + * being delivered to something that shouldn't be + * processing them. + */ + sigfillset(&sig_mask); + + if (pthread_sigmask(SIG_BLOCK, &sig_mask, NULL) != 0) { + fr_strerror_printf("Failed blocking child signals: %s", fr_syserror(errno)); + return -1; + } + + ret = pthread_create(thread, &attr, func, arg); + if (ret != 0) { + fr_strerror_printf("Failed creating thread: %s", fr_syserror(ret)); + return -1; + } + + return 0; +} + /** Create a scheduler and spawn the child threads. * - * @param[in] ctx the talloc context - * @param[in] el the event list, only for single-threaded mode. - * @param[in] logger the destination for all logging messages - * @param[in] lvl the log level - * @param[in] max_networks the number of network threads - * @param[in] max_workers the number of worker threads - * @param[in] worker_thread_instantiate callback for new worker threads - * @param[in] worker_thread_ctx context for callback + * @param[in] ctx talloc context. + * @param[in] el event list, only for single-threaded mode. + * @param[in] logger destination for all logging messages. + * @param[in] lvl log level. + * @param[in] max_networks number of network threads. + * @param[in] max_workers number of worker threads. + * @param[in] worker_thread_instantiate callback for new worker threads. + * @param[in] worker_thread_ctx context for callback. * @return * - NULL on error * - fr_schedule_t new scheduler @@ -325,8 +376,6 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el, { #ifdef HAVE_PTHREAD_H int i; - int rcode; - pthread_attr_t attr; fr_dlist_t *entry, *next; #endif fr_schedule_t *sc; @@ -401,9 +450,6 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el, } #ifdef HAVE_PTHREAD_H - (void) pthread_attr_init(&attr); - (void) pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); - /* * Create the list which holds the workers. */ @@ -411,7 +457,7 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el, memset(&sc->semaphore, 0, sizeof(sc->semaphore)); if (sem_init(&sc->semaphore, 0, SEMAPHORE_LOCKED) != 0) { - fr_strerror_printf("Failed creating semaphore: %s", fr_syserror(errno)); + fr_log(sc->log, L_ERR, "Failed creating semaphore: %s", fr_syserror(errno)); talloc_free(sc); return NULL; } @@ -424,9 +470,8 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el, sc->sn->sc = sc; sc->sn->id = 0; - rcode = pthread_create(&sc->sn->pthread_id, &attr, fr_schedule_network_thread, sc->sn); - if (rcode != 0) { - fr_strerror_printf("Failed creating network thread: %s", fr_syserror(errno)); + if (fr_schedule_pthread_create(&sc->sn->pthread_id, fr_schedule_network_thread, sc->sn) < 0) { + fr_log(sc->log, L_ERR, "Failed creating network thread %s", fr_strerror()); goto fail; } @@ -461,15 +506,15 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el, sw->status = FR_CHILD_INITIALIZING; fr_dlist_insert_head(&sc->workers, &sw->entry); - rcode = pthread_create(&sw->pthread_id, &attr, fr_schedule_worker_thread, sw); - if (rcode != 0) { - fr_log(sc->log, L_ERR, "Failed creating worker %d: %s\n", i, fr_syserror(errno)); + if (fr_schedule_pthread_create(&sw->pthread_id, fr_schedule_worker_thread, sw) < 0) { + fr_log(sc->log, L_ERR, "Failed creating worker %d: %s\n", i, fr_strerror()); break; } sc->num_workers++; } + /* * Wait for all of the workers to signal us that either * they've started, OR there's been a problem and they diff --git a/src/lib/io/schedule.h b/src/lib/io/schedule.h index 37eb58d9380..e166f623ad9 100644 --- a/src/lib/io/schedule.h +++ b/src/lib/io/schedule.h @@ -48,6 +48,7 @@ typedef int (*fr_schedule_thread_instantiate_t)(TALLOC_CTX *ctx, fr_event_list_t int fr_schedule_worker_id(void); +int fr_schedule_pthread_create(pthread_t *thread, void *(*func)(void *), void *arg); fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el, fr_log_t *log, fr_log_lvl_t lvl, int max_inputs, int max_workers, fr_schedule_thread_instantiate_t worker_thread_instantiate, diff --git a/src/modules/proto_bfd/proto_bfd.c b/src/modules/proto_bfd/proto_bfd.c index 3d549e3fb50..f4ac416dd6f 100644 --- a/src/modules/proto_bfd/proto_bfd.c +++ b/src/modules/proto_bfd/proto_bfd.c @@ -323,7 +323,6 @@ static void *bfd_child_thread(void *ctx) static int bfd_pthread_create(bfd_state_t *session) { - int rcode; pthread_attr_t attr; if (pipe(session->pipefd) < 0) { @@ -365,12 +364,10 @@ static int bfd_pthread_create(bfd_state_t *session) * Note that the function returns non-zero on error, NOT * -1. The return code is the error, and errno isn't set. */ - rcode = pthread_create(&session->pthread_id, &attr, - bfd_child_thread, session); - if (rcode != 0) { + if (fr_schedule_pthread_create(&session->pthread_id, bfd_child_thread, session) < 0) { talloc_free(session->el); session->el = NULL; - ERROR("Thread create failed: %s", fr_syserror(rcode)); + ERROR("Thread create failed: %s", fr_strerror()); goto close_pipes; } pthread_attr_destroy(&attr); diff --git a/src/modules/rlm_sigtran/event.c b/src/modules/rlm_sigtran/event.c index 29bbf810f7e..e1e3466b11a 100644 --- a/src/modules/rlm_sigtran/event.c +++ b/src/modules/rlm_sigtran/event.c @@ -497,12 +497,12 @@ int sigtran_event_start(void) sem_init(&event_thread_running, 0, 0); if (sigtran_sccp_global_init() < 0) { - ERROR("osmocom thread - Failed initialising SCCP layer"); + ERROR("main thread - Failed initialising SCCP layer"); return -1; } - if (pthread_create(&event_thread, NULL, sigtran_event_loop, NULL) < 0) { - ERROR("osmocom thread - Failed spawning thread for multiplexer event loop: %s", fr_syserror(errno)); + if (fr_schedule_pthread_create(&event_thread, sigtran_event_loop, NULL) < 0) { + ERROR("main thread - Failed spawning thread for multiplexer event loop: %s", fr_syserror(errno)); return -1; } @@ -517,13 +517,13 @@ int sigtran_event_start(void) if ((sigtran_client_do_transaction(ctrl_pipe[0], txn) < 0) || (txn->response.type != SIGTRAN_RESPONSE_OK)) { - ERROR("osmocom thread - libosmo thread died"); + ERROR("main thread - libosmo thread died"); talloc_free(txn); return -1; } talloc_free(txn); - DEBUG2("osmocom thread - libosmo thread responding"); + DEBUG2("main thread - libosmo thread responding"); } #endif