git.lttng.org Git - lttng-tools.git/log

configure: add '-Wnull-dereference' to warning flags

Change-Id: Ife5ad6963262c5c2715954fcd34c94015fb30aa6
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

fix: warning '-Wstringop-truncation' with GCC 11.2

Building with GCC 11.2 results in the following warning :

  In file included from ../../src/common/tracker.h:18,
                 from ../../src/bin/lttng-sessiond/trace-ust.h:17,
                 from test_ust_data.cpp:19:
../../src/common/sessiond-comm/sessiond-comm.h:569:14: note: while referencing ‘lttng_event_exclusion::names’
  569 |         char names[0][LTTNG_SYMBOL_NAME_LEN];
      |              ^~~~~
test_ust_data.cpp:209:16: warning: ‘char* strncpy(char*, const char*, size_t)’ specified bound 256 equals destination size [-Wstringop-truncation]
  209 |         strncpy(LTTNG_EVENT_EXCLUSION_NAME_AT(exclusion, 0),
      |         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  210 |                 get_random_string(), LTTNG_SYMBOL_NAME_LEN);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_ust_data.cpp:211:16: warning: ‘char* strncpy(char*, const char*, size_t)’ specified bound 256 equals destination size [-Wstringop-truncation]
  211 |         strncpy(LTTNG_EVENT_EXCLUSION_NAME_AT(exclusion, 1),
      |         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  212 |                 get_random_string(), LTTNG_SYMBOL_NAME_LEN);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Change-Id: I78eea760b4684227ee457c3368c6397d0a767af5
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

fix: warning '-Wformat-overflow' with GCC 11.2

