Fix: mark consumer channels as logically deleted during deletion

A consumer channel that is "logically" deleted (but not yet reclaimed
with regards to RCU) can be iterated-on when creating a trace chunk.

Such a deleted channel will have released its reference to its
previous trace chunk, but the trace chunk creation operation will
set its current trace chunk to the new trace chunk, thus acquiring
a new reference to the trace chunk.

The clean-up performed by the RCU thread already assumes that a
logical deletion already took place and will not re-release the
reference to the channel's current trace chunk.

In effect, the reference to the trace chunk is leaked and will
prevent any rotation out of the trace chunk from completing.

Reported-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/common/consumer/consumer.c b/src/common/consumer/consumer.c
index 8483146c65ab6ecf3d1b026d1f33c62be4442c8a..82fe0168f2c85b11cac63390ddd427a763dc4149 100644 (file)
--- a/src/common/consumer/consumer.c
+++ b/src/common/consumer/consumer.c
@@ -411,7 +411,8 @@ void consumer_del_channel(struct lttng_consumer_channel *channel)
                rcu_read_unlock();
        }
 
-        call_rcu(&channel->node.head, free_channel_rcu);
+       channel->is_deleted = true;
+       call_rcu(&channel->node.head, free_channel_rcu);
 end:
        pthread_mutex_unlock(&channel->lock);
        pthread_mutex_unlock(&consumer_data.lock);
@@ -1021,6 +1022,16 @@ int lttng_consumer_channel_set_trace_chunk(
        unsigned long channel_hash;
 
        pthread_mutex_lock(&channel->lock);
+       if (channel->is_deleted) {
+               /*
+                * The channel has been logically deleted and should no longer
+                * be used. It has released its reference to its current trace
+                * chunk and should not acquire a new one.
+                *
+                * Return success as there is nothing for the caller to do.
+                */
+               goto end;
+       }
        /*
         * A stream can transition to a state where it and its channel
         * no longer belong to a trace chunk. For instance, this happens when

diff --git a/src/common/consumer/consumer.h b/src/common/consumer/consumer.h
index 26492c0e4a49f229a958b86a374cfb4938d72f7f..13fc61617881de52d86d2839262aaacca7f0d906 100644 (file)
--- a/src/common/consumer/consumer.h
+++ b/src/common/consumer/consumer.h
@@ -107,6 +107,12 @@ struct consumer_metadata_cache;
 struct lttng_consumer_channel {
        /* Is the channel published in the channel hash tables? */
        bool is_published;
+       /*
+        * Was the channel deleted (logically) and waiting to be reclaimed?
+        * If this flag is set, no modification that is not cleaned-up by the
+        * RCU reclamation callback should be made
+        */
+       bool is_deleted;
        /* HT node used for consumer_data.channel_ht */
        struct lttng_ht_node_u64 node;
        /* HT node used for consumer_data.channels_by_session_id_ht */