From 88ebf5a7a2c9b4d56953a6daa66715e4043f049c Mon Sep 17 00:00:00 2001 From: Jonathan Rajotte Date: Thu, 25 Jun 2020 16:52:33 -0400 Subject: [PATCH] Fix: consumer: dangling chunk on buffer allocation failure MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jérémie Galarneau Change-Id: Idf19b1d306cf347619ccfda1621d91d8303f3c7c --- src/bin/lttng-sessiond/ust-app.c | 19 ++++++++++++------- src/common/ust-consumer/ust-consumer.c | 5 +++++ 2 files changed, 17 insertions(+), 7 deletions(-) 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); } -- 2.34.1