Building with GCC 11.2 results in the following warning :

  In file included from rotation-thread.cpp:11:
  In function 'int handle_job_queue(rotation_thread_handle*, rotation_thread*, rotation_thread_timer_queue*)',
      inlined from 'void* thread_rotation(void*)' at rotation-thread.cpp:844:27:
  ../../../src/common/error.h:139:32: warning: '%s' directive argument is null [-Wformat-overflow=]
    139 |                         fprintf((type) == PRINT_MSG ? stdout : stderr, fmt, ## args);   \
        |                         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ../../../src/common/error.h:155:25: note: in expansion of macro '__lttng_print'
    155 |                         __lttng_print(type,                             \
        |                         ^~~~~~~~~~~~~
  ../../../src/common/error.h:195:27: note: in expansion of macro '_ERRMSG'
    195 | #define DBG(fmt, args...) _ERRMSG("DBG1", PRINT_DBG, fmt, ## args)
        |                           ^~~~~~~
  rotation-thread.cpp:587:25: note: in expansion of macro 'DBG'
    587 |                         DBG("Session \"%s\" not found",
        |                         ^~~

Use an empty string for the format string if 'session->name' is NULL.

Change-Id: Ibe29b43c0e8afd13b1c28770e8f7451340cc1e81
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Cleanup: DIST_SUBDIRS is redundant when using AM conditionals

From automake's documentation[1] :

  If SUBDIRS is defined conditionally using Automake conditionals,
  Automake will define DIST_SUBDIRS automatically from the possible
  values of SUBDIRS in all conditions.

[1] https://www.gnu.org/software/automake/manual/html_node/SUBDIRS-vs-DIST_005fSUBDIRS.html

Change-Id: I8495f1f4452ccde4920ecd63bfd37de4eb10c281
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Fix: relayd: session id is ignored by 2.11+ create session command

The id of the session used by the sessiond is not returned by
cmd_create_session_2_11 and its caller sets the value in the
relay_session to an uninitialized value.

Up until recently this didn't have much effect as this uninitialized
value was stored and used to perform look-ups in the trace chunk
registry, which would work.

However, the recent multi-consumer rotation fixes make this problem more
significant as this 'id' is used as a key to join relay sessions
originating from the same session daemon.

This was discovered by enabling the '-Wunused-parameter' warning.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7e33f5f93dc46bb630c431408c9472be3a75c030

Fix: consumerd: use-after-free of metadata bucket

Observed issue
==============

When consumer_stream_destroy() is called from, for example, the error
path in setup_metadata(), consumer_stream_free() can end up being called
twice on the same stream.  Since the stream->metadata_bucket is not set
to NULL after being destroyed, it leads to a use-after-free:

ERROR: AddressSanitizer: heap-use-after-free on address 0x604000000318
READ of size 8 at 0x604000000318 thread T7
     #0 in metadata_bucket_destroy
     #1 in consumer_stream_free
     #2 in consumer_stream_destroy
     #3 in setup_metadata
     #4 in lttng_ustconsumer_recv_cmd
     #5 in lttng_consumer_recv_cmd
     #6 in consumer_thread_sessiond_poll
     #7 in start_thread nptl/pthread_create.c:481
     #8 in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfcbde)

0x604000000318 is located 8 bytes inside of 48-byte region [0x604000000310,0x604000000340)
freed by thread T7 here:
     #0 in __interceptor_free
     #1 in metadata_bucket_destroy
     #2 in consumer_stream_free
     #3 in consumer_stream_destroy
     #4 in clean_channel_stream_list
     #5 in consumer_del_channel
     #6 in consumer_stream_destroy
     #7 in setup_metadata
     #8 in lttng_ustconsumer_recv_cmd
     #9 in lttng_consumer_recv_cmd
     #10 in consumer_thread_sessiond_poll
     #11 in start_thread nptl/pthread_create.c:481

previously allocated by thread T7 here:
     #0 in __interceptor_calloc
     #1 in zmalloc
     #2 in metadata_bucket_create
     #3 in consumer_stream_enable_metadata_bucketization
     #4 in lttng_ustconsumer_set_stream_ops
     #5 in lttng_ustconsumer_on_recv_stream
     #6 in lttng_consumer_on_recv_stream
     #7 in create_ust_streams
     #8 in ask_channel
     #9 in lttng_ustconsumer_recv_cmd
     #10 in lttng_consumer_recv_cmd
     #11 in consumer_thread_sessiond_poll
     #12 in start_thread nptl/pthread_create.c:481

Thread T7 created by T0 here:
     #0 in __interceptor_pthread_create
     #1 in main
     #2 in __libc_start_main ../csu/libc-start.c:332

SUMMARY: AddressSanitizer: heap-use-after-free in metadata_bucket_destroy

This can be easily reproduced by forcing a failure during the setup
of the metadata reproducible using the following change:

  diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
  index fa1c71299..97ed59632 100644

  --- a/src/common/ust-consumer/ust-consumer.c
  +++ b/src/common/ust-consumer/ust-consumer.c
  @@ -908,8 +908,7 @@ static int setup_metadata(struct lttng_consumer_local_data *ctx, uint64_t key)

           /* Send metadata stream to relayd if needed. */
           if (metadata->metadata_stream->net_seq_idx != (uint64_t) -1ULL) {
  -                ret = consumer_send_relayd_stream(metadata->metadata_stream,
  -                                metadata->pathname);
  +                ret = -1;
                   if (ret < 0) {
                           ret = LTTCOMM_CONSUMERD_ERROR_METADATA;
                           goto error;

Cause
=====

Channels have a list of streams that are being "setup" and are not
yet monitored for consumption. During this setup phase, the streams are
owned by the channel. On destruction of the channel, any stream in that
list will thus be cleaned-up.

When destroying a consumer stream, a reference to its channel is 'put'.
This can result in the destruction of the channel.

In the situation described above, the release of the channel's reference
is done before the stream is removed from the channel's stream list.
This causes the channel's clean-up to invoke (again) the current
stream's clean-up, resulting in the double-free of the metadata bucket.

This problem is present in a number of error paths.

Solution
========

Some error paths already manually removed the consumer stream from it's
channel's stream list before invoking consumer_stream_destroy(). The
various error paths that have to deal with this possible situation are
changed to simply invoke consumer_stream_destroy().

consumer_stream_destroy() is modified to always remove the stream from
its channel's list before performing the rest of the clean-up. This
ensures that those double clean-ups can't occur.

Drawbacks
=========

None.

Reported-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Tested-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ibeca9b675b86fc46be3f57826f7158de4da43df8

Fix: ust-consumerd: leak of stream control structure

The following leak is reported by LeakSanitizer when
setup_metadata() fails to send the metadata stream to the relay
daemon:

  ==3050181==ERROR: LeakSanitizer: detected memory leaks

  Direct leak of 240 byte(s) in 5 object(s) allocated from:
      #0 0x7f5fce02cfb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
      #1 0x7f5fcdd95a7a in zmalloc ../../../src/common/macros.h:23
      #2 0x7f5fcdd95a7a in lttng_ust_ctl_create_stream /home/jgalar/EfficiOS/src/lttng-ust/src/lib/lttng-ust-ctl/ustctl.c:1649

A consumer stream can have an allocated
`struct lttng_ust_ctl_consumer_stream *` (ustream) even if it is
not globally visible at the time of its teardown.

In the case of the user space consumer, the only site that creates
consumer stream instances ensures that the allocation of the
lttng_ust_ctl_consumer_stream succeeded, ensuring that the
consumer stream's 'ustream' is always set.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia4be7969e85bd8812ae13b042e1e100812a63c1d

Fix: liblttng-ctl: erroneous flat size computation

compute_flattened_size() erroneously computes (over-estimates) the size
of the allocation required to hold the flat array of struct lttng_event
returned to the user by lttng_list_{events, syscalls, tracepoints}.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I0a80ef0fa66428b7df34303804a024e80b635c69

fix: msgpack requires limits.h for UINT_MAX

Building with '-Wundef' reveals this issue :

  unpack.c: In function ‘template_callback_array’:
  unpack.c:197:17: warning: "UINT_MAX" is not defined, evaluates to 0 [-Wundef]
    197 | #if SIZE_MAX == UINT_MAX
        |                 ^~~~~~~~
  unpack.c: In function ‘template_callback_map’:
  unpack.c:241:17: warning: "UINT_MAX" is not defined, evaluates to 0 [-Wundef]
    241 | #if SIZE_MAX == UINT_MAX
        |                 ^~~~~~~~

Change-Id: I7dadd9f7013d613509f66e67ff1beb8ae593d2bf
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Split warning flags list for C and C++

When using Ccache [1], some flags specific to C are accepted by the C++
compiler but result in warning messages on each invocation of the
compiler. To remediate this, split the warning flags detection list in
three, a common base and a specific list for C and C++.

[1] https://github.com/ccache/ccache/issues/738

Change-Id: I9ef360efbfae445845ca1016e5f5eebdd3bdb0ac
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Tests: add a multi-domain ust+kernel rotation test

Validate that multi-domain rotations work as intended for both local and
remote outputs. This validates the fix introduced by c5c793.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I91ddd313fcfbc5421db281baea34ba2d0aae2c82

Fix: relayd: connection abruptly closed on viewer stream creation failure

Commit fe88e5175 explains (and fixes) an issue that could cause the
creation of viewer streams to fail. Currently, the error path causes the
relay daemon to abruptly close the connection to its live viewer peer.
This behaviour makes it impossible for the viewer to determine if an
error occurred or if the network connection simply failed.

Returning an `LTTNG_VIEWER_NEW_STREAMS_ERR` status code allows the
viewer to report a precise error. The viewer connection is closed since
the internal error is unlikely to be recoverable.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I557a8b66c1bd1c0bf361cfbabe962d8a6808f4f4

Fix: relayd: live client fails on clear of multi-domain session

Observed issue
==============

Two test cases of the clear/test_ust test suite occasionally fail in the
integration jobs testing cross-bitness (32/64) LTTng deployments.

Babeltrace fails with the following error when a clear occurs while a
live client consumes a trace:

  02-28 16:55:03.262 32362 32362 E PLUGIN/SRC.CTF.LTTNG-LIVE/VIEWER lttng_live_recv@viewer-connection.c:198 [lttng-live] Remote side has closed connection
  02-28 16:55:03.262 32362 32362 E PLUGIN/SRC.CTF.LTTNG-LIVE/VIEWER lttng_live_session_get_new_streams@viewer-connection.c:1706 [lttng-live] Error receiving get new streams reply
  02-28 16:55:03.262 32362 32362 E PLUGIN/SRC.CTF.LTTNG-LIVE lttng_live_msg_iter_next@lttng-live.c:1665 [lttng-live] Error preparing the next batch of messages: live-iter-status=LTTNG_LIVE_ITERATOR_STATUS_ERROR
  02-28 16:55:03.262 32362 32362 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:864 Component input port message iterator's "next" method failed: iter-addr=0x55eab7eb1170, iter-upstream-comp-name="lttng-live", iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=SOURCE, iter-upstream-comp-class-name="lttng-live", iter-upstream-comp-class-partial-descr="Connect to an LTTng relay daemon", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
  02-28 16:55:03.262 32362 32362 E PLUGIN/FLT.UTILS.MUXER muxer_upstream_msg_iter_next@muxer.c:454 [muxer] Upstream iterator's next method returned an error: status=ERROR
  02-28 16:55:03.262 32362 32362 E PLUGIN/FLT.UTILS.MUXER validate_muxer_upstream_msg_iters@muxer.c:991 [muxer] Cannot validate muxer's upstream message iterator wrapper: muxer-msg-iter-addr=0x55eab7eb1120, muxer-upstream-msg-iter-wrap-addr=0x55eab7eb3a70
  02-28 16:55:03.262 32362 32362 E PLUGIN/FLT.UTILS.MUXER muxer_msg_iter_next@muxer.c:1415 [muxer] Cannot get next message: comp-addr=0x55eab7eb0470, muxer-comp-addr=0x55eab7eb0510, muxer-msg-iter-addr=0x55eab7eb1120, msg-iter-addr=0x55eab7eb0fb0, status=ERROR
  02-28 16:55:03.262 32362 32362 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:864 Component input port message iterator's "next" method failed: iter-addr=0x55eab7eb0fb0, iter-upstream-comp-name="muxer", iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=FILTER, iter-upstream-comp-class-name="muxer", iter-upstream-comp-class-partial-descr="Sort messages from multiple inpu", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
  02-28 16:55:03.262 32362 32362 W LIB/GRAPH consume_graph_sink@graph.c:473 Component's "consume" method failed: status=ERROR, comp-addr=0x55eab7eb0760, comp-name="pretty", comp-log-level=WARNING, comp-class-type=SINK, comp-class-name="pretty", comp-class-partial-descr="Pretty-print messages (`text` fo", comp-class-is-frozen=1, comp-class-so-handle-addr=0x55eab7ebd910, comp-class-so-handle-path="/root/workspace/joraj_integration_base_job/deps-64/build/lib/babeltrace2/plugins/babeltrace-plugin-text.so", comp-input-port-count=1, comp-output-port-count=0
  02-28 16:55:03.262 32362 32362 E CLI cmd_run@babeltrace2.c:2548 Graph failed to complete successfully

  ERROR:    [Babeltrace CLI] (babeltrace2.c:2548)
    Graph failed to complete successfully
  CAUSED BY [libbabeltrace2] (graph.c:473)
    Component's "consume" method failed: status=ERROR, comp-addr=0x55eab7eb0760,
    comp-name="pretty", comp-log-level=WARNING, comp-class-type=SINK,
    comp-class-name="pretty", comp-class-partial-descr="Pretty-print messages
    (`text` fo", comp-class-is-frozen=1, comp-class-so-handle-addr=0x55eab7ebd910,
    comp-class-so-handle-path="/root/workspace/joraj_integration_base_job/deps-64/build/lib/babeltrace2/plugins/babeltrace-plugin-text.so",
    comp-input-port-count=1, comp-output-port-count=0
  CAUSED BY [libbabeltrace2] (iterator.c:864)
    Component input port message iterator's "next" method failed:
    iter-addr=0x55eab7eb0fb0, iter-upstream-comp-name="muxer",
    iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=FILTER,
    iter-upstream-comp-class-name="muxer",
    iter-upstream-comp-class-partial-descr="Sort messages from multiple inpu",
    iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
  CAUSED BY [muxer: 'filter.utils.muxer'] (muxer.c:991)
    Cannot validate muxer's upstream message iterator wrapper:
    muxer-msg-iter-addr=0x55eab7eb1120,
    muxer-upstream-msg-iter-wrap-addr=0x55eab7eb3a70
  CAUSED BY [muxer: 'filter.utils.muxer'] (muxer.c:454)
    Upstream iterator's next method returned an error: status=ERROR
  CAUSED BY [libbabeltrace2] (iterator.c:864)
    Component input port message iterator's "next" method failed:
    iter-addr=0x55eab7eb1170, iter-upstream-comp-name="lttng-live",
    iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=SOURCE,
    iter-upstream-comp-class-name="lttng-live",
    iter-upstream-comp-class-partial-descr="Connect to an LTTng relay daemon",
    iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
  CAUSED BY [lttng-live: 'source.ctf.lttng-live'] (lttng-live.c:1665)
    Error preparing the next batch of messages:
    live-iter-status=LTTNG_LIVE_ITERATOR_STATUS_ERROR
  CAUSED BY [lttng-live: 'source.ctf.lttng-live'] (viewer-connection.c:1706)
    Error receiving get new streams reply
  CAUSED BY [lttng-live: 'source.ctf.lttng-live'] (viewer-connection.c:198)
    Remote side has closed connection

Looking at the relay daemon logs, we see the following error:
  DBG1 - 16:55:03.262106718 [32139/32146]: Adding new file "ust/pid/gen-ust-events-32373-20220228-165503/chan_0" to trace chunk "(unnamed)" (in lttng_trace_chunk_add_file() at trace-chunk.cpp:1310)
  PERROR - 16:55:03.262133333 [32139/32146]: Failed to open fs handle to ust/pid/gen-ust-events-32373-20220228-165503/chan_0, open() returned: No such file or directory (in fd_tracker_open_fs_handle() at fd-tracker/fd-tracker.cpp:548)

Cause
=====

Adding more debugging logging allows us to see the following situation
takes place:

- relay thread: Create trace chunk on session 1.
- live thread: get new streams against session 1, returns NO_NEW_STREAMS
  since the session has an 'ongoing_rotation'.
- live thread: get new streams against session 2, sees no rotation
  ongoing and attempts to open `chan_0` when creating a viewer stream

The "ongoing rotation" check was introduced in a7ceb342d and, in a
nutshell, prevents live viewers from creating new viewer streams during
a rotation.

The "ongoing rotation" state is entered when a CREATE_NEW_TRACE_CHUNK
command is issued against a session.

However, this presumes that a relay_session maps 1:1 to a session on the
session daemon's end. This isn't the case as, in multi-domain
scenarios (tracing 32-bit, 64-bit, and kernel events), a single session
daemon session can map to multiple relay_session objects. This is
because the consumer daemons maintain independant connections to the
relay daemon.

To synchronize rotations accross related relay_session instances, the
relay daemon uses the same trace chunk instances accross relay_session
instances. This means that while a trace chunk is created against a
specific relay session, it can be used by other relay_session instances.

To manage shared trace chunks between relay_sessions, the relay daemon
makes use of the trace_chunk_registry. This registry allows
relay_sessions to share trace chunk instances using a unique key tuple:
  - session daemon instance uuid,
  - session daemon session id,
  - trace chunk id.

There is no equivalent mechanism to track the "ongoing_rotation" state
accross relay_sessions originating from the same sessiond session.

In the current scenario, this causes the live client to correctly see
that no new streams are available for session 1 (say, the 32-bit user
space session). Unfortunately, this state is not entered for other
sessions (64-bit and kernel relay sessions). Hence, the viewer succeds
in acquiring new streams from session 2, exposing the race the 'ongoing
rotation' state aims to protect against.

Solution
========

Like the trace chunk instances, the "ongoing rotation" state must be
shared accross relay sessions that originate from the same session
daemon session.

To "emulate" this shared state, session_has_ongoing_rotation() checks
if any relay session originating from the same sessiond session
have an ongoing rotation. If it is the case, we temporarily prevent
live viewers from acquiring new streams.

Known drawbacks
===============

session_has_ongoing_rotation() iterates over all sessions, acquiring
their lock in the process, which is certainly undesirable from a
performance standpoint.

Optimizing this is not a great challenge, but is beyond the scope
of this immediate fix.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I457a32fa497d151ca933c25433c80665268a7c1c

Fix: rotation: hang on destroy when using scheduled rotation based on timer

Observed issue
==============

The following scenario results in a hang for `lttng destroy`:

lttng create test
lttng enable-event -u -a
lttng enable-rotation --timer 100000
lttng start
lttng stop
lttng start
lttng destroy

Cause
=====

There is an imbalance in how many times we start the rotation timer.

The rotation timer is only removed on `lttng destroy` or when disabling
a time-based-rotation. On the other hand, the timer is "started"
on `lttng start` and when enabling a time based rotation.

The imbalance emerging from a start/stop/start sequence would prevent the
teardown of the session object since each time the timer is started a
reference to the session is held.

Solution
========

Do not start the rotation schedule timer if it was already launched.

Known drawbacks
=========

None.

Change-Id: Ic5b8938166358fe7629187bebdf02a09e90846c0
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Fix: consumerd: fd still open after `lttng snapshot record` returns

Observed issue
=====

Using a snapshot output located on a pramfs mount:

  lttng snapshot record
  rm -rf /my_mount/my_trace_output

`rm` fails with ENOTEMPTY on rmdir for /my_mount/my_trace_output.

At that point, the lttng-consumerd daemon have an open fd on:
  /my_mount/my_trace_output/ust

Note that a sleep between both command "fixes" the issue.

Cause
=====

The reclaim for the in-registry trace chunks can happen after the LTTng
CLI returns since we use `call_rcu`.

```
static
void lttng_trace_chunk_release(struct urcu_ref *ref)

....

  if (chunk->in_registry_element) {
    struct lttng_trace_chunk_registry_element *element;

    element = container_of(chunk, typeof(*element), chunk);
    if (element->registry) {
      rcu_read_lock();
      cds_lfht_del(element->registry->ht, &element->trace_chunk_registry_ht_node);
      rcu_read_unlock();
->    call_rcu(&element->rcu_node, free_lttng_trace_chunk_registry_element);
   } else {

```

The delayed reclaim of the `lttng_trace_chunk_registry_element` can
result in lttng-consumerd holding an open fd for the "chunk directory"
of the chunk since the close() is only done during the "*fini" phase of
the chunk (`lttng_trace_chunk_fini`).

Solution
========

Considering that the rcu lookup+refcount access scheme is used for the
trace chunk object and that at that point the refcount for the trace
chunk object is effectively zero, we can move the
`lttng_trace_chunk_fini` safely outside of the
`free_lttng_trace_chunk_registry_element` call_rcu call.

Known drawbacks
=========

Even if this solves the current situation, it is important to note that
the actual object holding the reference is itself refcounted and only
close the fd on release. This means that we are still exposed to this
problem if at some point the directory handle is shared and outlives the
trace chunk for some reason in the future.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6da3948824bf8b092fc8248b1bb0263fdd5887be

Fix: event: unchecked return value for lttng_strncpy

Reported by Coverity:

CID 1475806: Unchecked return value (CHECKED_RETURN)
Calling lttng_strncpy without checking return value.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib2b16bef61bcc24fea381c18fd8f914e9a84a6e7

Fix: event: format specifier for ssize_t

Observed issue
==============

  In file included from event.cpp:15:
  event.cpp: In function ‘ssize_t lttng_event_create_from_payload(lttng_payload_view*, lttng_event**, lttng_event_exclusion**, char**, lttng_bytecode**)’:
  ../../src/common/error.h:191:28: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘ssize_t’ {aka ‘int’} [-Wformat=]
    191 |  __lttng_print(PRINT_WARN, "Warning: " fmt "\n", ## args)
        |                            ^~~~~~~~~~~~~~~~~~~~
  ../../src/common/error.h:139:51: note: in definition of macro ‘__lttng_print’
    139 |    fprintf((type) == PRINT_MSG ? stdout : stderr, fmt, ## args); \
        |                                                   ^~~
  event.cpp:624:4: note: in expansion of macro ‘WARN’
    624 |    WARN("Userspace probe location from the received buffer is not the advertised length: header length = %" PRIu32 ", payload length = %lu", event_comm->userspace_probe_location_len, ret);
        |    ^~~~

Solution
========

Albeit there is no "canonical" way of printing ssize_t, use '%zd' since
we already make use of it elsewhere.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Id41e6ccf07bd580813f169b65d281a4fa305fb48

Fix: sessiond: use after free

Introduced by 88b02a8e7cbdbc728cc681cd07bbe0c738964717

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9b1c3eeed7e51976af1a4982fc522066b3b29461

Fix: sessiond: `break` used instead of `goto`

On error path, use `goto`.

Reported by Coverity:

CID 1475805: Unused value (UNUSED_VALUE)
Assigning value LTTNG_ERR_UNK to ret_code here, but that stored value is overwritten before it can be used.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ife3b5486b8463242dfecb61a3a9718e8eb7149b4

Fix: sessiond: lttng_channel object is not reclaimed

The reclaim is also missing on the error path.

Reported by Coverity:

CID 1475807: Resource leak (RESOURCE_LEAK)
Variable channel going out of scope leaks the storage it points to.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9ae59900f6bc33282b8b8a65b834a718428a7f50

Fix: common: local_attr might leak

Reported by Coverity:

    CID 1475808: Resource leak (RESOURCE_LEAK)
    Variable local_attr going out of scope leaks the storage it points to.

    CID 1475810: Resource leak (RESOURCE_LEAK)
    Variable local_attr going out of scope leaks the storage it points to.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ibe2ee6516618827b6f52676d9a9f7d510cadf1bc

Fix: session: end goto label used for error path

Reported by Coverity:

    CID 1475801: Unused value (UNUSED_VALUE)
    Assigning value LTTNG_ERR_FATAL to ret_code here, but that stored value is overwritten before it can be used

    CID 1475802: Unused value (UNUSED_VALUE)
    Assigning value LTTNG_ERR_NOMEM to ret_code here, but that stored value is overwritten before it can be used.

    CID 1475814: Unused value (UNUSED_VALUE)
    Assigning value LTTNG_ERR_UST_CHAN_NOT_FOUND to ret_code here, but that stored value is overwritten before it can be used.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I79fb5d65ea0f4f3dfbbef2a42ec3dcf0542043c5

Fix: sessiond: event, filter expression, bytecode, exclusions might leak

Reported by Coverity:

    CID 1475803: Resource leak (RESOURCE_LEAK)
    Variable filter_expression going out of scope leaks the storage it points to.

    CID 1475816: Resource leak (RESOURCE_LEAK)
    Variable exclusions going out of scope leaks the storage it points to.

    CID 1475817: Resource leak (RESOURCE_LEAK)
    Variable bytecode going out of scope leaks the storage it points to.

    CID 1475820: Resource leak (RESOURCE_LEAK)
    Variable event going out of scope leaks the storage it points to.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I705e3cc1a1894af52d6ddeee94798a887dc82bba

Fix: lttng-sessiond: event_context might leak

If the parsing completed but is not the expected size, the
*out_event_context object leaks.

Reported by Coverity:

CID 1475821 (#1 of 1): Resource leak (RESOURCE_LEAK)
Variable event_context going out of scope leaks the storage it points to.

Change-Id: I8198f7f3f1e1af3dcb377d89674402ec49cb788b
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Fix: lttng-ctl: uninitialized pointer read

Reported by Coverity:

CID 1475822: Uninitialized pointer read (UNINIT)
Using uninitialized value reply._fd_handles.array.size when calling lttng_payload_reset.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ied7f5de61d736e8d8f9c869a36322262a4d79cd5

Fix: lttng-sessiond: nb_events is uninitialized

Reported by Coverity:

CID 1475823: Uninitialized scalar variable (UNINIT)
Using uninitialized value nb_events.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I623b1ed0c1ef9b4acddb1e46e7311dfcdc974629

Fix: ser/des: missing null terminator on payload append

Reported by the lttng-ust-java-tests_master_build CI job.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I47b048c65a9855347c9b02fe44d9fdbae7e06dbf

Fix: test: test plan/skip must come after TAP initialization

TAP is initialized during the sourcing of `utils.sh`. Calling it before
yield nothing.

Move the platform validation after the `PLAN_TEST` call.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib7b4434440ed7590b39b0580ea294045ace40495

Fix: test: missing test plan for non-x86/arm platforms (i.e powerpc)

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9a34a0b45fd011943436ce6e45c8f0d1666df26f

Fix: remove debugging output

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7bcdb7c47f811cf99dee1e4e483cc22847f481c5

Fix: lttng: truncated addresses and offsets on 32-bit builds

The lttng client parses hexadecimal addresses using, at some point,
strtoul(). Using this function effectively caps addresses and
offsets to MAX_UINT32 resulting in failures to enable kprobes
against a 64-bit kernel using a 32-bit client.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: If619e9e84413de5cd32d8c06f363152caaf5ac46

Fix: liblttng-ctl comm: lttng_event_field is not packed

Observed issue
==============

For MI testing where the lttng-sessiond is 64 bit and the lttng CLI is
32 bit, the tracepoint field listing fails with partial garbage output.

The size of the struct differs between bitness for x86-64 and x86
leading to serialization/deserialization problem across client
(liblttng-ctl) and lttng-sessiond.

sizeof(struct lttng_event_field):

x86: 1136
x86-64: 1144

The struct cannot be marked as LTTNG_PACKED since it is part of the API.

Solution
========

Adopt a similar pattern to the new APIs with a "serialize" &
"create_from_buffer" approach. The only particularity is that we need to
flatten the event_field on listing.

Most of the complexity is moved to `src/common/event.c`

Known drawbacks
=========

None.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I280d9809d110237574e2606ee93a7aeba41e704e

Fix: liblttng-ctl comm: lttng_event_context is not packed

Observed issue
==============

The size of the struct differs between bitness for x86-64 and x86
leading to serialization/deserialization problem across client
(liblttng-ctl) and lttng-sessiond.

sizeof(struct lttng_event_context):

x86: 308
x86-64: 312

The struct cannot be marked as LTTNG_PACKED since it is part of the API.

Solution
========

Adopt a similar pattern to the new API with a "serialize" &
"create_from_buffer" approach.

Most of the complexity is moved to `src/common/event.c`

Known drawbacks
=========

None.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ieb400eab2a2df4070ff51cb2b44929d3ea945ce4

Fix: liblttng-ctl comm: lttng_event is not packed

Observed issue
==============

In `lttcomm_session_msg` the lttng_event struct is marked as
LTTNG_PACKED. This statement have no effect as explained in commit [2].

Solution
========

Adopt a similar pattern to the new API with a "serialize" &
"create_from_buffer" approach.

Most of the complexity is moved to `src/common/event.c`

Known drawbacks
=========

None.

Note
====

Jérémie Galarneau: This patch was extensively modified from the original
patch applying against stable-2.12 to accomodate for the use of the
lttng_payload utils throughout the liblttng-ctl <-> lttng-sessiond
communication code.

Some changes were also made to build as C++.

Reference
========
[1] https://review.lttng.org/gitweb?p=lttng-tools.git;a=commit;h=7bd95aee4660c6419a4a65429fc27754481e7e90

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I35d848519dacb2b119324e88f262aa95951e4ac6

libcommon: move event.c to libcommon-lgpl

The `event.c` license is already LGPL. There is no technical reason why
it was not part of the lgpl side of libcommon, simply that nothing that
is LGPL needed it. This will change in upcoming commits with the
addition of ser/des functions of `struct lttng_event` and other structs
related to `lttng_event` for liblttng-ctl.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I1e15a547e526198f971a287a726d0e6229a733b0

Fix: liblttng-ctl comm: lttng_channel is not packed

Observed issue
==============

The size of the struct differs between bitness for x86-64 and x86
leading to serialization/deserialization problem across client
(liblttng-ctl) and lttng-sessiond.

sizeof(struct lttng_channel):

x86: 608
x86-64: 624

The struct cannot be marked as LTTNG_PACKED since it is part of the API.

Solution
========

Adopt a similar pattern to the new API with a "serialize" &
"create_from_buffer" approach. The only particularity is that we need to
flatten the channels on listing.

Most of the complexity is moved to `src/common/channel.c`

Known drawbacks
=========

None.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Id5c9aaf3cf8b3d739b71263c02cae8d4d2fedfe3

Fix: conversion from KB to bytes overflow on arm32

Observed issue
==============

On enable channel the memory available check fails on arm32 when
available memory, in bytes, is larger than 2^32.

Cause
=====

`read_proc_meminfo_field` converts the read value (in KB) to bytes and
stores it into a size_t variable.

On the system running the reproducer the value of the `value_kb` variable
is 4839692, yielding an overflow when multiplied with 1024 since
`size_t` is 32 bit long. `size_t` can be larger in certain situation
(i.e LARGEFILE) but this is irrelevant to the problem at hand.

Solution
========

Convert all the checks to use uint64_t.

Known drawbacks
=========

None.

References
==========

The multiplication overflow check scheme is borrowed from
`src/common/time.c`

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I067da25659ab4115e5494e48aab45a1c35f56652

Fix: Unexpected payload size in cmd_recv_stream_2_11

Observed issue
==============

For the following scenario:

lttng-relayd: 64 bit
lttng-sessiond: 64 bit
lttng-consumerd: 32 bit
application: 32 bit

Commands
   lttng create --set-url=net://127.0.0.1
   lttng enable-event -u -a
   lttng start
   ./application

On application start the lttng-relayd reports this error:

  DEBUG1 - 14:16:38.216442600 [2004731/2004735]: Done receiving control command payload: fd = 19, payload size = 4376 bytes (in relay_process_control_receive_payload() at main.c:3456)
  DEBUG3 - 14:16:38.216469462 [2004731/2004735]: Processing "RELAYD_ADD_STREAM" command for socket 19 (in relay_process_control_command() at main.c:3327)
  Error: Unexpected payload size in "cmd_recv_stream_2_11": expected >= 3519925694 bytes, got 4376 bytes

Cause
=====

In `relayd_add_stream`, instead of taking the > 2.11 protocol path, the
`relayd_add_stream_2_2` function is called.

The value of the rsock version number are:

  major: 21845
  minor: 2

Which is simply invalid since we know that the version should be 2.12.

The relayd sock version numbers are set during the
LTTNG_CONSUMER_ADD_RELAYD_SOCKET command between the lttng-sessiond and
the lttng-consumerd process. It is important to note here that both
processes do NOT have the same bitness.

The serialization and deserialization of `struct lttcomm_relayd_sock` is
the culprit.

`struct lttcomm_relayd_sock` contains a `struct lttcomm_sock`:

struct lttcomm_sock {
    int32_t fd;
    enum lttcomm_sock_proto proto;
    struct lttcomm_sockaddr sockaddr;
    const struct lttcomm_proto_ops *ops;
} LTTNG_PACKED;

Note that `ops` is a pointer and its size varies based on the bitness of
the application. Hence the size of the `struct lttcomm_sock` differs
across bitness. Since it is the first member of `struct
lttcomm_relayd_sock`, the memory layout is simply invalid across
bitness (amd64/x86).

This results in invalid parsing for the overall "struct
lttcomm_relayd_sock" when dealing with a lttng-consumerd with a
different bitness than the lttng-sessiond. As far as I know local
tracing scenarios are not affected since this is only relevant when
dealing with a lttng-relayd.

Solution
========

Pass the socket protocol type, relayd major, relayd minor in
`lttcomm_consumer_msg`. On the receiver side, query the network stack to
get the peer information to populate a basic `lttcomm_sock`. Leaving
this work to the OS saves us from having to serialize the `sockaddr_in*`
structs.

Known drawbacks
=========

We rely on `getpeername` for the first time. Compatibility might be a
problem.

This code path assumes a lot of thing that cannot be asserted against
such as the fact that the socket from which we fetch the info must be
`connected`. Still at this point, the socket is completely setup and the
rest of the code depends on it already.

From GETPEERNAME(2):

```
       For stream sockets, once a connect(2) has been performed, either
       socket can call getpeername() to obtain the address of the peer
       socket.  On the other hand, datagram sockets  are connectionless.
       Calling connect(2) on a datagram socket merely sets the peer
       address for outgoing datagrams sent with write(2) or recv(2).
       The caller of connect(2) can use getpeername() to obtain the
       peer address that it earlier set for the socket.  However, the
       peer socket is unaware of this information, and calling
       getpeername() on the  peer  socket will  return  no useful
       information (unless a connect(2) call was also executed on the
       peer).  Note also that the receiver of a datagram can obtain the
       address of the sender when using recvfrom(2).
```

But here we are always "the caller of connect".

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic157c4137b2f20e394c907136687fcbd126f90a0

Tests: add kernel test into the `make check` test suite.

The Jenkins CI mostly run the `make check` suite. Only the Lava base CI
run the root_regression test suite. Most of those test can be run on
`make check` without incurring any major extra time.

Only `regression/tools/streaming/test_high_throughput_limits` is left in
root_regression since it is currently "unreasonable" in term of the time
it takes to run. This could be tackled another time.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3d59705e84549c2a1840cf96a7723396e2e0c402

Build dist fix: some kernel tests are not distributed

A missing '\' causes some kernel test scripts to not be added
the EXTRA_DIST variable. This causes those tests to not be
shipped as part of the tarball.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3c824b4c7ea913cb4cdc7ac6f98df05267ca5764

Tests: fix: test_kernel_function: erroneous test count

The test_kernel_function test runs one extra test which causes it to be
marked as failed. Adjust the count to fit the number of tests.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia861cf7b0123fdf19405bb84271ddda76f9fd6e7

Tests: fix: test_kernel_function: event name mismatch

The test_kernel_function test expects the name of the function
exit event to be suffixed with "_return". This was changed to
"_exit" during the development of the 2.13 release.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6e7f85af33699b2bf0439f74343d521f896594e1

Tests: fix: select_poll_epoll: test assumes epoll fd value

The test currently assumes that epoll fds are always == 3, which
is not always the case depending on the execution environment.

This change causes `select_poll_epoll` to produce a JSON file
containing the application's pid and epoll fd values that is
then used by the validation script.

Note that the test is converted to C++ to allow the use of
internal utils (common/error.h/cpp) without changing their linkage.

However, the code is still regular C to ease the backport of this
fix.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie373c63f6e6b9267ae2d785c9f0a532a5de37905

Fix: tests: test_kernel: break should only be used in loops

Using `break` in a function's scope makes no sense in bash. I am
guessing the original author meant to exit early from the various tests.

Regardless, the rest of the test can be ran without issues. I am not
sure traces of failed tests should be kept, but that's a separate issue.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I286ccb796afbbca4e3866e6fd0b35a3746045346

Tests: test_kernel: add comments regarding the filling of buffers

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3a8652d7dd95c015d6978349d166e8f291c9a5f3

Fix: tools/snapshots/test_kernel flaky test

When tracing all system calls, nothing guarantees that the first system
call won't come from some _other_ program on the system, on a CPU != 0,
and stay invariant between the two snapshots (when it should not be).

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

Tests: Fix: test_list_triggers_cli: support in-kernel builtin lttng-modules

This commit changes the grep call to remove the [lttng_tracer] string
from the pattern. When building the lttng modules directly in the kernel
there is not mention of [lttng_tracer].

Furthermore, the symbol type "t" is "T" in my test VM. The difference
may be due to the builtin nature of lttng on this VM.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I046905dc51524c23ad3671b66614b86a084ef8e2
Depends-on: lttng-ust: If1f29dd64538bc5979d3e89aa5cca3be4e41046f

Add Log4j 2.x agent tests for the 'log4j' domain

Add integration tests for the new Log4j 2.x agent in Log4j 1.x compat
mode using the current 'log4j' domain, use the new configure switch
'--enable-test-java-agent-log4j2' to enable it or
'--enable-test-java-agent-all' to enable all Java agents tests.

To run only this new test, use this command :

cd tests/regression && make check TESTS="ust/java-log4j2/test_agent_log4j2_domain_log4j"

Change-Id: Id780c9ee13913c91c18548f58b14cc600e77e9fa
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

src/common: use single Makefile for parallel builds

Use a single Makefile in 'src/common' as it contains multiple
subdirectories with a small number of objects to compile. This allows
faster parallel builds since parallelism in automake is applied per
Makefile.

There is anectodal evidence of a 25 seconds improvement to the build
process on a 36 core machine.

Change-Id: If2ce266050e345d58b00bf65b574ccf5168f28f1
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Fix: missing RCU read side critical sections

Based on the comments of the called functions.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ica81b648ce88290c4ca7507fb00a78480457cf01

Enforce documented RCU preconditions with assertions

Mindlessly add `rcu_read_ongoing()` assertions to functions that are
documented as "must be called" within a RCU critical section.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I25f9903938123394e6960ab2a338be6abaf2fe72

Remove ht-cleanup thread

The hashtable cleanup thread was introduced to prevent deadlocks
happening when the `cds_lfht_destroy()` function was called concurrently
with userspace-rcu hashtable resizes.

This was fixed in the userspace-rcu project in commit:
  commit d0ec0ed2fcb5d67a28587dcb778606e64f5b7b83
  Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
  Date:   Tue May 30 15:51:45 2017 -0400

      Use workqueue in rculfhash

That commit makes it so that the `cds_lfht_destroy()` function can
safely be called within RCU read-side critical sections.

This commit is included in the 0.10 release of urcu. The LTTng-Tools
project now has a minimum version dependency on urcu 0.11.

Because it's now safe to call `cds_lfht_destroy()` within RCU critical
sections, the need for the hash table cleanup thread disappears. This
commit replaces all uses of `ht_cleanup_push()` by `lttng_ht_destroy()`
and remove all uses and mentions of the ht_cleanup thread.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I163a281b64a6b3eed62c58515932f71f3b52fea6

Cleanup: remove outdated comments on RCU requirements

The ht-cleanup thread was introduced to work-around the limitation that
the hash table destroy could not be called from a RCU read-side critical
section because of requirements from the rculfhash API, some comments
stating that destruction code should not be called from RCU read-side
critical sections were left in place when they should in fact have been
removed.

This ht-cleanup thread work around was introduced by the following
commit:
  commit 0b2dc8df2a6d7b3341a72a04767dd6328907c97c
  Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
  Date:   Fri Jun 14 07:44:52 2013 -0400

    Fix: hash table cleanup call_rcu deadlock

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ifdaa63e6146b51fdb251e75795e382b5f390f7be

relayd: add logging statements in `viewer_get_packet()`

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Idf3c7a5e87636abe63ff59d22bb3104f1d3519e5

relayd: add logging statements in `viewer_attach_session()`

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I2cdae4bd809442d5de7dcfb838f346e1e4370628

relayd: add logging statements in `viewer_get_next_index()`

Change the byte order of the `status` field right before we send the
index to the viewer to allow for useful logging along the way.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I50bb246810486e13fde3085e21fea4bb5ef3776b

Fix: _lttng_variant_statedump should expect lttng_ust_ctl_atype_variant_nestable

The precondition check in _lttng_variant_statedump is too strict: it
should also expect lttng_ust_ctl_atype_variant_nestable. Remove this
check entirely, which is redundant with the switch/case in the only
caller sites in the same compile unit.

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

Fix context mismatch across UST version due to legacy array context field

Observed issue
==============

Tracing applications linked against both LTTng-UST 2.12.x and 2.13.1 in
the same session fails with:

"Error: Registering application channel due to context field mismatch"

for some applications with the "procname" context enabled.

Cause
=====

The procname context uses the legacy array field type prior to LTTng-UST
2.13, and uses the array_nestable field type starting from LTTng-UST
2.13. The field comparison in lttng-sessiond is unaware of the fact that
they map to the exact same binary trace layout and are therefore
compatible.

Solution
========

Introduce a new fixup function "ust_app_fixup_legacy_context_fields" in
lttng-sessiond for channel context fields, which detects the presence of
the legacy array, struct and variant types, and rewrites them as
array_nestable, struct_nestable, and variant_nestable types.

Reject the legacy sequence type in channel context fields because it is
not used by LTTng-UST 2.12 and older.

Rewriting those legacy context types as the new "nestable" types ensures
that field comparison functions will correctly handle a mix of 2.12 and
2.13 LTTng-UST tracers using a procname context to a given session's
channel.

Known Drawbacks
===============

None.

Fixes: fb2772932 ("Validate channel context mismatch across UST applications")
Change-Id: Ic51b92130ae8e1f7b510924f78ff46459dc2f578
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Relicence all source and header files included in LGPL code

All code included in libcommon-lgpl.a should be LGPL. Some were licensed
as GPLv2 by mistake. We need to relicense those.

EfficiOS owns the copyright to all of the affected source files and
agrees to this relicensing from GPLv2 to LGPLv2.1.

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

Move utils_expand_path and utils_expand_path_keep_symlink to libpath.la

Move the GPLv2 helper functions utils_expand_path and
utils_expand_path_keep_symlink to libpath.la. This will allow utils.cpp
to be relicensed to LGPLv2.1 by making sure EfficiOS owns the copyright
for the entire source file.

Statically include libpath.la into libcommon-gpl.la.

The "lttng" executable is GPLv2 and only depends on libcommon-lgpl.la,
so it needs to explicitly list libpath.la as its dependency.

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

Link lttng executable on libcommon-lgpl.a

Allows testing whether liblttng-ctl.so has any dependency on
libcommon-gpl.a. Will trigger a link-time failure if it is the case.

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

Introduce libcommon-lgpl for liblttng-ctl

liblttng-ctl is a LGPLv2.1 library should should not use GPLv2 code.
Introduce libcommon-lgpl as a static archive containing only LGPLv2.1
compatible code.

This also removes the dependency from liblttng-ctl to liburcu.

Include some source files in libcommon-lgpl.a which are indirectly needed
by source files required in libcommon-lgpl.a:

- endpoint.cpp,
- lttng-elf.cpp,
- lttng-elf.h.

Include some source files in libcommon-lgpl.a which are only needed to
link the lttng executable:

- domain.cpp,
- spawn-viewer.cpp, spawn-viewer.h.

Introduce the new source file hashtable/seed.cpp to move the
lttng_ht_seed symbol in a source file which does not require
liburcu-cds, so it can be present in libcommon-lgpl. This allows
building compile units which are needed in the lgpl common library which
also contain functions which directly refer to lttng_ht_seed.

Programs and libraries which use libhashtable.la are changed to use
libcommon-gpl.la instead. libhashtable becomes internal to libcommon.

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

Rename libcommon.so to libcommon-gpl.so

libcommon is a static library is currently used by both liblttng-ctl
(LGPLv2.1) and all lttng-tools executables (GPLv2).

Given that some code in libcommon depends on liburcu, this introduces an
indirect dependency from liblttng-ctl to liburcu, which is unwanted.

This first step renames libcommon.so to libcommon-gpl.so. Following
steps will introduce a more lightweight libcommon-lgpl.so which only
contains LGPLv2.1 code, and removes the dependency on liburcu.

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

Copyright ownership transfer

Apply copyright ownership transfer from Jonathan Rajotte-Julien and
Olivier Cotte to EfficiOS Inc.

Link: https://lists.lttng.org/pipermail/lttng-dev/2022-January/030111.html
Link: https://lists.lttng.org/pipermail/lttng-dev/2022-January/030113.html
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Olivier Moussavou Cotte <olivier.moussavoucotte@mail.mcgill.ca>
Signed-off-by: Jonathan Rajotte-Julien <jonathan.r.julien@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I0c2800ec17f5d435c8a18f2dae096809b8037b5d

Docs: relayd: live: clarify ownership of vstream after viewer release

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I2b0a838bb72bec1ec693ee0c722b748105d7280c

Copyright ownership transfer

Apply copyright ownership transfer from David Goulet, Julien Desfossez,
and Simon Marchi to EfficiOS Inc.

Link: https://lists.lttng.org/pipermail/lttng-dev/2022-January/030087.html
Link: https://lists.lttng.org/pipermail/lttng-dev/2022-January/030092.html
Link: https://lists.lttng.org/pipermail/lttng-dev/2022-January/030091.html
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Signed-off-by: Julien Desfossez <ju@klipix.org>
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Id13575afd4a2a09bb91a8d2b7a12dc3db8dc329f

Fix: relayd: erroneous rundir permission logging message

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I2eb0901896feab684d397aa43f8c9036018d9c95

Fix: sessiond: rotation thread: fatal error when not finding a session

The rotation thread implements scheduled rotations (by size) by
registering a trigger that monitors the session's consumed size and
notifies when the next rotation's size threshold is exceeded.

The notification is delivered asynchronously which doesn't prevent
the session from being destroyed before the rotation thread has
had the time to process the notification (and perform a rotation).

Since it is possible for a session to be destroyed by the time the
notification is processed, the rotation thread shouldn't handle
this eventuality as a fatal error (shutting down the thread).

Note that nobody reported this issue nor did I attempt to reproduce it.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I588054bad3542854851f28d34f2c758bdf420a34

Fix: relayd: rotation failure for multi-domain session

Observed issue
==============

Rotating a multi-domain streaming session results in the following
error:

  $ lttng rotate
  Waiting for rotation to complete...
  Error: Failed to retrieve rotation state.

Meanwhile, the relay daemon logs indicate the following:

  DBG1 - 14:56:04.213163667 [265774/265778]: lttng_trace_chunk_rename_path from .tmp_new_chunk to (null) (in lttng_trace_chunk_rename_path_no_lock() at trace-chunk.cpp:759)
  PERROR - 14:56:04.213242941 [265774/265778]: Failed to move trace chunk directory ".tmp_new_chunk" to "20220112T145604-0500-1": No such file or directory (in lttng_trace_chunk_rename_path_no_lock() at trace-chunk.cpp:799)
  DBG1 - 14:56:04.213396931 [265774/265778]: aborting session 2 (in session_abort() at session.cpp:588)
  DBG1 - 14:56:04.213512198 [265774/265778]: Control connection closed with 22 (in relay_thread_close_connection() at main.cpp:3874)

The 'abort' of session 2 here causes the kernel consumer to fail to
consume subbuffers:

  Error: Relayd send index failed. Cleaning up relayd 3.
  Error: Error consuming subbuffer: (0)
  [...]

Cause
=====

Following the flow of execution in the relay daemon shows that different
trace chunks are used by the two relay sessions that result from the
streaming of a single multi-domain session. Both trace chunks "own" the
same output directory.

When a rotation is performed, the first trace chunk to be closed will
move the directory. Then, the second trace chunk to be closed will
attempt to do the same, failing to do so as seen in the relay daemon
log.

Solution
========

Using different trace chunk instances for relay sessions belonging to a
single sessiond session goes against the intended use of the sessiond
trace chunk registry.

A sessiond trace chunk registry allows the relay daemon to share trace
chunks used by different "relay sessions" when they were created for the
same user-visible session daemon session. Tracing multiple domains (e.g.
ust and kernel) results in per-domain relay sessions being created.

Sharing trace chunks, and their output directory more specifically, is
essential to properly implement session rotations. The sharing of output
directory handles allows directory renames to be performed once and
without races that would stem from from multiple renames.

The reason why sessiond trace chunk registry returns different trace
chunk instances for two relay sessions is that the wrong session `id` is
used to publish trace chunks. The `id` that must be used to share trace
chunks accross the relay sessions that belong to the same sessiond
session is `id_sessiond`.

`id_sessiond` is optional as it is only provided by consumers v2.11+.
Otherwise, it is fine to use the relay session `id`: it is a unique id
for a given session daemon instance and those consumers will not issue a
session rotation (or clear) as the feature didn't exist.

A reference counting bug revealed by this change is also fixed in the
implementation of the sessiond trace chunk registry.

When the trace chunk is first published, two references to the published
chunks exist. One is taken by the registry while the other is being
returned to the caller. In the use case of the relay daemon, the
reference held by the registry itself is undesirable.

We want the trace chunk to be removed from the registry as soon as it is
not being used by the relay daemon (through a session or a stream). This
differs from the behaviour of the consumer daemon which relies on an
explicit command from the session daemon to release the registry's
reference.

In cases where the trace chunk had already been published, the reference
belonging to the sessiond trace chunk registry instance has already been
'put' by the firt publication. We must simply return the published trace
chunk with a reference taken on behalf of the caller.

Known drawbacks
===============

None.

Change-Id: Ic33443b114a87574a1b26ac5ccb022e47f886ddd
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Docs: relayd: document the role of session_trace_chunk_registry

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6158dadfe92d23c0081aac888c3e54b3c9c711a3

Fix: lttng-ctl: lttng_list_sessions: initialize out_sessions to NULL when returning 0

Observed issue
==============

Users of lttng-ctl API's lttng_list_sessions observe application crash
when freeing the *out_sessions output value when lttng_list_sessions
returns 0.

Cause
=====

The implementation does not set *out_sessions to NULL when
lttng_ctl_ask_sessiond() sets the sessions variable to NULL.
This causes the user application to attempt to free(3) an uninitialized
pointer.

Solution
========

Initialize out_sessions to NULL before invoking
lttng_ctl_ask_sessiond(), so it is initialized when lttng_list_sessions
returns 0, thus allowing *out_sessions to be subsequently freed.
A free(3) on a NULL pointer is a no-op.

Known drawbacks
===============

None.

History
=======

This was introduced by those two commits:

b178f53e90 ("Generate session name and default output on sessiond's end")
27ea4ba825 ("Fix: error when listing sessions with no session")

This is a regression present in the stable-2.11, stable-2.12,
stable-2.13, and master branches.

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

relayd: add `lttng_viewer_command_str()`

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Icb7be4901727654c6339ed47344320140fdaf9fb

relayd: add `lttcomm_relayd_command_str()`

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Idfd708843d19ca1b2cfe458a3b2b37e55f9c3274

Fix: lttng: initialize variable in run_command_string

I got some crashes when using `lttng track` and hitting some error
paths.  The tracker_handle variable is run_command_string is passed to
lttng_process_attr_tracker_handle_destroy uninitialized if
lttng_session_get_tracker_handle fails.

    $ valgrind lttng track --kernel --pid 569878
    ==634572== Memcheck, a memory error detector
    ==634572== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
    ==634572== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
    ==634572== Command: lttng track --kernel --pid 569878
    ==634572==
    Error: Unknown error
    ==634572== Conditional jump or move depends on uninitialised value(s)
    ==634572==    at 0x4875007: lttng_process_attr_tracker_handle_destroy (tracker.cpp:25)
    ==634572==    by 0x13AC55: run_command_string(cmd_type, char const*, lttng_domain_type, lttng_process_attr, char const*, mi_writer*) (track-untrack.cpp:485)
    ==634572==    by 0x13ADA5: run_command(cmd_type, char const*, process_attr_command_args const*, mi_writer*) (track-untrack.cpp:535)
    ==634572==    by 0x13B472: cmd_track_untrack(cmd_type, int, char const**, char const*) (track-untrack.cpp:740)
    ==634572==    by 0x13B5D9: cmd_track(int, char const**) (track-untrack.cpp:805)
    ==634572==    by 0x14C598: handle_command(int, char**) (lttng.cpp:237)
    ==634572==    by 0x14CCE9: parse_args(int, char**) (lttng.cpp:426)
    ==634572==    by 0x14CE65: main (lttng.cpp:475)

Fix it by initializing it to NULL.

Change-Id: Id2693e75c3c5c83cef58db3109973d7ab679b859
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Fix: enable-rotation: wrong type of rotation specified in message

48a4000 changed the initialization of the schedule_type_str to allow the
code to compile as C++. The C-version of the code explicitly specified
the enum values which was "safer" while the C++ version assumes that the
array is initialized in the order of the enum's values.

This results in an incorrect message when enabling a rotation schedule.
For instance, enabling a periodic rotation will print that a
"size-based" rotation was successfully enabled.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3cb6fcb6a67d7b0fa4c7913694923b5f03255389

common: replace uses of calloc(..., 1) with zmalloc

I found these uses of calloc, which could be replaced with zmalloc. I
presumed that the reason for using calloc was for the fact that it
initializes the memory to zeroes.

Change-Id: Icd8b84d707978e949c55d9694e6dbc10319c0169
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Fix: consumer-stream: live viewers observe timestamps going backwards

Observed issue
==============

When stress-testing over an entire weekend, we caught the following
occurrences of timestamps going backwards with babeltrace in live viewer
mode:

One occurrence:

  # Test ust streaming live clear with viewer with new metadata after clear
  # Parameters: tracing_active=0, clear_twice=0, buffer_type=pid

6-7 occurrences:

  # Test ust basic streaming live with viewer
  # Parameters: tracing_active=1, clear_twice=0, buffer_type=uid

Relevant log of the relayd focused on the context of the stream
triggering the issue (stream 994):

506:DBG1 - 17:26:54.966288486 [3866648/3866655]: Relay viewer stream 994 not found (in viewer_stream_get_by_id() at viewer-stream.cpp:268)
603:DBG1 - 17:26:54.970265349 [3866648/3866655]: Sending stream 994 to viewer (in send_viewer_streams() at live.cpp:241)
843:DBG1 - 17:26:55.959835404 [3866648/3866652]: stream_add_index for stream 994 (in stream_add_index() at stream.cpp:1233)
844:DBG3 - 17:26:55.959866303 [3866648/3866652]: Finding index for stream id 994 and seq_num 0 (in relay_index_get_by_id_or_create() at index.cpp:112)
845:DBG2 - 17:26:55.959896611 [3866648/3866652]: Creating relay index for stream id 994 and seqnum 0 (in relay_index_create() at index.cpp:34)
846:DBG2 - 17:26:55.959928097 [3866648/3866652]: Adding relay index with stream id 994 and seqnum 0 (in relay_index_add_unique() at index.cpp:70)
847:DBG2 - 17:26:55.959968163 [3866648/3866652]: Index found or created in HT for stream ID 994 and seqnum 0 (in relay_index_get_by_id_or_create() at index.cpp:144)
884:DBG3 - 17:26:55.961676906 [3866648/3866652]: Receiving data for stream id 994 seqnum 0, 0 bytes received, 84 bytes left to receive (in relay_process_data_receive_payload() at main.cpp:3583)
885:DBG1 - 17:26:55.961783540 [3866648/3866652]: Wrote to stream 994: data_length = 84, padding_length = 0 (in stream_write() at stream.cpp:1113)
886:DBG1 - 17:26:55.961862441 [3866648/3866652]: Wrote to stream 994: data_length = 0, padding_length = 4012 (in stream_write() at stream.cpp:1113)
887:DBG1 - 17:26:55.961895593 [3866648/3866652]: handle_index_data: stream 994 net_seq_num 0 data offset 0 (in stream_update_index() at stream.cpp:1140)
888:DBG3 - 17:26:55.961945901 [3866648/3866652]: Finding index for stream id 994 and seq_num 0 (in relay_index_get_by_id_or_create() at index.cpp:112)
889:DBG2 - 17:26:55.961983056 [3866648/3866652]: Index found or created in HT for stream ID 994 and seqnum 0 (in relay_index_get_by_id_or_create() at index.cpp:144)
894:DBG2 - 17:26:55.962334285 [3866648/3866652]: Writing index for stream ID 994 and seq num 0 (in relay_index_try_flush() at index.cpp:275)
895:DBG2 - 17:26:55.962390756 [3866648/3866652]: index put for stream id 994 and seqnum 0 refcount 1 (in relay_index_put() at index.cpp:237)
1743:DBG1 - 17:26:56.083287172 [3866648/3866655]: Check index status: index_received_seqcount 1 index_sent_seqcount 0 for stream 994 (in check_index_status() at live.cpp:1454)
1746:DBG1 - 17:26:56.083446379 [3866648/3866655]: Sending viewer index for stream 994 offset 0 (in viewer_get_next_index() at live.cpp:1801)
1748:DBG1 - 17:26:56.083544877 [3866648/3866655]: Index 1 for stream 994 sent (in viewer_get_next_index() at live.cpp:1842)
1751:DBG1 - 17:26:56.083778149 [3866648/3866655]: Sent 4108 bytes for stream 994 (in viewer_get_packet() at live.cpp:1968)
2858:DBG1 - 17:26:56.907744916 [3866648/3866652]: stream_add_index for stream 994 (in stream_add_index() at stream.cpp:1233)
2859:DBG1 - 17:26:56.907762798 [3866648/3866652]: Received live beacon for stream 994 (in stream_add_index() at stream.cpp:1237)
2862:DBG3 - 17:26:56.907855597 [3866648/3866652]: Receiving data for stream id 994 seqnum 1, 0 bytes received, 84 bytes left to receive (in relay_process_data_receive_payload() at main.cpp:3583)
2863:DBG1 - 17:26:56.907950195 [3866648/3866652]: Wrote to stream 994: data_length = 84, padding_length = 0 (in stream_write() at stream.cpp:1113)
2864:DBG1 - 17:26:56.908002911 [3866648/3866652]: Wrote to stream 994: data_length = 0, padding_length = 4012 (in stream_write() at stream.cpp:1113)
2865:DBG1 - 17:26:56.908024312 [3866648/3866652]: handle_index_data: stream 994 net_seq_num 1 data offset 4096 (in stream_update_index() at stream.cpp:1140)
2866:DBG3 - 17:26:56.908043082 [3866648/3866652]: Finding index for stream id 994 and seq_num 1 (in relay_index_get_by_id_or_create() at index.cpp:112)
2867:DBG2 - 17:26:56.908061879 [3866648/3866652]: Creating relay index for stream id 994 and seqnum 1 (in relay_index_create() at index.cpp:34)
2868:DBG2 - 17:26:56.908082115 [3866648/3866652]: Adding relay index with stream id 994 and seqnum 1 (in relay_index_add_unique() at index.cpp:70)
2869:DBG2 - 17:26:56.908101275 [3866648/3866652]: Index found or created in HT for stream ID 994 and seqnum 1 (in relay_index_get_by_id_or_create() at index.cpp:144)
3011:DBG1 - 17:26:56.913436908 [3866648/3866655]: Check index status: index_received_seqcount 1 index_sent_seqcount 1 for stream 994 (in check_index_status() at live.cpp:1454)
3012:DBG1 - 17:26:56.913457688 [3866648/3866655]: Check index status: inactive with beacon, for stream 994 (in check_index_status() at live.cpp:1492)
3014:DBG1 - 17:26:56.913507164 [3866648/3866655]: Index 1 for stream 994 sent (in viewer_get_next_index() at live.cpp:1842)
3043:DBG1 - 17:26:56.914167206 [3866648/3866652]: stream_add_index for stream 994 (in stream_add_index() at stream.cpp:1233)
3044:DBG3 - 17:26:56.914186324 [3866648/3866652]: Finding index for stream id 994 and seq_num 1 (in relay_index_get_by_id_or_create() at index.cpp:112)
3045:DBG2 - 17:26:56.914205597 [3866648/3866652]: Index found or created in HT for stream ID 994 and seqnum 1 (in relay_index_get_by_id_or_create() at index.cpp:144)
3046:DBG2 - 17:26:56.914227502 [3866648/3866652]: Writing index for stream ID 994 and seq num 1 (in relay_index_try_flush() at index.cpp:275)
3047:DBG2 - 17:26:56.914299536 [3866648/3866652]: index put for stream id 994 and seqnum 1 refcount 1 (in relay_index_put() at index.cpp:237)
3587:DBG1 - 17:26:57.067977800 [3866648/3866652]: Set begin data pending flag to stream 994 (in relay_begin_data_pending() at main.cpp:2240)
3644:DBG1 - 17:26:57.070915787 [3866648/3866652]: Data pending for stream id 994: prev_data_seq 1, prev_index_seq 1, and last_seq 1 (in relay_data_pending() at main.cpp:2091)
3913:DBG1 - 17:26:57.093492259 [3866648/3866652]: try_rotate_stream_index: Stream 994 (rotate_at_packet_seq_num = 2, received_packet_seq_num = (value = 1, is_set = 1)) (in try_rotate_stream_index() at stream.cpp:482)
3914:DBG1 - 17:26:57.093525950 [3866648/3866652]: Rotating stream 994 index file (in try_rotate_stream_index() at stream.cpp:511)
3915:DBG1 - 17:26:57.093561064 [3866648/3866652]: try_rotate_stream_data: Stream 994 (rotate_at_index_packet_seq_num = 2, rotate_at_prev_data_net_seq = 1, prev_data_seq = 1) (in try_rotate_stream_data() at stream.cpp:357)
3916:DBG1 - 17:26:57.093591085 [3866648/3866652]: Rotating stream 994 data file with size 8192 (in stream_rotate_data_file() at stream.cpp:138)
3917:DBG1 - 17:26:57.093626697 [3866648/3866652]: stream_rotate_data_file: reset tracefile_size_current for stream 994 was 8192 (in stream_rotate_data_file() at stream.cpp:169)
3918:DBG1 - 17:26:57.093656578 [3866648/3866652]: Rotation completed for stream 994 (in stream_complete_rotation() at stream.cpp:66)
4238:DBG1 - 17:26:57.635064782 [3866648/3866652]: Trying to close stream 994 (in try_stream_close() at stream.cpp:883)
4239:DBG1 - 17:26:57.635098224 [3866648/3866652]: Succeeded in closing stream 994 (in try_stream_close() at stream.cpp:983)
4744:DBG1 - 17:26:57.741972785 [3866648/3866655]: Check index status: index_received_seqcount 2 index_sent_seqcount 1 for stream 994 (in check_index_status() at live.cpp:1454)
4745:DBG1 - 17:26:57.742030216 [3866648/3866655]: Sending viewer index for stream 994 offset 4096 (in viewer_get_next_index() at live.cpp:1801)
4747:DBG1 - 17:26:57.742088421 [3866648/3866655]: Index 2 for stream 994 sent (in viewer_get_next_index() at live.cpp:1842)
4750:DBG1 - 17:26:57.742197990 [3866648/3866655]: Sent 4108 bytes for stream 994 (in viewer_get_packet() at live.cpp:1968)
4755:DBG1 - 17:26:57.932525633 [3866648/3866655]: Releasing stream id 994 (in stream_release() at stream.cpp:778)
4756:DBG1 - 17:26:57.932555313 [3866648/3866655]: Rotation completed for stream 994 (in stream_complete_rotation() at stream.cpp:66)

Cause
=====

This log shows the following sequence:

   consumer-data                       consumer-live-timer                     relayd-main                     relayd-live               babeltrace

1) lttng_consumer_read_subbuffer
  2) get next subbuf
  3) write data packet to data socket
     (starting here the data packet is
     somewhere on the network)
  4) put next subbuf
  5) post_consume()
    6) consumer_stream_sync_metadata_index()
      7) wait for metadata
         8) consumer_stream_sync_metadata()
                                       9) check_stream()
                                       10) set missed_metadata_flush
      11) call send_live_beacon(()
        12) sample empty ring buffer
        13) read current timestamp
        14) send inactivity beacon (empty packet)
                                                                               15) receives a live beacon                                                     (@ 17:26:56.907762798)
      16) call consumer_stream_send_index()
         17) send packet index to relayd
                                                                               18) receives a data packet                                                     (@ 17:26:56.907855597)
                                                                                  (at this point the data
                                                                                   packet is received from the
                                                                                   network)
                                                                                                                                         19) ask for next index
                                                                                                               20) informs the live viewer of the live beacon (@ 17:26:56.913457688)
                                                                               21) receives an index packet                                                   (@ 17:26:56.914227502)
                                                                                                                                         22) ask for next index
                                                                                                               23) sends the packet to the viewer             (@ 17:26:57.742197990)
                                                                                                                                         24) observes time going
                                                                                                                                             backwards between the
                                                                                                                                             previous live beacon
                                                                                                                                             and the data packet.

