Fix: consumer output memory leak on creation

The sockets hash table was allocated two times hence losing the first
reference at the second allocation.

Furthermore, when a kernel/ust session is created, a default consumer is
allocated but was lost short after that when the tracing session current
consumer was copied and the pointer was overwritten.

Note that this fixes the memory leak but there is a code logic that
seems wrong when it comes to handle the consumer object trace path and
subdir during the session creation. A comment has been added and it
should be fixed.

Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c
index 1e20948887b1f889ae67bb81e3e844499edef869..aa050eceb56a48622ddfbf04b02c87d5f88ee9f6 100644 (file)
--- a/src/bin/lttng-sessiond/consumer.c
+++ b/src/bin/lttng-sessiond/consumer.c
@@ -314,6 +314,7 @@ void consumer_destroy_output(struct consumer_output *obj)
  */
 struct consumer_output *consumer_copy_output(struct consumer_output *obj)
 {
+       struct lttng_ht *tmp_ht_ptr;
        struct lttng_ht_iter iter;
        struct consumer_socket *socket, *copy_sock;
        struct consumer_output *output;
@@ -324,11 +325,13 @@ struct consumer_output *consumer_copy_output(struct consumer_output *obj)
        if (output == NULL) {
                goto error;
        }
+       /* Avoid losing the HT reference after the memcpy() */
+       tmp_ht_ptr = output->socks;
 
        memcpy(output, obj, sizeof(struct consumer_output));
 
-       /* Copy sockets */
-       output->socks = lttng_ht_new(0, LTTNG_HT_TYPE_ULONG);
+       /* Putting back the HT pointer and start copying socket(s). */
+       output->socks = tmp_ht_ptr;
 
        cds_lfht_for_each_entry(obj->socks->ht, &iter.iter, socket, node.node) {
                /* Create new socket object. */
@@ -337,6 +340,7 @@ struct consumer_output *consumer_copy_output(struct consumer_output *obj)
                        goto malloc_error;
                }
 
+               copy_sock->registered = socket->registered;
                copy_sock->lock = socket->lock;
                consumer_add_socket(copy_sock, output);
        }

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 2f0a39fede12d5e130d9fc8f88f011dd2f80dc96..b5a98432fb53468c6f1b1cc1dbb5e61918bddf95 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -1926,6 +1926,15 @@ static int copy_session_consumer(int domain, struct ltt_session *session)
        switch (domain) {
        case LTTNG_DOMAIN_KERNEL:
                DBG3("Copying tracing session consumer output in kernel session");
+               /*
+                * XXX: We should audit the session creation and what this function
+                * does "extra" in order to avoid a destroy since this function is used
+                * in the domain session creation (kernel and ust) only. Same for UST
+                * domain.
+                */
+               if (session->kernel_session->consumer) {
+                       consumer_destroy_output(session->kernel_session->consumer);
+               }
                session->kernel_session->consumer =
                        consumer_copy_output(session->consumer);
                /* Ease our life a bit for the next part */
@@ -1934,6 +1943,9 @@ static int copy_session_consumer(int domain, struct ltt_session *session)
                break;
        case LTTNG_DOMAIN_UST:
                DBG3("Copying tracing session consumer output in UST session");
+               if (session->ust_session->consumer) {
+                       consumer_destroy_output(session->ust_session->consumer);
+               }
                session->ust_session->consumer =
                        consumer_copy_output(session->consumer);
                /* Ease our life a bit for the next part */

diff --git a/src/bin/lttng-sessiond/trace-ust.c b/src/bin/lttng-sessiond/trace-ust.c
index 3e382b7a5fb738eb11f7fb5041bce65657067249..864a4b0d1bb0dcd25c2e2de54b16999f8d3d22cc 100644 (file)
--- a/src/bin/lttng-sessiond/trace-ust.c
+++ b/src/bin/lttng-sessiond/trace-ust.c
@@ -111,7 +111,7 @@ struct ltt_ust_session *trace_ust_create_session(char *path,
 
        lus->consumer = consumer_create_output(CONSUMER_DST_LOCAL);
        if (lus->consumer == NULL) {
-               goto error_free_session;
+               goto error_consumer;
        }
 
        /*
@@ -128,7 +128,7 @@ struct ltt_ust_session *trace_ust_create_session(char *path,
                                "%s" DEFAULT_UST_TRACE_DIR, path);
                if (ret < 0) {
                        PERROR("snprintf UST consumer trace path");
-                       goto error;
+                       goto error_path;
                }
 
                /* Set session path */
@@ -136,7 +136,7 @@ struct ltt_ust_session *trace_ust_create_session(char *path,
                                path);
                if (ret < 0) {
                        PERROR("snprintf kernel traces path");
-                       goto error_free_session;
+                       goto error_path;
                }
        }
 
@@ -144,7 +144,9 @@ struct ltt_ust_session *trace_ust_create_session(char *path,
 
        return lus;
 
-error_free_session:
+error_path:
+       consumer_destroy_output(lus->consumer);
+error_consumer:
        lttng_ht_destroy(lus->domain_global.channels);
        lttng_ht_destroy(lus->domain_exec);
        lttng_ht_destroy(lus->domain_pid);