Fix: join consumer timer thread

Detaching the timer thread has the unfortunate side-effect of letting
the health management data structures be freed by main() while the timer
thread may still be using them (if, e.g., main() exits quickly).

Overcome this situation by tearing down and joining the timer thread.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-consumerd/lttng-consumerd.c b/src/bin/lttng-consumerd/lttng-consumerd.c
index 2f8eed1085dfed209ba5f00979621e2f603d44be..50e2f4648befe1e5858457d7e198c04911737f37 100644 (file)
--- a/src/bin/lttng-consumerd/lttng-consumerd.c
+++ b/src/bin/lttng-consumerd/lttng-consumerd.c
@@ -56,6 +56,7 @@
 
 static pthread_t channel_thread, data_thread, metadata_thread,
                sessiond_thread, metadata_timer_thread, health_thread;
+static bool metadata_timer_thread_online;
 
 /* to count the number of times the user pressed ctrl+c */
 static int sigintcount = 0;
@@ -506,6 +507,20 @@ int main(int argc, char **argv)
        }
        cmm_smp_mb();   /* Read ready before following operations */
 
+       /*
+        * Create the thread to manage the UST metadata periodic timer and
+        * live timer.
+        */
+       ret = pthread_create(&metadata_timer_thread, NULL,
+                       consumer_timer_thread, (void *) ctx);
+       if (ret) {
+               errno = ret;
+               PERROR("pthread_create");
+               retval = -1;
+               goto exit_metadata_timer_thread;
+       }
+       metadata_timer_thread_online = true;
+
        /* Create thread to manage channels */
        ret = pthread_create(&channel_thread, default_pthread_attr(),
                        consumer_thread_channel_poll,
@@ -549,34 +564,12 @@ int main(int argc, char **argv)
                goto exit_sessiond_thread;
        }
 
-       /*
-        * Create the thread to manage the UST metadata periodic timer and
-        * live timer.
-        */
-       ret = pthread_create(&metadata_timer_thread, default_pthread_attr(),
-                       consumer_timer_thread, (void *) ctx);
-       if (ret) {
-               errno = ret;
-               PERROR("pthread_create");
-               retval = -1;
-               goto exit_metadata_timer_thread;
-       }
-
-       ret = pthread_detach(metadata_timer_thread);
-       if (ret) {
-               errno = ret;
-               PERROR("pthread_detach");
-               retval = -1;
-               goto exit_metadata_timer_detach;
-       }
 
        /*
         * This is where we start awaiting program completion (e.g. through
         * signal that asks threads to teardown.
         */
 
-exit_metadata_timer_detach:
-exit_metadata_timer_thread:
        ret = pthread_join(sessiond_thread, &status);
        if (ret) {
                errno = ret;
@@ -609,6 +602,8 @@ exit_metadata_thread:
        }
 exit_channel_thread:
 