The issue is caused by consumer_stream_sync_metadata_index which is
called after sending a data packet (therefore after having consumed a
data packet from the ring buffer). It invokes the send_live_beacon
callback before sending the index associated with the data packet that
was just sent.

However, this introduces a discrepancy between the live beacon
inactivity guarantees and the yet-to-be-sent packet index: the data
packet sent at [3] can be anywhere on the network, not even received by
the relay daemon, when the live beacon is sampling a now empty ring
buffer at [12], and thus sends a live inactivity beacon to the relay
daemon. Then, when the index is sent by consumer_stream_send_index
[16], its timestamp is in the past compared to the inactivity beacon
sent by send_live_beacon [11].

The purpose of the field "stream->indexes_in_flight" is to prevent
setting the inactivity timestamp in the relay stream when data is
missing for indexes that were received. This works because the indexes
are sent over the control socket, which is where the inactivity beacons
are also sent. It does not however prevent issues the other way around:
data sent prior to the inactivity beacon may or may not have reached
the relay daemon. It is therefore important to make sure that consuming
ring buffer data and sending that data's index vs sampling for an empty
ring buffer and sending an inactivity beacon are correctly ordered.

Solution
========

Send inactivity beacon after packet index.

Also document the purpose of sending an inactivity beacon in this
scenario.

Note
====

This issue is present since lttng-tools 2.7.0 (backported to 2.6.1),
where lttng_ustconsumer_read_subbuffer() invokes
consumer_flush_ust_index() prior to call consumer_stream_write_index().
It was introduced by commit 288bdb302a1 ("Fix: sessiond vs consumerd
push/get metadata deadlock").

Known drawbacks
===============

None.

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

Fix: relayd: ressource leaks on viewer_stream_create error

Observed issue
==============

When facing failure to open viewer stream chunks in the context of "Fix:
relayd: failure to open chunk files concurrently with session clear",
we observe that the relay daemon triggers an assertion due to a
non-empty session hash table on cleanup.

Cause
=====

viewer_stream_create() does a stream_get(), but without any matching
stream_put() on error. This in turn holds a reference on the ctf_trace,
which holds a reference on the session.

By inspecting the code, we notice that the following ressources can be
leaked on error:

- vstream->stream_file.handle,
- vstream->index_file,
- vstream->stream.

In non-error scenarios, viewer_stream_release() is responsible for
releasing references on the composite objects.

The vstream->stream_file.trace_chunk is not an issue because it is put
in the destroy handler (as well as within the release, before having its
vstream->stream_file.trace_chunk pointer set to NULL).

Solution
========

Properly put references on all objects which are contained by the viewer
stream on error by introducing
viewer_stream_release_composite_objects(), which is used both in the
error path of viewer_stream_create() and in viewer_stream_release().

Note
====

Why not move those "put" operations in viewer_stream_destroy ?

This is done in the release to ensure we put references on composite
objects immediately when our own reference reaches 0, rather than
waiting for a grace period through call_rcu, which could then cause
chained call_rcu callbacks and require multiple invocation of
rcu_barrier on relayd exit to guarantee that all callbacks have been
executed and all ressources properly freed.

Known drawbacks
===============

None.

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

Fix: relayd: live: erroneous message timestamp observed from live viewer

Observed issue
==============

Another situation where erroneous message timestamp is observed by the
live viewer. Happens rarely (only two occurrences while running
ust_clear in stress-tests overnight on a 16-core machine).

Triggered with the following test:

  # Test ust streaming live clear with viewer with new metadata after clear
  # Parameters: tracing_active=0, clear_twice=1, buffer_type=pid

Babeltrace 2 error:

  11-19 06:21:02.571 488497 488497 E PLUGIN/SRC.CTF.LTTNG-LIVE handle_late_message@lttng-live.c:1206 [lttng-live] Invalid live stream state: have a late message that is not a packet discarded or event discarded message: late-msg-type=PACKET_BEGINNING
  11-19 06:21:02.578 488497 488497 E PLUGIN/SRC.CTF.LTTNG-LIVE next_stream_iterator_for_trace@lttng-live.c:1360 [lttng-live] Late message could not be handled correctly: lttng-live-msg-iter-addr=0x55b4cdee1590, stream-name="stream-1024", curr-msg-ts=1637302861380968303, last-msg-ts=1637302861472798701
  11-19 06:21:02.578 488497 488497 E PLUGIN/SRC.CTF.LTTNG-LIVE lttng_live_msg_iter_next@lttng-live.c:1802 [lttng-live] Error preparing the next batch of messages: live-iter-status=LTTNG_LIVE_ITERATOR_STATUS_ERROR
  11-19 06:21:02.579 488497 488497 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:866 Component input port message iterator's "next" method failed: iter-addr=0x55b4cdecfe30, iter-upstream-comp-name="lttng-live", iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=SOURCE, iter-upstream-comp-class-name="lttng-live", iter-upstream-comp-class-partial-descr="Connect to an LTTng relay daemon", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR

Cause
=====

viewer_get_next_index() does not protect its use of the session
ongoing_rotation state by any synchronization mechanism. It only takes
the stream lock, which is not used to protect changes to the ongoing
rotation state with respect to the chunk rename operation performed by
chunk creation at the beginning of the clear command.

Solution
========

Protect the use of the session ongoing rotation state and file open
operations by the session lock in viewer_get_next_index().

Known drawbacks
===============

I don't expect this to cause any real scalability concern considering
the fact that the relay daemon has only two threads, one to handle
session daemon commands, and the other to handle viewer commands.

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

relayd: clarify viewer_get_next_index no rotation condition

The condition identifying cases where a viewer_get_next_index must not
perform a viewer stream rotation is relatively tricky to comprehend.

Split it in two intermediate variables, and use those variables to
print debugging statements when conditions for not performing a
rotation match.

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

Fix: relayd: failure to open chunk files concurrently with session clear

Observed issue
==============

When stress-testing ust clear with an active live viewer, we observe a
situation where the live viewer thread fails to open chunk files in
make_viewer_streams() when executed after the creation of the new trace
chunk at the beginning of the clear command:

  DBG3 - 16:19:50.923577790 [40834/40838]: Processing "RELAYD_CREATE_TRACE_CHUNK" command for socket 19 (in relay_process_control_command() at main.cpp:3262)
  DBG1 - 16:19:50.923577730 [40834/40841]: Relay viewer stream 225 not found (in viewer_stream_get_by_id() at viewer-stream.cpp:265)
  DBG1 - 16:19:50.923600762 [40834/40838]: lttng_trace_chunk_rename_path from  to .tmp_old_chunk (in lttng_trace_chunk_rename_path_no_lock() at trace-chunk.cpp:759)
  DBG1 - 16:19:50.923627202 [40834/40841]: Opening trace chunk file "ust/uid/0/64-bit/my_chan-0_26" (in _lttng_trace_chunk_open_fs_handle_locked() at trace-chunk.cpp:1359)
  DBG1 - 16:19:50.923664685 [40834/40841]: Adding new file "ust/uid/0/64-bit/my_chan-0_26" to trace chunk "(unnamed)" (in lttng_trace_chunk_add_file() at trace-chunk.cpp:1309)
  DBG1 - 16:19:50.923706441 [40834/40841]: Relay viewer stream 226 not found (in viewer_stream_get_by_id() at viewer-stream.cpp:265)
  DBG1 - 16:19:50.923727770 [40834/40841]: Opening trace chunk file "ust/uid/0/64-bit/my_chan-0_27" (in _lttng_trace_chunk_open_fs_handle_locked() at trace-chunk.cpp:1359)
  DBG1 - 16:19:50.923744686 [40834/40841]: Adding new file "ust/uid/0/64-bit/my_chan-0_27" to trace chunk "(unnamed)" (in lttng_trace_chunk_add_file() at trace-chunk.cpp:1309)
  DBG1 - 16:19:50.923773427 [40834/40841]: Relay viewer stream 227 not found (in viewer_stream_get_by_id() at viewer-stream.cpp:265)
  DBG1 - 16:19:50.923803791 [40834/40841]: Opening trace chunk file "ust/uid/0/64-bit/my_chan-0_28" (in _lttng_trace_chunk_open_fs_handle_locked() at trace-chunk.cpp:1359)
  DBG1 - 16:19:50.923831589 [40834/40841]: Adding new file "ust/uid/0/64-bit/my_chan-0_28" to trace chunk "(unnamed)" (in lttng_trace_chunk_add_file() at trace-chunk.cpp:1309)
  DBG1 - 16:19:50.923865981 [40834/40841]: Relay viewer stream 228 not found (in viewer_stream_get_by_id() at viewer-stream.cpp:265)
  DBG1 - 16:19:50.923889329 [40834/40841]: Opening trace chunk file "ust/uid/0/64-bit/index/my_chan-0_29.idx" (in _lttng_trace_chunk_open_fs_handle_locked() at trace-chunk.cpp:1359)
  DBG1 - 16:19:50.923905142 [40834/40838]: Creating trace chunk: chunk_id = 1, creation time = 20211118-161950 (in lttng_trace_chunk_create() at trace-chunk.cpp:440)
  DBG1 - 16:19:50.923907984 [40834/40841]: Adding new file "ust/uid/0/64-bit/index/my_chan-0_29.idx" to trace chunk "(unnamed)" (in lttng_trace_chunk_add_file() at trace-chunk.cpp:1309)
  DBG1 - 16:19:50.923937804 [40834/40838]: Chunk name set to "20211118T161950+0000-1" (in lttng_trace_chunk_create() at trace-chunk.cpp:471)
  PERROR - 16:19:50.923984288 [40834/40841]: Failed to open fs handle to ust/uid/0/64-bit/index/my_chan-0_29.idx, open() returned: No such file or directory (in fd_tracker_open_fs_handle() at fd-tracker.cpp:548)
  DBG1 - 16:19:50.924050763 [40834/40841]: Opening trace chunk file "ust/uid/0/64-bit/my_chan-0_29" (in _lttng_trace_chunk_open_fs_handle_locked() at trace-chunk.cpp:1359)
  DBG1 - 16:19:50.924074480 [40834/40841]: Adding new file "ust/uid/0/64-bit/my_chan-0_29" to trace chunk "(unnamed)" (in lttng_trace_chunk_add_file() at trace-chunk.cpp:1309)
  PERROR - 16:19:50.924094720 [40834/40841]: Failed to open fs handle to ust/uid/0/64-bit/my_chan-0_29, open() returned: No such file or directory (in fd_tracker_open_fs_handle() at fd-tracker.cpp:548)
  DBG1 - 16:19:50.924193679 [40834/40841]: Viewer connection closed with 23 (in thread_worker() at live.cpp:2542)
  DBG1 - 16:19:50.924227482 [40834/40838]: Attempting to publish trace chunk: sessiond {34038782-6f74-4b2d-801e-752cf3d8885e}, session_id = 7, chunk_id = 1 (in sessiond_trace_chunk_registry_publish_chunk() at sessiond-trace-chunks.cpp:385)
  DBG1 - 16:19:50.924312916 [40834/40838]: Reset communication state of relay connection (fd = 19) (in connection_reset_protocol_state() at connection.cpp:82)
  DBG3 - 16:19:50.924350200 [40834/40838]: Relayd worker thread polling... (in relay_thread_worker() at main.cpp:3833)
  DBG3 - 16:19:50.924365222 [40834/40841]: Relayd live viewer worker thread polling... (in thread_worker() at live.cpp:2456)
  DBG1 - 16:19:50.926718319 [40834/40838]: Done receiving control command header: fd = 19, cmd = 18, cmd_version = 0, payload size = 532 bytes (in relay_process_control_receive_header() at main.cpp:3422)
  DBG3 - 16:19:50.926755574 [40834/40838]: Relayd worker thread polling... (in relay_thread_worker() at main.cpp:3833)
  DBG1 - 16:19:50.926787638 [40834/40838]: Done receiving control command payload: fd = 19, payload size = 532 bytes (in relay_process_control_receive_payload() at main.cpp:3339)
  DBG3 - 16:19:50.926811247 [40834/40838]: Processing "RELAYD_ROTATE_STREAMS" command for socket 19 (in relay_process_control_command() at main.cpp:3258)

Cause
=====

This is caused by relay_create_trace_chunk() using
lttng_trace_chunk_rename_path() to move away each trace subdirectory
into the subdirectory .tmp_old_chunk, and making this the new top-level
chunk directory (temporarily). This is a temporary state which will be
resorbed on relay_close_trace_chunk(), moving back the top-level chunk
directory to its original place.

Attempts to open chunk files from the prior chunk may result in failures,
because the chunk lock protecting the chunk rename operation only
protects the chunk owned by the relay thread, not its copy(ies) owned by
the live viewer thread.

This intermediate state should _not_ be observed by the live viewer
thread. The session ongoing rotation state should prevent the live
viewer threads from observing this.

Solution
========

Set the ongoing rotation state in relay_create_trace_chunk() earlier:
before invoking lttng_trace_chunk_rename_path(). Also ensure that the
session ongoing rotation state is protected by the session lock.

On the live thread side, introduce use of the session ongoing rotation
state in viewer_get_new_streams() and viewer_attach_session() to
effectively skip creation of the viewer streams if a session has a
rotation ongoing.

Viewers are expected to deal with the LTTNG_VIEWER_NEW_STREAMS_NO_NEW
reply (or handle the fact that no streams are currently available) and
try again later.

Both Babeltrace 2.0 and Babeltrace 1.5 appear to handle those replies
correctly.

Known drawbacks
===============

None.

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

common: compile argpar-utils as C++

argpar-utils was in review while everything was converted to be compiled
as C++, so it missed the boat. Do it now.

Change-Id: Ica005b3b27cc0bd9ef6e886d7078649096c7c7a2
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Fix: relayd: live: metadata stream reference count < 0 assert

Observed issue
==============

While running tests/regression/tools/clear/test_ust test in a loop we
eventually witness the following error:

The symptom on the Babeltrace side is a Connection reset by peer. This
is caused by a relayd abort after an assertion failure due to a
reference count being lower than 0.

Test case:

  # Test ust streaming live clear with viewer
  # Parameters: tracing_active=0, clear_twice=0, buffer_type=pid

Tools commit: https://review.lttng.org/c/lttng-tools/+/6754/1

Relay daemon assertion failure stack trace:

  (gdb) bt
  #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
  #1  0x00007ffa2e850859 in __GI_abort () at abort.c:79
  #2  0x00007ffa2e850729 in __assert_fail_base (fmt=0x7ffa2e9e6588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x56221bdb96f4 "res >= 0",
      file=0x56221bdb97a0 "/root/virtenv/usr/include/urcu/ref.h", line=66, function=<optimized out>) at assert.c:92
  #3  0x00007ffa2e861f36 in __GI___assert_fail (assertion=assertion@entry=0x56221bdb96f4 "res >= 0", file=file@entry=0x56221bdb97a0 "/root/virtenv/usr/include/urcu/ref.h", line=line@entry=66,
      function=function@entry=0x56221bdbd638 <__PRETTY_FUNCTION__.7554> "urcu_ref_put") at assert.c:101
  #4  0x000056221bd6a1cc in urcu_ref_put (release=<optimized out>, ref=0x7ffa24008cb0) at /root/virtenv/usr/include/urcu/ref.h:66
  #5  viewer_stream_put (vstream=vstream@entry=0x7ffa24008cb0) at viewer-stream.c:279
  #6  0x000056221bd5e4c5 in viewer_get_metadata (conn=conn@entry=0x7ffa0c000fc0) at live.c:2211
  #7  0x000056221bd63778 in process_control (conn=0x7ffa0c000fc0, recv_hdr=0x7ffa297c5af0) at live.c:2376
  #8  thread_worker (data=<optimized out>) at live.c:2541
  #9  0x00007ffa2ea28609 in start_thread (arg=<optimized out>) at pthread_create.c:477
  #10 0x00007ffa2e94d293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Babeltrace crash:

