--- /dev/null
+Upstream-Status: Backport
+
+From 87250ba19aec78f36e301494a03f5678fcb6fbb4 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Galarneau?=
+ <jeremie.galarneau@efficios.com>
+Date: Mon, 1 Nov 2021 15:43:55 -0400
+Subject: [PATCH] Fix: relayd: live: mishandled initial null trace chunk
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Observed issue
+==============
+
+As reported in #1323 (https://bugs.lttng.org/issues/1323), crashes of
+the relay daemon are observed when running the user space clear tests.
+
+The crash occurs with the following stack trace:
+ #0 0x000055fbb861d6ae in urcu_ref_get_unless_zero (ref=0x28) at /usr/local/include/urcu/ref.h:85
+ #1 lttng_trace_chunk_get (chunk=0x0) at trace-chunk.c:1836
+ #2 0x000055fbb86051e2 in make_viewer_streams (relay_session=relay_session@entry=0x7f6ea002d540, viewer_session=<optimized out>, seek_t=seek_t@entry=LTTNG_VIEWER_SEEK_BEGINNING, nb_total=nb_total@entry=0x7f6ea9607b00, nb_unsent=nb_unsent@entry=0x7f6ea9607aec, nb_created=nb_created@entry=0x7f6ea9607ae8, closed=<optimized out>) at live.c:405
+ #3 0x000055fbb86061d9 in viewer_get_new_streams (conn=0x7f6e94000fc0) at live.c:1155
+ #4 process_control (conn=0x7f6e94000fc0, recv_hdr=0x7f6ea9607af0) at live.c:2353
+ #5 thread_worker (data=<optimized out>) at live.c:2515
+ #6 0x00007f6eae86a609 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
+ #7 0x00007f6eae78f293 in clone () from /lib/x86_64-linux-gnu/libc.so.6
+
+The race window during which this occurs seems very small as it can take
+hours to reproduce this crash. However, a minimal reproducer could be
+identified, as stated in the bug report.
+
+Essentially, the same crash can be reproduced by attaching a live viewer
+to a session that has seen events being produced, been stopped and been
+cleared.
+
+Cause
+=====
+
+The crash occurs as an attempt is made to take a reference to a viewer
+session’s trace chunk as viewer streams are created. The crux of the
+problem is that the code doesn’t expect a viewer session’s trace chunk
+to be NULL.
+
+The viewer session’s current trace chunk is initially set, when a viewer
+attaches to the viewer session, to a copy the corresponding
+relay_session’s current trace chunk.
+
+A live session always attempts to "catch-up" to the newest available
+trace chunk. This means that when a viewer reaches the end of a trace
+chunk, the viewer session may not transition to the "next" one: it jumps
+to the most recent trace chunk available (the one being produced by the
+relay_session). Hence, if the producer performs multiple rotations
+before a viewer completes the consumption of a trace chunk, it will skip
+over those "intermediary" trace chunks.
+
+A viewer session updates its current trace chunk when:
+ 1) new viewer streams are created,
+ 2) a new index is requested,
+ 3) metadata is requested.
+
+Hence, as a general principle, the viewer session will reference the
+most recent trace chunk available _even if its streams do not point to
+it_. It indicates which trace chunk viewer streams should transition to
+when the end of their current trace chunk is reached.
+
+The live code properly handles transitions to a null chunk. This can be
+verified by attaching a viewer to a live session, stopping the session,
+clearing it (thus entering a null trace chunk), and resuming tracing.
+
+The only issue is that the case where the first trace chunk of a viewer
+session is "null" (no active trace chunk) is mishandled in two places:
+ 1) in make_viewer_streams(), where the crash is observed,
+ 2) in viewer_get_metadata().
+
+Solution
+========
+
+In make_viewer_streams(), it is assumed that a viewer session will have
+a non-null trace chunk whenever a rotation is not ongoing. This is
+reflected by the fact that a reference is always acquired on the viewer
+session’s trace chunk.
+
+That code is one of the three places that can cause a viewer session’s
+trace chunk to be updated. We still want to update the viewer session to
+the most recently seen trace chunk (null, in this case). However, there
+is no reference to acquire and the trace chunk to use for the creation
+of the viewer stream is NULL. This is properly handled by
+viewer_stream_create().
+
+The second site to change is viewer_get_metadata() which doesn’t handle
+a viewer metadata stream not having an active trace chunk at all.
+Thankfully, the protocol allows us to express this condition by
+returning the LTTNG_VIEWER_NO_NEW_METADATA status code when a viewer
+metadata stream doesn’t have an open file and doesn’t have a current
+trace chunk.
+
+Surprisingly, this bug didn’t trigger in the case where a transition to
+a null chunk occurred _after_ attaching to a viewer session.
+
+This is because viewers will typically ask for metadata as a result of an
+LTTNG_VIEWER_FLAG_NEW_METADATA reply to the GET_NEXT_INDEX command. When
+a session is stopped and all data was consumed, this command returns
+that no new data is available, causing the viewers to wait and ask again
+later.
+
+However, when attaching, babeltrace2 (at least, and probably babeltrace 1.x)
+always asks for an initial segment of metadata before asking for an
+index.
+
+Known drawbacks
+===============
+
+None.
+
+Fixes: #1323
+
+Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
+Change-Id: I516fca60755e6897f6b7170c12d706ef57ad61a5
+---
+ src/bin/lttng-relayd/live.c | 47 ++++++++++++++++++++++++-----------
+ src/bin/lttng-relayd/stream.h | 5 ++++
+ 2 files changed, 38 insertions(+), 14 deletions(-)
+
+Index: lttng-tools-2.13.1/src/bin/lttng-relayd/live.c
+===================================================================
+--- lttng-tools-2.13.1.orig/src/bin/lttng-relayd/live.c
++++ lttng-tools-2.13.1/src/bin/lttng-relayd/live.c
+@@ -384,8 +384,6 @@ static int make_viewer_streams(struct re
+ goto error_unlock;
+ }
+ } else {
+- bool reference_acquired;
+-
+ /*
+ * Transition the viewer session into the newest trace chunk available.
+ */
+@@ -402,11 +400,26 @@ static int make_viewer_streams(struct re
+ }
+ }
+
+- reference_acquired = lttng_trace_chunk_get(
+- viewer_session->current_trace_chunk);
+- assert(reference_acquired);
+- viewer_stream_trace_chunk =
+- viewer_session->current_trace_chunk;
++ if (relay_stream->trace_chunk) {
++ /*
++ * If the corresponding relay
++ * stream's trace chunk is set,
++ * the viewer stream will be
++ * created under it.
++ *
++ * Note that a relay stream can
++ * have a NULL output trace
++ * chunk (for instance, after a
++ * clear against a stopped
++ * session).
++ */
++ const bool reference_acquired = lttng_trace_chunk_get(
++ viewer_session->current_trace_chunk);
++
++ assert(reference_acquired);
++ viewer_stream_trace_chunk =
++ viewer_session->current_trace_chunk;
++ }
+ }
+
+ viewer_stream = viewer_stream_create(
+@@ -2016,8 +2029,9 @@ int viewer_get_metadata(struct relay_con
+ }
+ }
+
+- if (conn->viewer_session->current_trace_chunk !=
+- vstream->stream_file.trace_chunk) {
++ if (conn->viewer_session->current_trace_chunk &&
++ conn->viewer_session->current_trace_chunk !=
++ vstream->stream_file.trace_chunk) {
+ bool acquired_reference;
+
+ DBG("Viewer session and viewer stream chunk differ: "
+@@ -2034,11 +2048,16 @@ int viewer_get_metadata(struct relay_con
+
+ len = vstream->stream->metadata_received - vstream->metadata_sent;
+
+- /*
+- * Either this is the first time the metadata file is read, or a
+- * rotation of the corresponding relay stream has occurred.
+- */
+- if (!vstream->stream_file.handle && len > 0) {
++ if (!vstream->stream_file.trace_chunk) {
++ reply.status = htobe32(LTTNG_VIEWER_NO_NEW_METADATA);
++ len = 0;
++ goto send_reply;
++ } else if (vstream->stream_file.trace_chunk &&
++ !vstream->stream_file.handle && len > 0) {
++ /*
++ * Either this is the first time the metadata file is read, or a
++ * rotation of the corresponding relay stream has occurred.
++ */
+ struct fs_handle *fs_handle;
+ char file_path[LTTNG_PATH_MAX];
+ enum lttng_trace_chunk_status status;
+Index: lttng-tools-2.13.1/src/bin/lttng-relayd/stream.h
+===================================================================
+--- lttng-tools-2.13.1.orig/src/bin/lttng-relayd/stream.h
++++ lttng-tools-2.13.1/src/bin/lttng-relayd/stream.h
+@@ -174,6 +174,11 @@ struct relay_stream {
+ /*
+ * The trace chunk to which the file currently being produced (if any)
+ * belongs.
++ *
++ * Note that a relay stream can have no output trace chunk. For
++ * instance, after a session stop followed by a session clear,
++ * streams will not have an output trace chunk until the session
++ * is resumed.
+ */
+ struct lttng_trace_chunk *trace_chunk;
+ LTTNG_OPTIONAL(struct relay_stream_rotation) ongoing_rotation;
--- /dev/null
+Upstream-Status: Backport
+
+From 8f0646a03fbf31c19b85ec367dc2c3db56e6dbf7 Mon Sep 17 00:00:00 2001
+From: Francis Deslauriers <francis.deslauriers@efficios.com>
+Date: Mon, 25 Oct 2021 11:32:24 -0400
+Subject: [PATCH] Typo: occurences -> occurrences
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
+Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
+Change-Id: I719e26febd639f3b047b6aa6361fc6734088e871
+---
+ configure.ac | 2 +-
+ src/bin/lttng-relayd/live.c | 2 +-
+ src/bin/lttng-sessiond/event-notifier-error-accounting.c | 2 +-
+ src/bin/lttng-sessiond/ust-app.c | 2 +-
+ tests/utils/utils.sh | 8 ++++----
+ 5 files changed, 8 insertions(+), 8 deletions(-)
+
+diff --git a/configure.ac b/configure.ac
+index 12cc7a17e..27148c105 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -253,7 +253,7 @@ AS_IF([test "x$libtool_fixup" = "xyes"],
+ [
+ libtool_m4="$srcdir/m4/libtool.m4"
+ libtool_flag_pattern=".*link_all_deplibs\s*,\s*\$1\s*)"
+- AC_MSG_CHECKING([for occurence(s) of link_all_deplibs = no in $libtool_m4])
++ AC_MSG_CHECKING([for occurrence(s) of link_all_deplibs = no in $libtool_m4])
+ libtool_flag_pattern_count=$($GREP -c "$libtool_flag_pattern\s*=\s*no" $libtool_m4)
+ AS_IF([test $libtool_flag_pattern_count -ne 0],
+ [
+diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
+index 13078026b..42b0d947e 100644
+--- a/src/bin/lttng-relayd/live.c
++++ b/src/bin/lttng-relayd/live.c
+@@ -2036,7 +2036,7 @@ int viewer_get_metadata(struct relay_connection *conn)
+
+ /*
+ * Either this is the first time the metadata file is read, or a
+- * rotation of the corresponding relay stream has occured.
++ * rotation of the corresponding relay stream has occurred.
+ */
+ if (!vstream->stream_file.handle && len > 0) {
+ struct fs_handle *fs_handle;
+diff --git a/src/bin/lttng-sessiond/event-notifier-error-accounting.c b/src/bin/lttng-sessiond/event-notifier-error-accounting.c
+index d3e3692f5..1488d801c 100644
+--- a/src/bin/lttng-sessiond/event-notifier-error-accounting.c
++++ b/src/bin/lttng-sessiond/event-notifier-error-accounting.c
+@@ -488,7 +488,7 @@ struct ust_error_accounting_entry *ust_error_accounting_entry_create(
+ lttng_ust_ctl_destroy_counter(daemon_counter);
+ error_create_daemon_counter:
+ error_shm_alloc:
+- /* Error occured before per-cpu SHMs were handed-off to ustctl. */
++ /* Error occurred before per-cpu SHMs were handed-off to ustctl. */
+ if (cpu_counter_fds) {
+ for (i = 0; i < entry->nr_counter_cpu_fds; i++) {
+ if (cpu_counter_fds[i] < 0) {
+diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
+index b18988560..28c63e70c 100644
+--- a/src/bin/lttng-sessiond/ust-app.c
++++ b/src/bin/lttng-sessiond/ust-app.c
+@@ -1342,7 +1342,7 @@ static struct ust_app_event_notifier_rule *alloc_ust_app_event_notifier_rule(
+ case LTTNG_EVENT_RULE_GENERATE_EXCLUSIONS_STATUS_NONE:
+ break;
+ default:
+- /* Error occured. */
++ /* Error occurred. */
+ ERR("Failed to generate exclusions from trigger while allocating an event notifier rule");
+ goto error_put_trigger;
+ }
+diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh
+index e463e4fe3..42d99444f 100644
+--- a/tests/utils/utils.sh
++++ b/tests/utils/utils.sh
+@@ -1921,7 +1921,7 @@ function validate_trace
+ pass "Validate trace for event $i, $traced events"
+ else
+ fail "Validate trace for event $i"
+- diag "Found $traced occurences of $i"
++ diag "Found $traced occurrences of $i"
+ fi
+ done
+ ret=$?
+@@ -1949,7 +1949,7 @@ function validate_trace_count
+ pass "Validate trace for event $i, $traced events"
+ else
+ fail "Validate trace for event $i"
+- diag "Found $traced occurences of $i"
++ diag "Found $traced occurrences of $i"
+ fi
+ cnt=$(($cnt + $traced))
+ done
+@@ -1979,7 +1979,7 @@ function validate_trace_count_range_incl_min_excl_max
+ pass "Validate trace for event $i, $traced events"
+ else
+ fail "Validate trace for event $i"
+- diag "Found $traced occurences of $i"
++ diag "Found $traced occurrences of $i"
+ fi
+ cnt=$(($cnt + $traced))
+ done
+@@ -2013,7 +2013,7 @@ function validate_trace_exp()
+ pass "Validate trace for expression '${event_exp}', $traced events"
+ else
+ fail "Validate trace for expression '${event_exp}'"
+- diag "Found $traced occurences of '${event_exp}'"
++ diag "Found $traced occurrences of '${event_exp}'"
+ fi
+ ret=$?
+ return $ret
projects / thirdparty / openembedded / openembedded-core-contrib.git / commitdiff
