Fix: action executor: deadlock on session_list_lock

[lttng-tools.git] / src / bin / lttng-sessiond / action-executor.c

diff --git a/src/bin/lttng-sessiond/action-executor.c b/src/bin/lttng-sessiond/action-executor.c
index cbcd08b5f84c0047cf14ae6b601a8b158a574c25..a259933521517c358bd2ab2aa9be32b53c1f8756 100644 (file)
--- a/src/bin/lttng-sessiond/action-executor.c
+++ b/src/bin/lttng-sessiond/action-executor.c
@@ -16,8 +16,8 @@
 #include <common/macros.h>
 #include <common/optional.h>
 #include <lttng/action/action-internal.h>
-#include <lttng/action/group-internal.h>
-#include <lttng/action/group.h>
+#include <lttng/action/list-internal.h>
+#include <lttng/action/list.h>
 #include <lttng/action/notify-internal.h>
 #include <lttng/action/notify.h>
 #include <lttng/action/rotate-session.h>
@@ -135,7 +135,7 @@ static int action_executor_snapshot_session_handler(
                struct action_executor *executor,
                const struct action_work_item *,
                struct action_work_subitem *);
-static int action_executor_group_handler(struct action_executor *executor,
+static int action_executor_list_handler(struct action_executor *executor,
                const struct action_work_item *,
                struct action_work_subitem *);
 static int action_executor_generic_handler(struct action_executor *executor,
@@ -148,7 +148,7 @@ static const action_executor_handler action_executors[] = {
        [LTTNG_ACTION_TYPE_STOP_SESSION] = action_executor_stop_session_handler,
        [LTTNG_ACTION_TYPE_ROTATE_SESSION] = action_executor_rotate_session_handler,
        [LTTNG_ACTION_TYPE_SNAPSHOT_SESSION] = action_executor_snapshot_session_handler,
-       [LTTNG_ACTION_TYPE_GROUP] = action_executor_group_handler,
+       [LTTNG_ACTION_TYPE_LIST] = action_executor_list_handler,
 };
 
 /* Forward declaration */
@@ -206,7 +206,16 @@ static const char *get_trigger_name(const struct lttng_trigger *trigger)
        enum lttng_trigger_status trigger_status;
 
        trigger_status = lttng_trigger_get_name(trigger, &trigger_name);
-       assert(trigger_status == LTTNG_TRIGGER_STATUS_OK);
+       switch (trigger_status) {
+       case LTTNG_TRIGGER_STATUS_OK:
+               break;
+       case LTTNG_TRIGGER_STATUS_UNSET:
+               trigger_name = "(anonymous)";
+               break;
+       default:
+               trigger_name = "(failed to get name)";
+               break;
+       }
 
        return trigger_name;
 }
@@ -651,11 +660,11 @@ end:
        return ret;
 }
 
-static int action_executor_group_handler(struct action_executor *executor,
+static int action_executor_list_handler(struct action_executor *executor,
                const struct action_work_item *work_item,
                struct action_work_subitem *item)
 {
-       ERR("Execution of a group action by the action executor should never occur");
+       ERR("Execution of a list action by the action executor should never occur");
        abort();
 }
 
@@ -772,17 +781,7 @@ static void *action_executor_thread(void *_data)
                        uid_t trigger_owner_uid;
                        enum lttng_trigger_status trigger_status;
 
-                       trigger_status = lttng_trigger_get_name(
-                                       work_item->trigger, &trigger_name);
-                       switch (trigger_status) {
-                       case LTTNG_TRIGGER_STATUS_OK:
-                               break;
-                       case LTTNG_TRIGGER_STATUS_UNSET:
-                               trigger_name = "(unset)";
-                               break;
-                       default:
-                               abort();
-                       }
+                       trigger_name = get_trigger_name(work_item->trigger);
 
                        trigger_status = lttng_trigger_get_owner_uid(
                                        work_item->trigger, &trigger_owner_uid);
@@ -1011,7 +1010,7 @@ static int add_action_to_subitem_array(struct lttng_action *action,
        assert(action);
        assert(subitems);
 
-       if (type == LTTNG_ACTION_TYPE_GROUP) {
+       if (type == LTTNG_ACTION_TYPE_LIST) {
                unsigned int count, i;
 
                status = lttng_action_list_get_count(action, &count);
@@ -1032,7 +1031,7 @@ static int add_action_to_subitem_array(struct lttng_action *action,
 
                /*
                 * Go directly to the end since there is no need to add the
-                * group action by itself to the subitems array.
+                * list action by itself to the subitems array.
                 */
                goto end;
        }
@@ -1061,7 +1060,7 @@ static int add_action_to_subitem_array(struct lttng_action *action,
                                action, &session_name);
                assert(status == LTTNG_ACTION_STATUS_OK);
                break;
-       case LTTNG_ACTION_TYPE_GROUP:
+       case LTTNG_ACTION_TYPE_LIST:
        case LTTNG_ACTION_TYPE_UNKNOWN:
                /* Fallthrough */
        default:
@@ -1077,17 +1076,26 @@ static int add_action_to_subitem_array(struct lttng_action *action,
         * simplicity and consistency.
         */
        if (session_name != NULL) {
-               struct ltt_session *session = NULL;
+               uint64_t session_id;
 
-               session_lock_list();
-               session = session_find_by_name(session_name);
-               if (session) {
+               /*
+                * Instantaneous sampling of the session id if present.
+                *
+                * This method is preferred over `sessiond_find_by_name` then
+                * fetching the session'd id since `sessiond_find_by_name`
+                * requires the session list lock to be taken.
+                *
+                * Taking the session list lock can lead to a deadlock
+                * between the action executor and the notification thread
+                * (caller of add_action_to_subitem_array). It is okay if the
+                * session state changes between the enqueuing time and the
+                * execution time. The execution context is validated at
+                * execution time.
+                */
+               if (sample_session_id_by_name(session_name, &session_id)) {
                        LTTNG_OPTIONAL_SET(&subitem.context.session_id,
-                                       session->id);
-                       session_put(session);
+                                       session_id);
                }
-
-               session_unlock_list();
        }
 
        /* Get a reference to the action. */