11-17 05:13:05.197 47007 47007 D PLUGIN/SRC.CTF.LTTNG-LIVE/VIEWER lttng_live_get_next_index@viewer-connection.c:1494 [live] Requesting next index for stream: cmd=GET_NEXT_INDEX, viewer-stream-id=3352
11-17 05:13:05.215 47007 47007 E PLUGIN/SRC.CTF.LTTNG-LIVE/VIEWER lttng_live_recv@viewer-connection.c:245 [live] Error receiving from Relay: Connection reset by peer.
11-17 05:13:05.215 47007 47007 E PLUGIN/SRC.CTF.LTTNG-LIVE/VIEWER lttng_live_get_next_index@viewer-connection.c:1522 [live] Error receiving get next index reply
11-17 05:13:05.215 47007 47007 D PLUGIN/SRC.CTF.LTTNG-LIVE lttng_live_iterator_next_msg_on_stream@lttng-live.c:1069 [live] Returning from advancing live stream iterator: status=LTTNG_LIVE_ITERATOR_STATUS_ERROR, stream-name="stream-3352", viewer-stream-id=3352
11-17 05:13:05.215 47007 47007 E PLUGIN/SRC.CTF.LTTNG-LIVE lttng_live_msg_iter_next@lttng-live.c:1802 [live] Error preparing the next batch of messages: live-iter-status=LTTNG_LIVE_ITERATOR_STATUS_ERROR
11-17 05:13:05.216 47007 47007 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:866 Component input port message iterator's "next" method failed: iter-addr=0x563b7dd07700, iter-upstream-comp-name="live", iter-upstream-comp-log-level=TRACE, iter-upstream-comp-class-type=SOURCE, iter-upstream-comp-class-name="lttng-live", iter-upstream-comp-class-partial-descr="Connect to an LTTng relay daemon", iter-upstream-port-type=OUT
PUT, iter-upstream-port-name="out", status=ERROR
11-17 05:13:05.216 47007 47007 E PLUGIN/FLT.UTILS.MUXER muxer_upstream_msg_iter_next@muxer.c:446 [mux] Upstream iterator's next method returned an error: status=ERROR
11-17 05:13:05.216 47007 47007 E PLUGIN/FLT.UTILS.MUXER validate_muxer_upstream_msg_iters@muxer.c:989 [mux] Cannot validate muxer's upstream message iterator wrapper: muxer-msg-iter-addr=0x563b7dd04740, muxer-upstream-msg-iter-wrap-addr=0x563b7dd0db30
11-17 05:13:05.216 47007 47007 E PLUGIN/FLT.UTILS.MUXER muxer_msg_iter_next@muxer.c:1417 [mux] Cannot get next message: comp-addr=0x563b7dd1a760, muxer-comp-addr=0x563b7dd1b930, muxer-msg-iter-addr=0x563b7dd04740, msg-iter-addr=0x563b7dd07590, status=ERROR
11-17 05:13:05.216 47007 47007 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:866 Component input port message iterator's "next" method failed: iter-addr=0x563b7dd07590, iter-upstream-comp-name="mux", iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=FILTER, iter-upstream-comp-class-name="muxer", iter-upstream-comp-class-partial-descr="Sort messages from multiple inpu", iter-upstream-port-type=OUTPUT,
iter-upstream-port-name="out", status=ERROR
11-17 05:13:05.216 47007 47007 W LIB/GRAPH consume_graph_sink@graph.c:462 Component's "consume" method failed: status=ERROR, comp-addr=0x563b7dd06d20, comp-name="pretty", comp-log-level=WARNING, comp-class-type=SINK, comp-class-name="pretty", comp-class-partial-descr="Pretty-print messages (`text` fo", comp-class-is-frozen=1, comp-class-so-handle-addr=0x563b7dd051b0, comp-class-so-handle-path="/root/virtenv/usr/lib/babeltr
ace2/plugins/babeltrace-plugin-text.so", comp-input-port-count=1, comp-output-port-count=0
11-17 05:13:05.217 47007 47007 E CLI cmd_run@babeltrace2.c:2537 Graph failed to complete successfully

