Fix: memory/fd leak when cleaning streams in channel

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

diff --git a/src/common/consumer.c b/src/common/consumer.c
index 34751969dbedee0f6f35a08a057f836d32e609d8..c909548907d78c435392c4fa3c428ac0bab51d67 100644 (file)
--- 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 34344f4518eeff43b6bd73675bf906635f0531b5..b48a3c821d386fbf3cbfaceb8b01d9bbcc4f724d 100644 (file)
--- 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: