Fix: metadata stream is not marked as quiescent after packet commit

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 <jeremie.galarneau@efficios.com>

diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index 7547e0b853d1dae955af3daaf6d6fe263621a69a..a951788bd9025ee0c95a6db925f5313758696f2c 100644 (file)
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -2264,6 +2264,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;
@@ -2308,7 +2315,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) {
@@ -2498,7 +2504,6 @@ retry:
                        if (ret <= 0) {
                                goto end;
                        }
-                       ustctl_flush_buffer(stream->ustream, 1);
                        goto retry;
                }