Cause
=====

Both relay and live threads can put the ownership reference on the
metadata viewer stream concurrently without synchronization, thus
leading to a reference count going lower than 0.

The viewer stream ownership design initially planned for being owned by
the live thread, thus allowing the live thread to put the ownership
reference as soon as the associated relay stream is observed as closed,
and the viewer stream is considered as hung up.

However, in the specific case of the metadata viewer stream, the
responsibility of closing the metadata viewer stream is shared between
the relay and live threads, because the viewers expect to observe a
LTTNG_VIEWER_NO_NEW_METADATA message before the metadata stream
hangs up (see comment in viewer_get_metadata()). Therefore, if
viewer_get_metadata() is done before the metadata stream is closed, the
viewer will receive the LTTNG_VIEWER_NO_NEW_METADATA message, and set
the no_new_metadata_notified state to true. It's then the relay thread's
relay_close_stream() which will invoke the ownership put. However,
the live thread may concurrently try to put the viewer stream ownership
as well from a subsequent viewer_get_metadata(), thus leading to a
reference count < 0.

Solution
========

Fix this by putting the ownership reference from the live viewer thread
rather than the relay thread. This can be done by tracking the state of
no_new_metadata_notified within the live viewer thread.

Known drawbacks
===============

This will postpone reclaim of the metadata viewer stream from the
relay stream close to the following viewer_get_metadata (after a
LTTNG_VIEWER_NO_NEW_METADATA message has been replied), which I don't
think is an issue.

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

