Fix: consumerd: HT init/teardown with program

Hash tables shared between threads should not be initialized by a
specific thread, because the other threads could start using it before
it is initialized.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: David Goulet <dgoulet@efficios.com>
Conflicts:
	src/bin/lttng-consumerd/lttng-consumerd.c
	src/common/consumer.c

diff --git a/src/bin/lttng-consumerd/lttng-consumerd.c b/src/bin/lttng-consumerd/lttng-consumerd.c
index 04bcef847d0f2196615aebb55e0a3a28c1d35e2a..f64ce15331b84253f399989b89b3969a65d2c2cc 100644 (file)
--- a/src/bin/lttng-consumerd/lttng-consumerd.c
+++ b/src/bin/lttng-consumerd/lttng-consumerd.c
@@ -318,7 +318,10 @@ int main(int argc, char **argv)
        }
 
        /* Init */
-       lttng_consumer_init();
+       if (lttng_consumer_init() < 0) {
+               goto error;
+       }
+
        /* Init socket timeouts */
        lttcomm_init();
        lttcomm_inet_init();

diff --git a/src/common/consumer.c b/src/common/consumer.c
index bd4eba093d22aa63d1f494e9c0fdb84d91a2fb5f..44a430f8f6c88414d757bbf02bede0169d02499d 100644 (file)
--- a/src/common/consumer.c
+++ b/src/common/consumer.c
@@ -1248,6 +1248,57 @@ error:
        return NULL;
 }
 
+/*
+ * Iterate over all streams of the hashtable and free them properly.
+ */
+static void destroy_data_stream_ht(struct lttng_ht *ht)
+{
+       struct lttng_ht_iter iter;
+       struct lttng_consumer_stream *stream;
+
+       if (ht == NULL) {
+               return;
+       }
+
+       rcu_read_lock();
+       cds_lfht_for_each_entry(ht->ht, &iter.iter, stream, node.node) {
+               /*
+                * Ignore return value since we are currently cleaning up so any error
+                * can't be handled.
+                */
+               (void) consumer_del_stream(stream, ht);
+       }
+       rcu_read_unlock();
+
+       lttng_ht_destroy(ht);
+}
+
+/*
+ * Iterate over all streams of the metadata hashtable and free them
+ * properly.
+ */
+static void destroy_metadata_stream_ht(struct lttng_ht *ht)
+{
+       struct lttng_ht_iter iter;
+       struct lttng_consumer_stream *stream;
+
+       if (ht == NULL) {
+               return;
+       }
+
+       rcu_read_lock();
+       cds_lfht_for_each_entry(ht->ht, &iter.iter, stream, node.node) {
+               /*
+                * Ignore return value since we are currently cleaning up so any error
+                * can't be handled.
+                */
+               (void) consumer_del_metadata_stream(stream, ht);
+       }
+       rcu_read_unlock();
+
+       lttng_ht_destroy(ht);
+}
+
 /*
  * Close all fds associated with the instance and free the context.
  */
@@ -1257,6 +1308,9 @@ void lttng_consumer_destroy(struct lttng_consumer_local_data *ctx)
 
        DBG("Consumer destroying it. Closing everything.");
 
+       destroy_data_stream_ht(data_ht);
+       destroy_metadata_stream_ht(metadata_ht);
+
        ret = close(ctx->consumer_error_socket);
        if (ret) {
                PERROR("close");
@@ -1803,60 +1857,6 @@ int lttng_consumer_recv_cmd(struct lttng_consumer_local_data *ctx,
        }
 }
 
