From: David Goulet <dgoulet@efficios.com>
Date: Fri, 26 Oct 2012 16:20:41 +0000 (-0400)
Subject: Fix: consumer output memory leak on creation
X-Git-Tag: v2.1.0-rc6~16
X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=09a90bcd757486eebb38d6d6023c603d9af72b4e

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 1e2094888..aa050eceb 100644
--- 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 2f0a39fed..b5a98432f 100644
--- 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 3e382b7a5..864a4b0d1 100644
--- 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);