+exit_metadata_timer_thread:
+
        ret = pthread_join(health_thread, &status);
        if (ret) {
                errno = ret;
@@ -621,17 +616,38 @@ exit_health_thread:
 exit_health_pipe:
 
 exit_init_data:
-       tmp_ctx = ctx;
-       ctx = NULL;
-       cmm_barrier();  /* Clear ctx for signal handler. */
        /*
         * Wait for all pending call_rcu work to complete before tearing
         * down data structures. call_rcu worker may be trying to
         * perform lookups in those structures.
         */
        rcu_barrier();
-       lttng_consumer_destroy(tmp_ctx);
        lttng_consumer_cleanup();
+       /*
+        * Tearing down the metadata timer thread in a
+        * non-fully-symmetric fashion compared to its creation in case
+        * lttng_consumer_cleanup() ends up tearing down timers (which
+        * requires the timer thread to be alive).
+        */
+       if (metadata_timer_thread_online) {
+               /*
+                * Ensure the metadata timer thread exits only after all other
+                * threads are gone, because it is required to perform timer
+                * teardown synchronization.
+                */
+               kill(getpid(), LTTNG_CONSUMER_SIG_EXIT);
+               ret = pthread_join(metadata_timer_thread, &status);
+               if (ret) {
+                       errno = ret;
+                       PERROR("pthread_join metadata_timer_thread");
+                       retval = -1;
+               }
+               metadata_timer_thread_online = false;
+       }
+       tmp_ctx = ctx;
+       ctx = NULL;
+       cmm_barrier();  /* Clear ctx for signal handler. */
+       lttng_consumer_destroy(tmp_ctx);
 
        if (health_consumerd) {
                health_app_destroy(health_consumerd);

diff --git a/src/common/consumer/consumer-timer.c b/src/common/consumer/consumer-timer.c
index 1ba648c5fd97524e166ac4baab8adb1ff8836201..8c39275d55b3af1d0978e3bad68c9c0326239a08 100644 (file)
--- a/src/common/consumer/consumer-timer.c
+++ b/src/common/consumer/consumer-timer.c
@@ -61,6 +61,10 @@ static void setmask(sigset_t *mask)
        if (ret) {
                PERROR("sigaddset live");
        }
+       ret = sigaddset(mask, LTTNG_CONSUMER_SIG_EXIT);
+       if (ret) {
+               PERROR("sigaddset exit");
+       }
 }
 
 /*
@@ -546,7 +550,8 @@ int consumer_signal_init(void)
 
 /*
  * This thread is the sighandler for signals LTTNG_CONSUMER_SIG_SWITCH,
- * LTTNG_CONSUMER_SIG_TEARDOWN and LTTNG_CONSUMER_SIG_LIVE.
+ * LTTNG_CONSUMER_SIG_TEARDOWN, LTTNG_CONSUMER_SIG_LIVE, and
+ * LTTNG_CONSUMER_SIG_EXIT.
  */
 void *consumer_timer_thread(void *data)
 {
@@ -589,6 +594,9 @@ void *consumer_timer_thread(void *data)
                        DBG("Signal timer metadata thread teardown");
                } else if (signr == LTTNG_CONSUMER_SIG_LIVE) {
                        live_timer(ctx, info.si_signo, &info, NULL);
+               } else if (signr == LTTNG_CONSUMER_SIG_EXIT) {
+                       assert(consumer_quit);
+                       goto end;
                } else {
                        ERR("Unexpected signal %d\n", info.si_signo);
                }
@@ -597,10 +605,8 @@ void *consumer_timer_thread(void *data)
 error_testpoint:
        /* Only reached in testpoint error */
        health_error();
+end:
        health_unregister(health_consumerd);
-
        rcu_unregister_thread();
-
-       /* Never return */
        return NULL;
 }

diff --git a/src/common/consumer/consumer-timer.h b/src/common/consumer/consumer-timer.h
index 22e74574ce5622d7ec7e242b83865fa766b1f639..a970945c6a4789d1d52638aa6a1685a77fb66a2b 100644 (file)
--- a/src/common/consumer/consumer-timer.h
+++ b/src/common/consumer/consumer-timer.h
@@ -27,6 +27,7 @@
 #define LTTNG_CONSUMER_SIG_SWITCH      SIGRTMIN + 10
 #define LTTNG_CONSUMER_SIG_TEARDOWN    SIGRTMIN + 11
 #define LTTNG_CONSUMER_SIG_LIVE                SIGRTMIN + 12
+#define LTTNG_CONSUMER_SIG_EXIT                SIGRTMIN + 14
 
 #define CLOCKID CLOCK_MONOTONIC
 

diff --git a/src/common/consumer/consumer.h b/src/common/consumer/consumer.h
index 66092d3c5bc5888b0c2aa603a9c2255532c50af8..e1a4b7e9e44f39f5bd9b60cc750ac78ef98feac3 100644 (file)
--- a/src/common/consumer/consumer.h
+++ b/src/common/consumer/consumer.h
@@ -593,6 +593,12 @@ struct lttng_consumer_global_data {
        struct lttng_ht *stream_per_chan_id_ht;
 };
 
+/*
+ * Set to nonzero when the consumer is exiting. Updated by signal
+ * handler and thread exit, read by threads.
+ */
+extern volatile int consumer_quit;
+
 /*
  * Init consumer data structures.
  */