From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Tue, 3 Dec 2019 09:08:36 +0000 (-0500)
Subject: Fix: relayd: per-pid live: no new metadata vs close
X-Git-Tag: v2.12.0-rc1~165
X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=94f73d0839c106a783f3c805aa74a2dbfb6bd5f5

Fix: relayd: per-pid live: no new metadata vs close

When using lttng-live on a per-pid UST trace, the metadata stream is
closed after an application exits. The live viewer observes that the
stream has no more data when getting the metadata returns 0 bytes.

Internally in the relay daemon, it is assumed that a metadata stream
can be put (and thus removed) as soon as all the metadata has been sent
to the viewer, but this is not quite right. The viewer actually needs to
observe a 0-byte reply after receiving all the metadata in order to
gracefully perceive the metadata stream hang up.

Therefore, add a no_new_metadata_notified flag to the metadata stream
to track whether that 0-byte message has been sent to the viewer since
the last metadata content was sent, and postpone the reclamation of the
metadata stream until it is closed _and_ that 0-byte reply was sent to
the live viewer.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I0a05332284299d62b832046e4f9d22b6029c3a3e
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
---

diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
index f00f07db1..1b25671d5 100644
--- a/src/bin/lttng-relayd/live.c
+++ b/src/bin/lttng-relayd/live.c
@@ -1724,7 +1724,23 @@ int viewer_get_metadata(struct relay_connection *conn)
 
 	len = vstream->stream->metadata_received - vstream->metadata_sent;
 	if (len == 0) {
+		/*
+		 * The live viewers expect to receive a NO_NEW_METADATA
+		 * status before a stream disappears, otherwise they abort the
+		 * entire live connection when receiving an error status.
+		 */
 		reply.status = htobe32(LTTNG_VIEWER_NO_NEW_METADATA);
+		/*
+		 * The live viewer considers a closed 0 byte metadata stream as
+		 * an error.
+		 */
+		if (vstream->metadata_sent > 0) {
+			vstream->stream->no_new_metadata_notified = true;
+			if (vstream->stream->closed) {
+				/* Release ownership for the viewer metadata stream. */
+				viewer_stream_put(vstream);
+			}
+		}
 		goto send_reply;
 	}
 
@@ -1772,12 +1788,6 @@ int viewer_get_metadata(struct relay_connection *conn)
 		goto error;
 	}
 	vstream->metadata_sent += read_len;
-	if (vstream->metadata_sent == vstream->stream->metadata_received
-			&& vstream->stream->closed) {
-		/* Release ownership for the viewer metadata stream. */
-		viewer_stream_put(vstream);
-	}
-
 	reply.status = htobe32(LTTNG_VIEWER_METADATA_OK);
 
 	goto send_reply;
diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
index 9581d03eb..54f31d721 100644
--- a/src/bin/lttng-relayd/main.c
+++ b/src/bin/lttng-relayd/main.c
@@ -1521,7 +1521,7 @@ static int relay_close_stream(const struct lttcomm_relayd_hdr *recv_hdr,
 
 		vstream = viewer_stream_get_by_id(stream->stream_handle);
 		if (vstream) {
-			if (vstream->metadata_sent == stream->metadata_received) {
+			if (stream->no_new_metadata_notified) {
 				/*
 				 * Since all the metadata has been sent to the
 				 * viewer and that we have a request to close
diff --git a/src/bin/lttng-relayd/stream.c b/src/bin/lttng-relayd/stream.c
index c0aeb1719..427d30a00 100644
--- a/src/bin/lttng-relayd/stream.c
+++ b/src/bin/lttng-relayd/stream.c
@@ -1086,8 +1086,14 @@ int stream_write(struct relay_stream *stream,
 	}
 
 	if (stream->is_metadata) {
-		stream->metadata_received += packet ? packet->size : 0;
-		stream->metadata_received += padding_len;
+		size_t recv_len;
+
+		recv_len = packet ? packet->size : 0;
+		recv_len += padding_len;
+		stream->metadata_received += recv_len;
+		if (recv_len) {
+			stream->no_new_metadata_notified = false;
+		}
 	}
 
 	DBG("Wrote to %sstream %" PRIu64 ": data_length = %zu, padding_length = %zu",
diff --git a/src/bin/lttng-relayd/stream.h b/src/bin/lttng-relayd/stream.h
index 02948fa8e..c88e725c0 100644
--- a/src/bin/lttng-relayd/stream.h
+++ b/src/bin/lttng-relayd/stream.h
@@ -175,6 +175,8 @@ struct relay_stream {
 	struct cds_list_head recv_node;
 	/* Protected by session lock. */
 	bool published;
+	/* Notified viewer that no new metadata is available. */
+	bool no_new_metadata_notified;
 	/*
 	 * Node of stream within global stream hash table.
 	 */