From 0d88e04674ead21c741c6f4ed7fadf666c5e7bce Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Mon, 10 Jun 2019 13:31:31 -0400 Subject: [PATCH] Fix: metadata stream is not marked as quiescent after packet commit MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When a metadata stream's wait fd is hung-up or enters an error state, it is checked for quiescence in lttng_ustconsumer_on_stream_hangup(). If the stream is not quiescent, the current packet is closed through the flush_buffer operation. Currently, all commits to metadata streams are done on a packet basis. The various code paths using the commit_one_metadata_packet helper all perform a flush directly after the commit. Performing this flush leaves the stream in a "quiescent" state, but does not mark it as such. This results in an extraneous flush being performed in the err/hup handler, which leaves an empty packet to be consumed. This packet is then consumed during the execution of the err/hup handler. This bug results in an empty packet being appended to metadata streams. This packet is typically ignored by readers, but the fact that it is written at the time of the destruction of a session violates the immutability guarantee of the session stop command. Moreover, following the introduction of trace chunks, this results in the stream attempting to serialize the empty buffer to its output file _after_ its trace chunk has been closed, causing an assertion to hit. Hence, this fix performs the buffer flush and sets the stream as quiescent directly in commit_one_metadata_packet(). Signed-off-by: Jérémie Galarneau --- src/common/ust-consumer/ust-consumer.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index 83b2143d7..ff9d31d52 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -2513,6 +2513,13 @@ int commit_one_metadata_packet(struct lttng_consumer_stream *stream) stream->ust_metadata_pushed); ret = write_len; + /* + * Switch packet (but don't open the next one) on every commit of + * a metadata packet. Since the subbuffer is fully filled (with padding, + * if needed), the stream is "quiescent" after this commit. + */ + ustctl_flush_buffer(stream->ustream, 1); + stream->quiescent = true; end: pthread_mutex_unlock(&stream->chan->metadata_cache->lock); return ret; @@ -2557,7 +2564,6 @@ int lttng_ustconsumer_sync_metadata(struct lttng_consumer_local_data *ctx, retry = 1; } - ustctl_flush_buffer(metadata->ustream, 1); ret = ustctl_snapshot(metadata->ustream); if (ret < 0) { if (errno != EAGAIN) { @@ -2761,7 +2767,6 @@ retry: if (ret <= 0) { goto error; } - ustctl_flush_buffer(stream->ustream, 1); goto retry; } -- 2.34.1