From a6ef8ee6a852f9fa6394408fcc8d4d49891852d9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Sat, 24 Aug 2019 16:58:48 -0700 Subject: [PATCH] Clean-up: set stream's channel pointer to NULL after releasing ref MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit A stream's "chan" pointer to its parent channel remains set after the reference to the channel has been released. This can lead to accidental uses after the release of the channel through the stream object. Signed-off-by: Jérémie Galarneau --- src/common/consumer/consumer.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/common/consumer/consumer.c b/src/common/consumer/consumer.c index 82fe0168f..82be751c4 100644 --- a/src/common/consumer/consumer.c +++ b/src/common/consumer/consumer.c @@ -2264,7 +2264,8 @@ void lttng_consumer_close_all_metadata(void) void consumer_del_metadata_stream(struct lttng_consumer_stream *stream, struct lttng_ht *ht) { - struct lttng_consumer_channel *free_chan = NULL; + struct lttng_consumer_channel *channel = NULL; + bool free_channel = false; assert(stream); /* @@ -2276,11 +2277,17 @@ void consumer_del_metadata_stream(struct lttng_consumer_stream *stream, DBG3("Consumer delete metadata stream %d", stream->wait_fd); pthread_mutex_lock(&consumer_data.lock); - pthread_mutex_lock(&stream->chan->lock); + /* + * Note that this assumes that a stream's channel is never changed and + * that the stream's lock doesn't need to be taken to sample its + * channel. + */ + channel = stream->chan; + pthread_mutex_lock(&channel->lock); pthread_mutex_lock(&stream->lock); - if (stream->chan->metadata_cache) { + if (channel->metadata_cache) { /* Only applicable to userspace consumers. */ - pthread_mutex_lock(&stream->chan->metadata_cache->lock); + pthread_mutex_lock(&channel->metadata_cache->lock); } /* Remove any reference to that stream. */ @@ -2292,28 +2299,29 @@ void consumer_del_metadata_stream(struct lttng_consumer_stream *stream, consumer_stream_destroy_buffers(stream); /* Atomically decrement channel refcount since other threads can use it. */ - if (!uatomic_sub_return(&stream->chan->refcount, 1) - && !uatomic_read(&stream->chan->nb_init_stream_left)) { + if (!uatomic_sub_return(&channel->refcount, 1) + && !uatomic_read(&channel->nb_init_stream_left)) { /* Go for channel deletion! */ - free_chan = stream->chan; + free_channel = true; } + stream->chan = NULL; /* * Nullify the stream reference so it is not used after deletion. The * channel lock MUST be acquired before being able to check for a NULL * pointer value. */ - stream->chan->metadata_stream = NULL; + channel->metadata_stream = NULL; - if (stream->chan->metadata_cache) { - pthread_mutex_unlock(&stream->chan->metadata_cache->lock); + if (channel->metadata_cache) { + pthread_mutex_unlock(&channel->metadata_cache->lock); } pthread_mutex_unlock(&stream->lock); - pthread_mutex_unlock(&stream->chan->lock); + pthread_mutex_unlock(&channel->lock); pthread_mutex_unlock(&consumer_data.lock); - if (free_chan) { - consumer_del_channel(free_chan); + if (free_channel) { + consumer_del_channel(channel); } lttng_trace_chunk_put(stream->trace_chunk); -- 2.34.1