Fix: relayd: compare viewer chunks by ID rather than address

Observed issue
==============

In "Fix: relayd: live: erroneous message timestamp observed from live
viewer", we observe that sometimes viewer streams unexpectedly end up
performing a viewer stream rotation at session destroy.

Cause
=====

This may happen in the following scenario:

  1) Relay stream A is rotated to NULL.

  2) viewer_get_next_index for viewer stream A:
  2.1) observes a NULL rstream->trace_chunk, updates the viewer session
       current trace chunk to NULL (viewer_session_set_trace_chunk_copy).
  2.2) "Transition the viewer stream into the latest trace chunk
       available." does not issue viewer_stream_rotate_to_trace_chunk, because
       the condition (rstream->completed_rotation_count ==
       vstream->last_seen_rotation_count + 1 && !rstream->trace_chunk)
       evaluates to "true", and thus the entire if () evaluates to false.

  3) check_index_status detects rstream->closed and
     index_received_seqcount == index_sent_seqcount, thus replying HUP to
     viewer, effectively releasing ownership of the viewer stream.

  4) viewer_get_next_index for viewer stream B (not rotated to NULL yet):
  4.1) observes a non-NULL rstream->trace_chunk, updates the viewer
       session current trace chunk to *a new copy* of the non-NULL
       rstream->trace_chunk (viewer_session_set_trace_chunk_copy).
  4.2) the comparison (conn->viewer_session->current_trace_chunk !=
       vstream->stream_file.trace_chunk) done by pointer don't match, because
       the viewer session current trace chunk is a new copy.

Therefore, due to those stream close scenarios where the viewer session
can go back and forth between NULL and _different copies_ of the relay
chunk, we cannot use a comparison of chunks by address on the viewer
chunks.

Solution
========

Compare the viewer stream chunks by ID rather than address.

Known drawbacks
===============

The comparison is probably slightly slower, but I don't expect this to
be significant.

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

Fix: relayd: live: erroneous message timestamp observed from live viewer

Observed issue
==============

While running tests/regression/tools/clear/test_ust test in a loop we
eventually witness the following error:

  # Test ust basic streaming live with viewer
  # Parameters: tracing_active=0, clear_twice=0, buffer_type=uid
  ok 425 - Create session 6jbcTSKUG7s2RIp5 with uri:net://localhost and opts: --live
  ok 426 - Enable channel chan for session 6jbcTSKUG7s2RIp5
  ok 427 - Enable ust event tp:tptest for session 6jbcTSKUG7s2RIp5
  ok 428 - Start tracing for session 6jbcTSKUG7s2RIp5
  # Waiting for live trace at url: net://localhost
  ok 429 - Waiting for live trace at url: net://localhost
  # Waiting for live viewers on url: net://localhost
  ok 430 - Waiting for live viewers on url: net://localhost
  # Wait until viewer sees all 10 expected events
  ok 431 - Live viewer read 10 events, expect 10
  ok 432 - Destroy session 6jbcTSKUG7s2RIp5
  # Waiting for application to exit
  ok 433 - Wait for application to exit
  # Wait for viewer to exit
  10-28 22:07:37.935 764967 764967 E PLUGIN/SRC.CTF.LTTNG-LIVE next_stream_iterator_for_trace@lttng-live.c:1222 [lttng-live] Message's timestamp is less than lttng-live's message iterator's last returned timestamp: lttng-live-msg-iter-addr=0x55fe45d4977
  0, ts=1635458857116911882, last-msg-ts=1635458857123908033
  10-28 22:07:37.937 764967 764967 E PLUGIN/SRC.CTF.LTTNG-LIVE lttng_live_msg_iter_next@lttng-live.c:1662 [lttng-live] Error preparing the next batch of messages: live-iter-status=LTTNG_LIVE_ITERATOR_STATUS_ERROR
  10-28 22:07:37.937 764967 764967 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:861 Component input port message iterator's "next" method failed: iter-addr=0x55fe45d38c50, iter-upstream-comp-name="lttng-live", iter-upstream-comp-log-level=WARNING,
   iter-upstream-comp-class-type=SOURCE, iter-upstream-comp-class-name="lttng-live", iter-upstream-comp-class-partial-descr="Connect to an LTTng relay daemon", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
  10-28 22:07:37.937 764967 764967 E PLUGIN/FLT.UTILS.MUXER muxer_upstream_msg_iter_next@muxer.c:452 [muxer] Upstream iterator's next method returned an error: status=ERROR
  10-28 22:07:37.938 764967 764967 E PLUGIN/FLT.UTILS.MUXER validate_muxer_upstream_msg_iters@muxer.c:986 [muxer] Cannot validate muxer's upstream message iterator wrapper: muxer-msg-iter-addr=0x55fe45d33640, muxer-upstream-msg-iter-wrap-addr=0x55fe45d3
  af20
  10-28 22:07:37.938 764967 764967 E PLUGIN/FLT.UTILS.MUXER muxer_msg_iter_next@muxer.c:1411 [muxer] Cannot get next message: comp-addr=0x55fe45d49cb0, muxer-comp-addr=0x55fe45d49d30, muxer-msg-iter-addr=0x55fe45d33640, msg-iter-addr=0x55fe45d38ae0, sta
  tus=ERROR
  10-28 22:07:37.938 764967 764967 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:861 Component input port message iterator's "next" method failed: iter-addr=0x55fe45d38ae0, iter-upstream-comp-name="muxer", iter-upstream-comp-log-level=WARNING, iter
  -upstream-comp-class-type=FILTER, iter-upstream-comp-class-name="muxer", iter-upstream-comp-class-partial-descr="Sort messages from multiple inpu", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
  10-28 22:07:37.938 764967 764967 W LIB/GRAPH consume_graph_sink@graph.c:469 Component's "consume" method failed: status=ERROR, comp-addr=0x55fe45d49550, comp-name="pretty", comp-log-level=WARNING, comp-class-type=SINK, comp-class-name="pretty", comp-c
  lass-partial-descr="Pretty-print messages (`text` fo", comp-class-is-frozen=0, comp-class-so-handle-addr=0x55fe45d38460, comp-class-so-handle-path="/root/virtenv/usr/lib/babeltrace2/plugins/babeltrace-plugin-text.la", comp-input-port-count=1, comp-out
  put-port-count=0
  10-28 22:07:37.938 764967 764967 E CLI cmd_run@babeltrace2.c:2547 Graph failed to complete successfully

Cause
=====

When doing the rotation associated with a clear,
viewer_stream_sync_tracefile_array_tail aims at pushing forward the
index_sent_seqcount for sessions in tracefile rotation mode to the
oldest available seqcount (tail) for this stream. It does not take into
consideration that the current index_sent_seqcount may already be passed
that oldest available seqcount position, thus eventually re-emitting
trace data.

For sessions *not* in tracefile rotation mode (this is known because
`tracefile_array_get_seq_tail()` returns -1ULL), this function
erroneously resets the index_sent_seqcount to 0, thus also causing trace
data to be re-emitted.

Solution
========

Solve this by using the maximum between the current index_sent_seqcount
and the tracefile array tail position as new position in
viewer_stream_sync_tracefile_array_tail.

Notes
=====

This symptom is also observed without using the clear command, simply on
destroy with a live viewer attached. This is caused by another issue
(not addressed by this patch) which causes
viewer_stream_rotate_to_trace_chunk to be sometimes invoked when streams
are closed on destroy.

Known drawbacks
===============

None.

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

Validate channel context mismatch across UST applications

Observed issue
==============

Applications traced with LTTng-UST are expected to all provide the exact
same layout for their channel's context fields, else it leads to
corrupted traces. This is only enforced within LTTng-UST. There is
nothing in the session daemon that prevents this scenario, and it is
only observable when reading the corrupted trace.

This makes the entire trace unreadable from the point where it is
corrupted.

Cause
=====

Even though LTTng-UST sends the entire description of its context fields
along with the channel registration notification, there is no validation
of the context fields' content against the context fields present in the
ust registry.

Solution
========

Validate each registered UST channel context fields against the fields
present in the registry. Reject the application if there is a mismatch.

Known drawbacks
===============

None.

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

Fix: relayd comm: improperly packed rotate streams command header

Observed issue
==============

The implicit rotation of a session performed during its destruction
fails on LTTng 2.12 (and upwards) with the following errors:
  Error: Relayd rotate streams replied error 152
  Error: Relayd rotate stream failed. Cleaning up relayd 2
  Error: Rotate channel failed
  Failed to find relay daemon socket: relayd_id = 2
  Error: Failed to perform a quiet rotation as part of the destruction of session "my_session": Rotation failure on consumer

Cause
=====

Error 152 matches the LTTNG_ERR_INVALID_PROTOCOL error, which implies
that the consumer daemon sent an unexpected command to the relay daemon.

It was determined that the RELAYD_ROTATE_STREAMS command header is not
properly packed since the LTTNG_PACKED annotation was omitted from its
`new_chunk_id` optional field. The documentation of LTTNG_OPTIONAL_COMM
duly indicates that this is required.

Without the use of LTTNG_PACKED, various lengths of padding (3 or 7
bytes) are inserted between new_chunk_id's `is_set` and `value` field to
align `value`, which results in an incorrect interpretation of the
command's arguments.

The relay daemon catches the protocol error when it is built in a
configuration that inserts 7 bytes of padding, while the consumer only
inserts three.

Solution
========

The solution proposed here is not perfect, see "Known drawbacks".

First, if we were to annotate the field, patched consumer daemons would
send unintelligible command headers to unpatched relay daemons. Leaving
it as is is the least of all evils, see "Known drawbacks" for more
details.

From the relay daemon end, we can easily anticipate the padding problem
by reading the `stream_count` field and use it to determine the expected
size of the payload.

The difference between the actual size of the payload and the expected
size allows us to determine the padding size and use the appropriate
declaration of the structure to interpret the command's arguments.

Known drawbacks
===============

While this fix causes the relay daemon to handle all improperly packed
command headers received from an unpatched consumer daemon, the reverse
is not completely true.

The following tables show which cases are known to work and which are
known to be broken when patched and unpatched versions of the relay
and consumer daemons are mixed, with the various alignment constraints.

Note that here:
  - 4 byte alignement implies "daemon running on an architecture on
    which uint64_t is aligned on an 4-byte boundary" (e.g. x86),
  - 8 byte alignement implies "daemon running on an architecture on
    which uint64_t is aligned on an 8-byte boundary" (e.g. x86-64).

Scenario 1 - Unpatched relay daemon and unpatched consumer daemon
-----------------------------------------------------------------------------------
| Architecture alignment | 4 byte alignement consumerd | 8 byte alignment consumerd |
|------------------------|-----------------------------|----------------------------|
| 4 byte alignment relay |           Works             |           Fails            |
| 8 byte alignment relay |           Fails             |           Works            |
-----------------------------------------------------------------------------------

Scenario 2 - Patched relay daemon and unpatched consumer daemon
-----------------------------------------------------------------------------------
| Architecture alignment | 4 byte alignement consumerd | 8 byte alignment consumerd |
|------------------------|-----------------------------|----------------------------|
| 4 byte alignment relay |           Works             |           Works            |
| 8 byte alignment relay |           Works             |           Works            |
-----------------------------------------------------------------------------------

Scenario 3 - Unpatched relay daemon and patched consumer daemon
-----------------------------------------------------------------------------------
| Architecture alignment | 4 byte alignement consumerd | 8 byte alignment consumerd |
|------------------------|-----------------------------|----------------------------|
| 4 byte alignment relay |           Works             |           Fails            |
| 8 byte alignment relay |           Fails             |           Works            |
-----------------------------------------------------------------------------------

Scenario 4 - Patched relay daemon and patched consumer daemon
-----------------------------------------------------------------------------------
| Architecture alignment | 4 byte alignement consumerd | 8 byte alignment consumerd |
|------------------------|-----------------------------|----------------------------|
| 4 byte alignment relay |           Works             |           Works            |
| 8 byte alignment relay |           Works             |           Works            |
-----------------------------------------------------------------------------------

Note that Scenarios 1 and 3 are the same since this fix doesn't
change the behaviour of the consumer daemon.

Also note that packing the `new_chunk_id` field would break the two
working cases of scenario 3 which are, in all likelyhood, the more
common cases.

A new command using a properly packed version of the command's header
could be implemented in future versions, but this presents no benefit as
part of this fix.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3b822a9766ea5be51996a771a0d4a90efe29ce0b

Test: snapshot after regenerate metadata

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I48ae0e9d3d4e67172c4a686d42ceadf6bcb81ead

Fix: ust-consumer: segfault on snapshot after regenerate metadata

Observed issue
==============

lttng-consumerd segfaults for the following scenario:

  $ lttng create test --snapshot
  $ lttng enable-event -u -an
  $ lttng start
  # Run an app just to have some events
  $ lttng regenerate metadata
  $ lttng snapshot record

The following backtrace is obtained:

