From ea88ca2a0b3fa898e245893c43ae2185da0acc44 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 1 May 2013 16:52:59 -0400 Subject: [PATCH] Fix: consumerd metadata channel/cache/timer races There were various races with the way metadata was handled for UST. Fix this by: - Ensuring that metadata cache is destroyed only when the metadata channel is destroyed (it is already protected by refcounting). - Ensuring that close_metadata() command (issued by sessiond) just closes the stream wakeup_fd, and let the metadata management thread (which has ownership of the metadata stream) perform the teardown. - Ensure that switch timer, for metadata, is always stopped before any of the metadata stream or channel teardown is done. Signed-off-by: Mathieu Desnoyers Signed-off-by: David Goulet --- src/common/ust-consumer/ust-consumer.c | 40 ++++++++++++++++++-------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index 7d9444cd0..031a7cb26 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -610,7 +610,7 @@ error: */ static int close_metadata(uint64_t chan_key) { - int ret; + int ret = 0; struct lttng_consumer_channel *channel; DBG("UST consumer close metadata key %" PRIu64, chan_key); @@ -622,18 +622,22 @@ static int close_metadata(uint64_t chan_key) goto error; } - ret = ustctl_stream_close_wakeup_fd(channel->metadata_stream->ustream); - if (ret < 0) { - ERR("UST consumer unable to close fd of metadata (ret: %d)", ret); - ret = LTTCOMM_CONSUMERD_ERROR_METADATA; - goto error; - } - if (channel->switch_timer_enabled == 1) { - DBG("Deleting timer on metadata channel"); - consumer_timer_switch_stop(channel); + pthread_mutex_lock(&consumer_data.lock); + if (!cds_lfht_is_node_deleted(&channel->node.node)) { + if (channel->switch_timer_enabled == 1) { + DBG("Deleting timer on metadata channel"); + consumer_timer_switch_stop(channel); + } + ret = ustctl_stream_close_wakeup_fd(channel->metadata_stream->ustream); + if (ret < 0) { + ERR("UST consumer unable to close fd of metadata (ret: %d)", ret); + ret = LTTCOMM_CONSUMERD_ERROR_METADATA; + goto error_unlock; + } } - consumer_metadata_cache_destroy(channel); +error_unlock: + pthread_mutex_unlock(&consumer_data.lock); error: return ret; } @@ -923,10 +927,15 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx, */ ret = add_channel(channel, ctx); if (ret < 0) { + if (msg.u.ask_channel.type == LTTNG_UST_CHAN_METADATA) { + if (channel->switch_timer_enabled == 1) { + consumer_timer_switch_stop(channel); + } + consumer_metadata_cache_destroy(channel); + } goto end_channel_error; } - /* * Channel and streams are now created. Inform the session daemon that * everything went well and should wait to receive the channel and @@ -1193,6 +1202,10 @@ void lttng_ustconsumer_del_channel(struct lttng_consumer_channel *chan) assert(chan); assert(chan->uchan); + if (chan->switch_timer_enabled == 1) { + consumer_timer_switch_stop(chan); + } + consumer_metadata_cache_destroy(chan); ustctl_destroy_channel(chan->uchan); } @@ -1201,6 +1214,9 @@ void lttng_ustconsumer_del_stream(struct lttng_consumer_stream *stream) assert(stream); assert(stream->ustream); + if (stream->chan->switch_timer_enabled == 1) { + consumer_timer_switch_stop(stream->chan); + } ustctl_destroy_stream(stream->ustream); } -- 2.34.1