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 0b356a8b85bdfc7b30270c8598280d965688fada..86f74b8b27510579ce2748ec06a05af0d2e8c74a 100644 (file)
--- 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 37eb58d938079d7e759e5d18dabb63d937335ace..e166f623ad91f91c52c77e3a7106ea71731830a1 100644 (file)
--- 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 3d549e3fb5027af1a3b586aa2679a59b4485754f..f4ac416dd6f0741f95974d3b98f01465d747a278 100644 (file)
--- 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 29bbf810f7ec232b8734c85ca4895dc4a9dfb06e..e1e3466b11a2d1d3bf40651be6b05191cddbecb2 100644 (file)
--- 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