(gdb) bt
#0  __GI___pthread_mutex_lock (mutex=0x130) at ../nptl/pthread_mutex_lock.c:67
#1  0x000055b383cfaed3 in lttng_ustconsumer_recv_metadata (sock=29, key=4, offset=0, len=12391, version=1, channel=0x7fe90000a760, timer=0, wait=1) at ust-consumer.c:1347
#2  0x000055b383d00197 in lttng_ustconsumer_request_metadata (ctx=0x55b3855a1e50, channel=0x7fe90000a760, timer=0, wait=1) at ust-consumer.c:3336
#3  0x000055b383cf9e76 in snapshot_metadata (metadata_channel=0x7fe90000a760, key=4, path=0x7fe911a09944 "uid/1000/64-bit", relayd_id=18446744073709551615, ctx=0x55b3855a1e50) at ust-consum
#4  0x000055b383cfbe73 in lttng_ustconsumer_recv_cmd (ctx=0x55b3855a1e50, sock=28, consumer_sockpoll=0x7fe911a0dbb0) at ust-consumer.c:1853
#5  0x000055b383ccf9b7 in lttng_consumer_recv_cmd (ctx=0x55b3855a1e50, sock=28, consumer_sockpoll=0x7fe911a0dbb0) at consumer.c:2097
#6  0x000055b383cd3bfd in consumer_thread_sessiond_poll (data=0x55b3855a1e50) at consumer.c:3284
#7  0x00007fe914c22609 in start_thread (arg=<optimized out>) at pthread_create.c:477
#8  0x00007fe914b47293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) up
#1  0x000055b383cfaed3 in lttng_ustconsumer_recv_metadata (sock=29, key=4, offset=0, len=12391, version=1, channel=0x7fe90000a760, timer=0, wait=1) at ust-consumer.c:1347
1347                    pthread_mutex_lock(&channel->metadata_stream->lock);
(gdb) print channel->metadata_stream
$1 = (struct lttng_consumer_stream *) 0x0

Note that the following scenario also leads to a similar backtrace:

  $ lttng create test --snapshot
  $ lttng enable-event -u -an
  $ lttng start
  # Run an app just to have some events with a short duration
  $ lttng regenerate metadata
  # Run a second app just to have some events and a metadata flush while
  # the metadata cache status is set as `invalidated`.
  ^ lttng-consumerd segfault on app termination.

The backtrace:

(gdb) bt
#0  __GI___pthread_mutex_lock (mutex=0x130) at ../nptl/pthread_mutex_lock.c:67
#1  0x00005612a5a13ed3 in lttng_ustconsumer_recv_metadata (sock=28, key=2, offset=0, len=12391, version=1, channel=0x7f3978005400, timer=0, wait=1) at ust-consumer.c:1347
#2  0x00005612a5a14d0a in lttng_ustconsumer_recv_cmd (ctx=0x5612a6feee50, sock=28, consumer_sockpoll=0x7f3989494bb0) at ust-consumer.c:1818
#3  0x00005612a59e89b7 in lttng_consumer_recv_cmd (ctx=0x5612a6feee50, sock=28, consumer_sockpoll=0x7f3989494bb0) at consumer.c:2097
#4  0x00005612a59ecbfd in consumer_thread_sessiond_poll (data=0x5612a6feee50) at consumer.c:3284
#5  0x00007f398c6a9609 in start_thread (arg=<optimized out>) at pthread_create.c:477
#6  0x00007f398c5ce293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) up
#1  0x00005612a5a13ed3 in lttng_ustconsumer_recv_metadata (sock=28, key=2, offset=0, len=12391, version=1, channel=0x7f3978005400, timer=0, wait=1) at ust-consumer.c:1347
1347                    pthread_mutex_lock(&channel->metadata_stream->lock);
(gdb) print channel->metadata_stream
$1 = (struct lttng_consumer_stream *) 0x0
(gdb)

Cause
=====

For session configured in snapshot mode the metadata channel
metadata_stream field is NULL except for a "short" window during the
actual snapshot record action (snapshot_metadata).

The `regenerate metadata` effectively flag the metadata cache as invalid
leading to handling the cache invalidation state
(`CONSUMER_METADATA_CACHE_WRITE_STATUS_INVALIDATED`) in
`lttng_ustconsumer_recv_metadata`. This was introduced by
b1316da1ffbd276fc8271e7a9438e683ad352781 [1].

At that point the function expects the `channel->metadata_stream` to be
non null. This is simply not true for snapshot session metadata channels.

Note that we cannot simply swap `lttng_ustconsumer_request_metadata` and
`create_ust_streams` in `snapshot_metadata` to ensure that the
`channel->metadata_stream` is non null since
`lttng_ustconsumer_recv_metadata` ends up being called on metadata flush when
an app quit. This sequence of events corresponds to the second scenario
put forward in the `Observed Issue` section.

Solution
========

Null check `channel->metadata_stream` and perform only the operation
when it is non null. This partly mirror what is done in `consumer_metadata_wakeup_pipe`.

I am not sure if the check on `channel->monitor` is required but it
seems irrelevant to the notion of resetting the stream consumed position
when the stream exists.

With this taken care off, we find ourself with another
backtrace:

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50                                                                                                             [93/122]
#1  0x00007f75cf9b3859 in __GI_abort () at abort.c:79
#2  0x00007f75cf9b3729 in __assert_fail_base (fmt=0x7f75cfb49588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x55ab004e9c68 "pthread_mutex_trylock(&stream->lock)", file=0x55ab004
#3  0x00007f75cf9c4f36 in __GI___assert_fail (assertion=0x55ab004e9c68 "pthread_mutex_trylock(&stream->lock)", file=0x55ab004e8d7a "ust-consumer.c", line=1285, function=0x55ab004eb8a0 <__PR
#4  0x000055ab004b1b9c in metadata_stream_reset_cache_consumed_position (stream=0x7f75b400a850) at ust-consumer.c:1285
#5  0x000055ab004b4fef in commit_one_metadata_packet (stream=0x7f75b400a850) at ust-consumer.c:2551
#6  0x000055ab004b5f46 in get_next_subbuffer_metadata (stream=0x7f75b400a850, subbuffer=0x7f75cc972630) at ust-consumer.c:2927
#7  0x000055ab0048b6a9 in lttng_consumer_read_subbuffer (stream=0x7f75b400a850, ctx=0x55ab01d4ee50, locked_by_caller=true) at consumer.c:3522
#8  0x000055ab004b0f66 in snapshot_metadata (metadata_channel=0x7f75b4005400, key=2, path=0x7f75cc972944 "uid/1000/64-bit", relayd_id=18446744073709551615, ctx=0x55ab01d4ee50) at ust-consum
#9  0x000055ab004b2e86 in lttng_ustconsumer_recv_cmd (ctx=0x55ab01d4ee50, sock=28, consumer_sockpoll=0x7f75cc976bb0) at ust-consumer.c:1861
#10 0x000055ab004869b7 in lttng_consumer_recv_cmd (ctx=0x55ab01d4ee50, sock=28, consumer_sockpoll=0x7f75cc976bb0) at consumer.c:2097
#11 0x000055ab0048abfd in consumer_thread_sessiond_poll (data=0x55ab01d4ee50) at consumer.c:3284
#12 0x00007f75cfb8b609 in start_thread (arg=<optimized out>) at pthread_create.c:477
#13 0x00007f75cfab0293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Which is also caused in part to the invalidation of the cache.
`metadata_stream_reset_cache_consumed_position` expect that the stream
at that point be locked. Which is not the case despite what the last argument
to `lttng_consumer_read_subbuffer` indicates. To alleviate that we hold
the lock during the call to `lttng_consumer_read_subbuffer`.

Known drawbacks
=========

None.

References
=========

[1] https://git.lttng.org/?p=lttng-tools.git;a=commit;h=b1316da1ffbd276fc8271e7a9438e683ad352781

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib09fdc989b1baae26d0f496afeabc3e7cb27228c

lttng: list valid condition / action names if missing or unknown

I think it would be helpful to the user to list the condition and action
names, when the condition or action name is missing or unrecognized.
This patch implements that, here are some examples of the result:

    $ lttng add-trigger --action notify --condition
    Error: While parsing argument #4: Missing required argument for option `--condition`
    Error: Valid condition names are:
    Error:   event-rule-matches

    $ lttng add-trigger --action notify --condition pouet
    Error: While parsing argument #5: Unknown condition name 'pouet'
    Error: Valid condition names are:
    Error:   event-rule-matches

    $ lttng add-trigger --condition event-rule-matches --action
    Error: While parsing argument #4: Missing required argument for option `--action`
    Error: Valid action names are:
    Error:   notify

    $ lttng add-trigger --condition event-rule-matches --action pouet
    Error: While parsing argument #5: Unknown action name: pouet
    Error: Valid action names are:
    Error:   notify

To achieve this, add a new optional out argument to parse_next_item, to
allow the caller to get the argpar_error object if a parsing error
happened.  Because of this, the callers must now be able to
differentiate parsing error from memory errors: in the latter case, no
argpar_error object is returned.  So, add a new
PARSE_NEXT_ITEM_STATUS_ERROR_MEMORY status, and make users of
parse_next_item handle it.

In the add-trigger command implementation, handle the "missing opt arg"
case of OPT_ACTION and OPT_CONDITION specially to print the valid names.
Handle unknown names in parse_action and parse_condition.

Add a test for an unknown action name, it seems to be missing.  Change
the error message format slightly to make it match the messages for
unknown condition names.

Change-Id: I4c13cecacb3a2ff4367e391c4aba0d05f1f28f22
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

lttng: mention argument number on unknown action / condition name

When a unrecognized condition or action name is given, the error message
does not contain the part that mentions the argument index, like
argument parsing error messages have:

    $ lttng add-trigger  --action notify  --condition pouet
    Error: Unknown condition name 'pouet'

I think it would be useful to have it, it can help when you have really
long command lines (something possible with add-trigger).  The result
is:

    $ lttng add-trigger  --action notify  --condition pouet
    Error: While parsing argument #4 (`--condition`): Unknown condition name 'pouet'

    $ lttng add-trigger  --action notify  --condition=pouet
    Error: While parsing argument #4 (`--condition=pouet`): Unknown condition name 'pouet'

    $ lttng add-trigger --condition event-rule-matches --action pouet
    Error: While parsing argument #4 (`--action`): Unknown action name: pouet

    $ lttng add-trigger --condition event-rule-matches --action=pouet
    Error: While parsing argument #4 (`--action=pouet`): Unknown action name: pouet

Change-Id: Ic1a00cffa87ea7b3569b24c7417a00d7a52555f2
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

lttng: fix argument numbers in add-trigger error messages

Argument numbers in add-trigger argument parsing error messages are
wrong.  For example:

    $ ./src/bin/lttng/lttng add-trigger --condition event-rule-matches --foo
    Error: While parsing argument #1 (`--foo`): Unknown option `--foo`

This is due to the fact that multiple separate argpar iterators are
created to parse an add-trigger command line.  Iterators are created at
the top-level, to parse the top-level arguments.  Iterators are also
created when parsing a condition or an action, to parse the arguments
specific to that condition or action.  As a result, iterators are passed
a subset of the full command line, and the argument indices in the error
messages are off.

Fix that by passing around an "argc offset", which represents by how
much what's being parsed is offset from the full command-line.  Use that
to adjust the error messages to give indices that make sense to the
user:

    $ ./src/bin/lttng/lttng add-trigger --condition event-rule-matches --foo
    Error: While parsing argument #4 (`--foo`): Unknown option `--foo`

Change-Id: I1d312593d338641d0ec10abe515b640771a1f160
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

argpar-utils: tweak unknown option error message

Add the "While parsing argument #X" context, that is used when other
errors are encountered.  Before:

    Error: Unknown option `--hello`

After:

    Error: While parsing argument #1 (`--hello`): Unknown option `--hello`

Change-Id: Ifd84a1e8b1fe0456e09ff154cc515eaa2850a7e1
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

argpar: sync with upstream - adjust to iterator API

Sync with commit 143cec42e14e ("Force usage of ARGPAR_ASSERT() condition
when NDEBUG is defined").

The main change in this sync is the API that changed from
parse-all-at-once (the `argpar_parse` function) to something based on an
iterator, where we need to call `argpar_iter_next` to obtain the next
item.  This was prototyped here (in lttng-tools), so this patch converts
the code to the API that was actually implemented in upstream argpar.

A difference between what we had and the current argpar API is that
argpar does not provide a formatted error string anymore.  It provides
an `argpar_error` object contaning all the raw information needed to
create such string.  The new `format_arg_error_v` function formats the
errors using the exact same syntax as argpar did, such that no changes
in the tests are necessary.

The new `parse_next_item` function factors out the code around calling
argpar_iter_next that would otherwise be duplicated at a few places.

These two new functions are placed into a new `argpar-utils` convenience
library.  I originally put them in the `libcommon.la` convenience
library, but that caused some parts of the code that don't do any
argument parsing (e.g. liblttng-ctl) to have to be linked against
argpar.  As a separate library, we can limit that to just the `lttng`
binary.

Change-Id: I94aa90ffcd93f52b6073c4cd7caca78cfd0f2e05
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

configure: enable -Wformat=2

The -Wformat=2 diagnostic flag on GCC enables the -Wformat-nonliteral
-Wformat-security diagnostics, which are useful to catch some format
string mistakes.  -Wformat-security is also enabled by default with
Clang, meaning that there were some warnings only appearing with
Clang.

Try to enabled the -Wformat=2 flag to make things more consistent across
compilers and catch more mistakes.

The only issues are these, in tests/regression/ust/linking:

      CC       demo_builtin-demo.o
    In file included from /usr/include/stdio.h:866,
                     from /home/simark/src/lttng-tools/tests/regression/ust/linking/demo.c:9:
    /usr/include/bits/stdio2.h: In function ‘sprintf’:
    /usr/include/bits/stdio2.h:40:35: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
       40 |                                   __va_arg_pack ());
          |                                   ^~~~~~~~~~~~~

The reason this appears is that this directory uses -Wsystem-headers,
making the compiler show diagnostics in headers considered "system
headers".  Manually silence those warnings by disabling
-Wformat-nonliteral in that specific directory.

Change-Id: I4c7991e76b2f5405f3b3397348adb9134de37d41
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

common: move append_str to string-utils

Move the append_str function from filter-visitor-generate-bytecode.c to
the string-utils lib, so that it can be re-used elsewhere.

Change-Id: Ica09cb750ac7f3f2d6f3fb5fc786b683ceb6f79a
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

lttng-create(1): specify that `--shm-path` only applies to UST channels

This is a current limitation, but could change in the future.

Fixes: https://bugs.lttng.org/issues/1158
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie2b8114f35d7fd095790aae6add4653c5da169bf

Fix: sessiond: action-executor: misquoted strings in logging

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I5b37472c0b9e49116ec542c57346a78359d732d7

Refactor: action executor: fetch session based on execution context

There is little reason to fetch the session by name if at that point in
the action execution code path the id of the session is available.

Session that are in destroyed state are ignored. This simplify a bit the
overall execution flow.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I4a104e3b509c36b0ceea6e01f9dbd043ceb812b1

Tests: live kernel: no plan printed when non-root

The live kernel test does not produce a valid TAP output when
skipping due to not running the test as root.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I4a10de494260084216ddb1b8f4ee27a546c4d8ed