From 212d67a218a0e805950f85bd95143a996e957322 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 11 Feb 2014 13:16:45 -0500 Subject: [PATCH] Fix: memory/fd leak when cleaning streams in channel Signed-off-by: David Goulet --- src/common/consumer.c | 51 +++++++++++++++----------- src/common/ust-consumer/ust-consumer.c | 5 ++- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/common/consumer.c b/src/common/consumer.c index 34751969d..c90954890 100644 --- a/src/common/consumer.c +++ b/src/common/consumer.c @@ -150,6 +150,31 @@ error: return (int) ret; } +/* + * Cleanup the stream list of a channel. Those streams are not yet globally + * visible + */ +static void clean_channel_stream_list(struct lttng_consumer_channel *channel) +{ + struct lttng_consumer_stream *stream, *stmp; + + assert(channel); + + /* Delete streams that might have been left in the stream list. */ + cds_list_for_each_entry_safe(stream, stmp, &channel->streams.head, + send_node) { + cds_list_del(&stream->send_node); + /* + * Once a stream is added to this list, the buffers were created so we + * have a guarantee that this call will succeed. Setting the monitor + * mode to 0 so we don't lock nor try to delete the stream from the + * global hash table. + */ + stream->monitor = 0; + consumer_stream_destroy(stream, NULL); + } +} + /* * Find a stream. The consumer_data.lock must be locked during this * call. @@ -292,23 +317,14 @@ void consumer_del_channel(struct lttng_consumer_channel *channel) { int ret; struct lttng_ht_iter iter; - struct lttng_consumer_stream *stream, *stmp; DBG("Consumer delete channel key %" PRIu64, channel->key); pthread_mutex_lock(&consumer_data.lock); pthread_mutex_lock(&channel->lock); - /* Delete streams that might have been left in the stream list. */ - cds_list_for_each_entry_safe(stream, stmp, &channel->streams.head, - send_node) { - cds_list_del(&stream->send_node); - /* - * Once a stream is added to this list, the buffers were created so - * we have a guarantee that this call will succeed. - */ - consumer_stream_destroy(stream, NULL); - } + /* Destroy streams that might have been left in the stream list. */ + clean_channel_stream_list(channel); if (channel->live_timer_enabled == 1) { consumer_timer_live_stop(channel); @@ -2817,8 +2833,6 @@ restart: break; case CONSUMER_CHANNEL_DEL: { - struct lttng_consumer_stream *stream, *stmp; - /* * This command should never be called if the channel * has streams monitored by either the data or metadata @@ -2845,14 +2859,9 @@ restart: break; case LTTNG_CONSUMER32_UST: case LTTNG_CONSUMER64_UST: - /* Delete streams that might have been left in the stream list. */ - cds_list_for_each_entry_safe(stream, stmp, &chan->streams.head, - send_node) { - health_code_update(); - /* Remove from the list and destroy it. */ - cds_list_del(&stream->send_node); - consumer_stream_destroy(stream, NULL); - } + health_code_update(); + /* Destroy streams that might have been left in the stream list. */ + clean_channel_stream_list(chan); break; default: ERR("Unknown consumer_data type"); diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index 34344f451..b48a3c821 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -749,8 +749,9 @@ error: * the stream is still in the local stream list of the channel. This call * will make sure to clean that list. */ - cds_list_del(&metadata->metadata_stream->send_node); consumer_stream_destroy(metadata->metadata_stream, NULL); + cds_list_del(&metadata->metadata_stream->send_node); + metadata->metadata_stream = NULL; error_no_stream: end: return ret; @@ -842,8 +843,8 @@ error_stream: * Clean up the stream completly because the next snapshot will use a new * metadata stream. */ - cds_list_del(&metadata_stream->send_node); consumer_stream_destroy(metadata_stream, NULL); + cds_list_del(&metadata_stream->send_node); metadata_channel->metadata_stream = NULL; error: -- 2.34.1