Fix: sessiond: ust: deadlock with per-pid buffers

[lttng-tools.git] / src / bin / lttng-sessiond / ust-app.c

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index eb0f837160edf5b533bbc822ef0e8ddd768962f4..efc82b05241bd6348160a8ab70cafacadb4a37a8 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -27,7 +27,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <urcu/compiler.h>
-#include <lttng/ust-error.h>
 #include <signal.h>
 
 #include <common/common.h>
@@ -38,7 +37,8 @@
 #include "health-sessiond.h"
 #include "ust-app.h"
 #include "ust-consumer.h"
-#include "ust-ctl.h"
+#include "lttng-ust-ctl.h"
+#include "lttng-ust-error.h"
 #include "utils.h"
 #include "session.h"
 #include "lttng-sessiond.h"
@@ -486,9 +486,11 @@ void delete_ust_app_channel(int sock, struct ust_app_channel *ua_chan,
                registry = get_session_registry(ua_chan->session);
                if (registry) {
                        ust_registry_channel_del_free(registry, ua_chan->key,
-                               true);
+                               sock >= 0);
+               }
+               if (sock >= 0) {
+                       save_per_pid_lost_discarded_counters(ua_chan);
                }
-               save_per_pid_lost_discarded_counters(ua_chan);
        }
 
        if (ua_chan->obj != NULL) {
@@ -729,6 +731,10 @@ error:
  * nullified. The session lock MUST be held unless the application is
  * in the destroy path.
  *
+ * Do not hold the registry lock while communicating with the consumerd, because
+ * doing so causes inter-process deadlocks between consumerd and sessiond with
+ * the metadata request notification.
+ *
  * Return 0 on success else a negative value.
  */
 static int close_metadata(struct ust_registry_session *registry,
@@ -736,6 +742,8 @@ static int close_metadata(struct ust_registry_session *registry,
 {
        int ret;
        struct consumer_socket *socket;
+       uint64_t metadata_key;
+       bool registry_was_already_closed;
 
        assert(registry);
        assert(consumer);
@@ -743,8 +751,19 @@ static int close_metadata(struct ust_registry_session *registry,
        rcu_read_lock();
 
        pthread_mutex_lock(&registry->lock);
+       metadata_key = registry->metadata_key;
+       registry_was_already_closed = registry->metadata_closed;
+       if (metadata_key != 0) {
+               /*
+                * Metadata closed. Even on error this means that the consumer
+                * is not responding or not found so either way a second close
+                * should NOT be emit for this registry.
+                */
+               registry->metadata_closed = 1;
+       }
+       pthread_mutex_unlock(&registry->lock);
 
-       if (!registry->metadata_key || registry->metadata_closed) {
+       if (metadata_key == 0 || registry_was_already_closed) {
                ret = 0;
                goto end;
        }
@@ -754,23 +773,15 @@ static int close_metadata(struct ust_registry_session *registry,
                        consumer);
        if (!socket) {
                ret = -1;
-               goto error;
+               goto end;
        }
 
-       ret = consumer_close_metadata(socket, registry->metadata_key);
+       ret = consumer_close_metadata(socket, metadata_key);
        if (ret < 0) {
-               goto error;
+               goto end;
        }
 
-error:
-       /*
-        * Metadata closed. Even on error this means that the consumer is not
-        * responding or not found so either way a second close should NOT be emit
-        * for this registry.
-        */
-       registry->metadata_closed = 1;
 end:
-       pthread_mutex_unlock(&registry->lock);
        rcu_read_unlock();
        return ret;
 }
@@ -2917,15 +2928,6 @@ static int create_channel_per_uid(struct ust_app *app,
                created = true;
        }
 
-       /* Send buffers to the application. */
-       ret = send_channel_uid_to_ust(reg_chan, app, ua_sess, ua_chan);
-       if (ret < 0) {
-               if (ret != -ENOTCONN) {
-                       ERR("Error sending channel to application");
-               }
-               goto error;
-       }
-
        if (created) {
                enum lttng_error_code cmd_ret;
                struct ltt_session *session;
@@ -2961,6 +2963,15 @@ static int create_channel_per_uid(struct ust_app *app,
                }
        }
 
+       /* Send buffers to the application. */
+       ret = send_channel_uid_to_ust(reg_chan, app, ua_sess, ua_chan);
+       if (ret < 0) {
+               if (ret != -ENOTCONN) {
+                       ERR("Error sending channel to application");
+               }
+               goto error;
+       }
+
 error:
        return ret;
 }
@@ -3010,7 +3021,7 @@ static int create_channel_per_pid(struct ust_app *app,
        if (ret < 0) {
                ERR("Error creating UST channel \"%s\" on the consumer daemon",
                        ua_chan->name);
-               goto error;
+               goto error_remove_from_registry;
        }
 
        ret = send_channel_pid_to_ust(app, ua_sess, ua_chan);
@@ -3018,7 +3029,7 @@ static int create_channel_per_pid(struct ust_app *app,
                if (ret != -ENOTCONN) {
                        ERR("Error sending channel to application");
                }
-               goto error;
+               goto error_remove_from_registry;
        }
 
        session = session_find_by_id(ua_sess->tracing_id);
@@ -3041,9 +3052,13 @@ static int create_channel_per_pid(struct ust_app *app,
        if (cmd_ret != LTTNG_OK) {
                ret = - (int) cmd_ret;
                ERR("Failed to add channel to notification thread");
-               goto error;
+               goto error_remove_from_registry;
        }
 
+error_remove_from_registry:
+       if (ret) {
+               ust_registry_channel_del_free(registry, ua_chan->key, false);
+       }
 error:
        rcu_read_unlock();
        return ret;
@@ -5394,7 +5409,7 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
                size_t nr_fields, struct ustctl_field *fields)
 {
        int ret, ret_code = 0;
-       uint32_t chan_id, reg_count;
+       uint32_t chan_id;
        uint64_t chan_reg_key;
        enum ustctl_channel_header type;
        struct ust_app *app;
@@ -5446,13 +5461,12 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
        assert(chan_reg);
 
        if (!chan_reg->register_done) {
-               reg_count = ust_registry_get_event_count(chan_reg);
-               if (reg_count < 31) {
-                       type = USTCTL_CHANNEL_HEADER_COMPACT;
-               } else {
-                       type = USTCTL_CHANNEL_HEADER_LARGE;
-               }
-
+               /*
+                * TODO: eventually use the registry event count for
+                * this channel to better guess header type for per-pid
+                * buffers.
+                */
+               type = USTCTL_CHANNEL_HEADER_LARGE;
                chan_reg->nr_ctx_fields = nr_fields;
                chan_reg->ctx_fields = fields;
                fields = NULL;
@@ -5932,6 +5946,11 @@ int ust_app_snapshot_record(struct ltt_ust_session *usess,
                        struct buffer_reg_channel *reg_chan;
                        struct consumer_socket *socket;
 
+                       if (!reg->registry->reg.ust->metadata_key) {
+                               /* Skip since no metadata is present */
+                               continue;
+                       }
+
                        /* Get consumer socket to use to push the metadata.*/
                        socket = consumer_find_socket_by_bitness(reg->bits_per_long,
                                        usess->consumer);