-/*
- * Iterate over all streams of the hashtable and free them properly.
- *
- * WARNING: *MUST* be used with data stream only.
- */
-static void destroy_data_stream_ht(struct lttng_ht *ht)
-{
-       struct lttng_ht_iter iter;
-       struct lttng_consumer_stream *stream;
-
-       if (ht == NULL) {
-               return;
-       }
-
-       rcu_read_lock();
-       cds_lfht_for_each_entry(ht->ht, &iter.iter, stream, node.node) {
-               /*
-                * Ignore return value since we are currently cleaning up so any error
-                * can't be handled.
-                */
-               (void) consumer_del_stream(stream, ht);
-       }
-       rcu_read_unlock();
-
-       lttng_ht_destroy(ht);
-}
-
-/*
- * Iterate over all streams of the hashtable and free them properly.
- *
- * XXX: Should not be only for metadata stream or else use an other name.
- */
-static void destroy_stream_ht(struct lttng_ht *ht)
-{
-       struct lttng_ht_iter iter;
-       struct lttng_consumer_stream *stream;
-
-       if (ht == NULL) {
-               return;
-       }
-
-       rcu_read_lock();
-       cds_lfht_for_each_entry(ht->ht, &iter.iter, stream, node.node) {
-               /*
-                * Ignore return value since we are currently cleaning up so any error
-                * can't be handled.
-                */
-               (void) consumer_del_metadata_stream(stream, ht);
-       }
-       rcu_read_unlock();
-
-       lttng_ht_destroy(ht);
-}
-
 void lttng_consumer_close_metadata(void)
 {
        switch (consumer_data.type) {
@@ -2161,12 +2161,6 @@ void *consumer_thread_metadata_poll(void *data)
 
        rcu_register_thread();
 
-       metadata_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
-       if (!metadata_ht) {
-               /* ENOMEM at this point. Better to bail out. */
-               goto end_ht;
-       }
-
        DBG("Thread metadata poll started");
 
        /* Size is set to 1 for the consumer_metadata pipe */
@@ -2326,7 +2320,6 @@ end:
 
        lttng_poll_clean(&events);
 end_poll:
-       destroy_stream_ht(metadata_ht);
 end_ht:
        rcu_unregister_thread();
        return NULL;
@@ -2349,12 +2342,6 @@ void *consumer_thread_data_poll(void *data)
 
        rcu_register_thread();
 
-       data_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
-       if (data_ht == NULL) {
-               /* ENOMEM at this point. Better to bail out. */
-               goto end;
-       }
-
        local_stream = zmalloc(sizeof(struct lttng_consumer_stream *));
        if (local_stream == NULL) {
                PERROR("local_stream malloc");
@@ -2571,8 +2558,6 @@ end:
         */
        (void) lttng_pipe_write_close(ctx->consumer_metadata_pipe);
 
-       destroy_data_stream_ht(data_ht);
-
        rcu_unregister_thread();
        return NULL;
 }
@@ -3071,12 +3056,42 @@ int lttng_consumer_on_recv_stream(struct lttng_consumer_stream *stream)
 /*
  * Allocate and set consumer data hash tables.
  */
-void lttng_consumer_init(void)
+int lttng_consumer_init(void)
 {
        consumer_data.channel_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
+       if (!consumer_data.channel_ht) {
+               goto error;
+       }
+
        consumer_data.relayd_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
+       if (!consumer_data.relayd_ht) {
+               goto error;
+       }
+
        consumer_data.stream_list_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
+       if (!consumer_data.stream_list_ht) {
+               goto error;
+       }
+
        consumer_data.stream_per_chan_id_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
+       if (!consumer_data.stream_per_chan_id_ht) {
+               goto error;
+       }
+
+       data_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
+       if (!data_ht) {
+               goto error;
+       }
+
+       metadata_ht = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
+       if (!metadata_ht) {
+               goto error;
+       }
+
+       return 0;
+
+error:
+       return -1;
 }
 
 /*

diff --git a/src/common/consumer.h b/src/common/consumer.h
index 91f6b5ca15a6ddbdcbc8f28e641f824e34a4c64d..59499f5b511816eaf185782283271ac949f64345 100644 (file)
--- a/src/common/consumer.h
+++ b/src/common/consumer.h
@@ -492,7 +492,7 @@ struct lttng_consumer_global_data {
 /*
  * Init consumer data structures.
  */
-void lttng_consumer_init(void);
+int lttng_consumer_init(void);
 
 /*
  * Set the error socket for communication with a session daemon.