Fix: increment UST channel refcount at stream creation

Having the channel refcount incremented only when the stream is sent to a
thread could triggered a segfault with a race between a channel destroyed by
the channel thread and the stream pointers being inflight in the thread pipe.

Once read from the pipe, the streams are added to the data thread poll set and
immediately destroyed since the channel has hung up previously but the flush
buffer segfaulted on the channel that was already deleted by the channel thread
with a refcount set to 0.

This is fixed by incrementing the channel refcount once the stream is created
and the channel reference is set to it. This is done *before* the channel
object is visible in the channel thread. Now the channel thread detects the
hang up but will not destroy the channel because of the refcount > 0. The
channel cleanup will be done by the last stream hanging up.

Fixes #530

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/common/consumer.c b/src/common/consumer.c
index 2dd463c1431f72e9fd1bf35d8551fddad5b28f4a..e26388f8e50fab81d5df0999ab5fb73948e8faba 100644 (file)
--- a/src/common/consumer.c
+++ b/src/common/consumer.c
@@ -660,9 +660,6 @@ static int add_stream(struct lttng_consumer_stream *stream,
                uatomic_inc(&relayd->refcount);
        }
 
-       /* Update channel refcount once added without error(s). */
-       uatomic_inc(&stream->chan->refcount);
-
        /*
         * When nb_init_stream_left reaches 0, we don't need to trigger any action
         * in terms of destroying the associated channel, because the action that

diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index 031a7cb261a52fa8d6b0c4c53be291be5896b973..01fca9b7fcdd8efd82f56c4fcc455aa1dc756d21 100644 (file)
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -278,6 +278,12 @@ static int create_ust_streams(struct lttng_consumer_channel *channel,
                 */
                stream->wait_fd = wait_fd;
 
+               /*
+                * Increment channel refcount since the channel reference has now been
+                * assigned in the allocation process above.
+                */
+               uatomic_inc(&stream->chan->refcount);
+
                /*
                 * Order is important this is why a list is used. On error, the caller
                 * should clean this list.