From 282dadbc28bdf6c8565bd0cabd6a2fbffb6b9396 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Sun, 26 Jan 2014 22:39:44 -0500 Subject: [PATCH] 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 Signed-off-by: David Goulet --- src/bin/lttng-consumerd/lttng-consumerd.c | 8 +- src/common/consumer.c | 156 ++++++++++++---------- src/common/consumer.h | 2 +- 3 files changed, 90 insertions(+), 76 deletions(-) diff --git a/src/bin/lttng-consumerd/lttng-consumerd.c b/src/bin/lttng-consumerd/lttng-consumerd.c index 4a9a7a492..d9ec0f33e 100644 --- a/src/bin/lttng-consumerd/lttng-consumerd.c +++ b/src/bin/lttng-consumerd/lttng-consumerd.c @@ -341,7 +341,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(); @@ -405,9 +408,6 @@ int main(int argc, char **argv) ctx->type = opt_type; - /* Initialize communication library */ - lttcomm_init(); - ret = utils_create_pipe(health_quit_pipe); if (ret < 0) { goto error_health_pipe; diff --git a/src/common/consumer.c b/src/common/consumer.c index f47d8de1b..cbcb2f74d 100644 --- a/src/common/consumer.c +++ b/src/common/consumer.c @@ -1311,6 +1311,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. */ @@ -1320,6 +1371,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"); @@ -1894,60 +1948,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) { @@ -2256,12 +2256,6 @@ void *consumer_thread_metadata_poll(void *data) health_code_update(); - 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 */ @@ -2434,8 +2428,6 @@ end: lttng_poll_clean(&events); end_poll: - destroy_stream_ht(metadata_ht); -end_ht: if (err) { health_error(); ERR("Health error occurred in %s", __func__); @@ -2466,12 +2458,6 @@ void *consumer_thread_data_poll(void *data) health_code_update(); - 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"); @@ -2701,8 +2687,6 @@ end: */ (void) lttng_pipe_write_close(ctx->consumer_metadata_pipe); - destroy_data_stream_ht(data_ht); - if (err) { health_error(); ERR("Health error occurred in %s", __func__); @@ -3256,12 +3240,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 8d7e1d0a4..a7517a340 100644 --- a/src/common/consumer.h +++ b/src/common/consumer.h @@ -512,7 +512,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. -- 2.34.1