From: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Date: Thu, 25 Jun 2020 20:52:33 +0000 (-0400)
Subject: Fix: consumer: dangling chunk on buffer allocation failure
X-Git-Tag: v2.12.2~26
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=ab4df0529ae14c581c257f22f74ea35c04ecd8db;p=lttng-tools.git

Fix: consumer: dangling chunk on buffer allocation failure

Observed issue
==============

Using docker the /dev/shm mount size is 64 MB by default. On system
with many threads (256), combined with the default subbuffer count and
subbufer size, allocation failure of the subbuffer is inevitable since
the /dev/shm mountpoint is too small.

When a user try to destroy the problematic session, the destroy
command hangs.

  # Force the size of /dev/shm, make sure to remount it to its orignal
  # size when done testing.
  sudo mount -o remount,size=1G /dev/shm
  lttng create
  lttng enable-channel --subbuf-size 500M -u test
  lttng enable-event -u -a -c test
  lttng start
  # Run an app;
  ../lttng-ust/doc/examples/easy-ust/sample

lttng-sessiond should output:
  Error: ask_channel_creation consumer command failed
  Error: Error creating UST channel "test" on the consumer daemon

  lttng destroy

Output hang:
  Destroying session auto-20200626-112733..........

Cause
=====

The hang is caused by the check of ongoing rotation which never finishes.

The consumer reports that the trace chunk still exists. This is caused
by an imbalance of the reference count for the trace chunk on close.

This is caused by a missing lttng_trace_chunk_put in destroy_channel
which is called on the error path for the consumer channel creation.

Solution
========

Call lttng_trace_chunk_put if the channel->trace_chunk is assigned
inside the channel_destroy function.

The error handling in ust_app_channel_create is also reworked since the
return value for ust_app_channel_send would be squashed for scenario
where contexts are present.

Known drawbacks
=========

None

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Idf19b1d306cf347619ccfda1621d91d8303f3c7c
---

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index a4fea821a..8c314ec1c 100644
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -4112,11 +4112,14 @@ int ust_app_channel_create(struct ltt_ust_session *usess,
 		ret = ust_app_channel_allocate(ua_sess, uchan,
 			LTTNG_UST_CHAN_PER_CPU, usess,
 			&ua_chan);
-		if (ret == 0) {
-			ret = ust_app_channel_send(app, usess,
-				ua_sess, ua_chan);
-		} else {
-			goto end;
+		if (ret < 0) {
+			goto error;
+		}
+
+		ret = ust_app_channel_send(app, usess,
+			ua_sess, ua_chan);
+		if (ret) {
+			goto error;
 		}
 
 		/* Add contexts. */
@@ -4124,10 +4127,12 @@ int ust_app_channel_create(struct ltt_ust_session *usess,
 			ret = create_ust_app_channel_context(ua_chan,
 				&uctx->ctx, app);
 			if (ret) {
-				goto end;
+				goto error;
 			}
 		}
 	}
+
+error:
 	if (ret < 0) {
 		switch (ret) {
 		case -ENOTCONN:
@@ -4143,7 +4148,7 @@ int ust_app_channel_create(struct ltt_ust_session *usess,
 			break;
 		}
 	}
-end:
+
 	if (ret == 0 && _ua_chan) {
 		/*
 		 * Only return the application's channel on success. Note
diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index 6a561b4ca..e5cfbd912 100644
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -78,6 +78,11 @@ static void destroy_channel(struct lttng_consumer_channel *channel)
 		lttng_ustconsumer_del_channel(channel);
 		lttng_ustconsumer_free_channel(channel);
 	}
+
+	if (channel->trace_chunk) {
+		lttng_trace_chunk_put(channel->trace_chunk);
+	}
+
 	free(channel);
 }