From d9a970b7c05cf573d5d992615a5db3605e3219dd Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 24 May 2024 02:09:04 +0000 Subject: [PATCH] sessiond: introduce ltt_session::locked_ref look-up functions MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Introduce ltt_session::locked_ref look-up functions and use them at the various sites performing session look-ups by id or name. The users of those look-up functions are refactored to make them exception-safe. The biggest change is that process_client_msg is now exception-safe. The functions are also moved inside of the ltt_session class as static methods. This namespaces them implicitly. A number of functions that expect a locked ltt_session are modified to accept an ltt_session::locked_ref, thus enforcing the locking assumptions of the session at compile time. Change-Id: If9e4f8b25d03fa8d36a5898dd421da947ec4030c Signed-off-by: Jérémie Galarneau --- src/bin/lttng-sessiond/Makefile.am | 1 + src/bin/lttng-sessiond/action-executor.cpp | 215 +++--- src/bin/lttng-sessiond/agent-thread.cpp | 5 +- src/bin/lttng-sessiond/client.cpp | 661 ++++++++---------- src/bin/lttng-sessiond/cmd.cpp | 166 +++-- src/bin/lttng-sessiond/cmd.hpp | 14 +- src/bin/lttng-sessiond/ctl-utils.hpp | 37 + src/bin/lttng-sessiond/dispatch.cpp | 5 +- src/bin/lttng-sessiond/kernel-consumer.cpp | 19 +- src/bin/lttng-sessiond/lttng-sessiond.hpp | 1 - src/bin/lttng-sessiond/main.cpp | 8 +- src/bin/lttng-sessiond/manage-kernel.cpp | 20 +- src/bin/lttng-sessiond/rotation-thread.cpp | 33 +- src/bin/lttng-sessiond/save.cpp | 36 +- src/bin/lttng-sessiond/session.cpp | 286 +++++--- src/bin/lttng-sessiond/session.hpp | 351 +++++++--- src/bin/lttng-sessiond/ust-app.cpp | 57 +- .../lttng-sessiond/ust-registry-session.cpp | 6 +- src/common/exception.cpp | 7 + src/common/exception.hpp | 18 + tests/unit/test_session.cpp | 109 +-- 21 files changed, 1147 insertions(+), 908 deletions(-) create mode 100644 src/bin/lttng-sessiond/ctl-utils.hpp diff --git a/src/bin/lttng-sessiond/Makefile.am b/src/bin/lttng-sessiond/Makefile.am index e9b6ac1f6..15a922710 100644 --- a/src/bin/lttng-sessiond/Makefile.am +++ b/src/bin/lttng-sessiond/Makefile.am @@ -57,6 +57,7 @@ liblttng_sessiond_common_la_SOURCES = utils.cpp utils.hpp \ event-notifier-error-accounting.cpp event-notifier-error-accounting.hpp \ action-executor.cpp action-executor.hpp\ trigger-error-query.cpp \ + ctl-utils.hpp \ field.hpp field.cpp \ clock-class.hpp clock-class.cpp \ event-class.hpp event-class.cpp \ diff --git a/src/bin/lttng-sessiond/action-executor.cpp b/src/bin/lttng-sessiond/action-executor.cpp index 3fbb87f43..e3eded0fa 100644 --- a/src/bin/lttng-sessiond/action-executor.cpp +++ b/src/bin/lttng-sessiond/action-executor.cpp @@ -284,10 +284,8 @@ static int action_executor_start_session_handler(struct action_executor *executo const struct action_work_item *work_item, struct action_work_subitem *item) { - int ret = 0; const char *session_name; enum lttng_action_status action_status; - struct ltt_session *session; enum lttng_error_code cmd_ret; struct lttng_action *action = item->action; @@ -296,8 +294,7 @@ static int action_executor_start_session_handler(struct action_executor *executo action_status = lttng_action_start_session_get_session_name(action, &session_name); if (action_status != LTTNG_ACTION_STATUS_OK) { ERR("Failed to get session name from `%s` action", get_action_name(action)); - ret = -1; - goto end; + return -1; } /* @@ -310,21 +307,33 @@ static int action_executor_start_session_handler(struct action_executor *executo get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); - goto end; + return 0; } - session_lock_list(); - session = session_find_by_id(LTTNG_OPTIONAL_GET(item->context.session_id)); - if (!session) { - DBG("Failed to find session `%s` by name while executing `%s` action of trigger `%s`", - session_name, - get_action_name(action), - get_trigger_name(work_item->trigger)); + /* + * Mind the order of the declaration of list_lock vs target_session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + ltt_session::locked_ref session; + + try { + session = ltt_session::find_locked_session( + LTTNG_OPTIONAL_GET(item->context.session_id)); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + DBG_FMT("Failed to execution trigger action: {}, action=`{}`, trigger_name=`{}`, location='{}'", + ex.what(), + session_name, + get_action_name(action), + get_trigger_name(work_item->trigger), + ex.source_location); lttng_action_increase_execution_failure_count(action); - goto error_unlock_list; + return 0; } - session_lock(session); if (session->destroyed) { DBG("Session `%s` with id = %" PRIu64 " is flagged as destroyed. Skipping: action = `%s`, trigger = `%s`", @@ -332,14 +341,14 @@ static int action_executor_start_session_handler(struct action_executor *executo session->id, get_action_name(action), get_trigger_name(work_item->trigger)); - goto error_unlock_session; + return 0; } - if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_unlock_session; + if (!is_trigger_allowed_for_session(work_item->trigger, session.get())) { + return 0; } - cmd_ret = (lttng_error_code) cmd_start_trace(session); + cmd_ret = (lttng_error_code) cmd_start_trace(session.get()); switch (cmd_ret) { case LTTNG_OK: DBG("Successfully started session `%s` on behalf of trigger `%s`", @@ -360,13 +369,7 @@ static int action_executor_start_session_handler(struct action_executor *executo break; } -error_unlock_session: - session_unlock(session); - session_put(session); -error_unlock_list: - session_unlock_list(); -end: - return ret; + return 0; } static int action_executor_stop_session_handler(struct action_executor *executor @@ -374,10 +377,8 @@ static int action_executor_stop_session_handler(struct action_executor *executor const struct action_work_item *work_item, struct action_work_subitem *item) { - int ret = 0; const char *session_name; enum lttng_action_status action_status; - struct ltt_session *session; enum lttng_error_code cmd_ret; struct lttng_action *action = item->action; @@ -386,8 +387,7 @@ static int action_executor_stop_session_handler(struct action_executor *executor action_status = lttng_action_stop_session_get_session_name(action, &session_name); if (action_status != LTTNG_ACTION_STATUS_OK) { ERR("Failed to get session name from `%s` action", get_action_name(action)); - ret = -1; - goto end; + return -1; } /* @@ -400,21 +400,33 @@ static int action_executor_stop_session_handler(struct action_executor *executor get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); - goto end; + return 0; } - session_lock_list(); - session = session_find_by_id(LTTNG_OPTIONAL_GET(item->context.session_id)); - if (!session) { - DBG("Failed to find session `%s` by name while executing `%s` action of trigger `%s`", - session_name, - get_action_name(action), - get_trigger_name(work_item->trigger)); + /* + * Mind the order of the declaration of list_lock vs target_session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + ltt_session::locked_ref session; + + try { + session = ltt_session::find_locked_session( + LTTNG_OPTIONAL_GET(item->context.session_id)); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + DBG_FMT("Failed to execution trigger action: {}, action=`{}`, trigger_name=`{}`, location='{}'", + ex.what(), + session_name, + get_action_name(action), + get_trigger_name(work_item->trigger), + ex.source_location); lttng_action_increase_execution_failure_count(action); - goto error_unlock_list; + return 0; } - session_lock(session); if (session->destroyed) { DBG("Session `%s` with id = %" PRIu64 " is flagged as destroyed. Skipping: action = `%s`, trigger = `%s`", @@ -422,14 +434,14 @@ static int action_executor_stop_session_handler(struct action_executor *executor session->id, get_action_name(action), get_trigger_name(work_item->trigger)); - goto error_unlock_session; + return 0; } - if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_unlock_session; + if (!is_trigger_allowed_for_session(work_item->trigger, session.get())) { + return 0; } - cmd_ret = (lttng_error_code) cmd_stop_trace(session); + cmd_ret = (lttng_error_code) cmd_stop_trace(session.get()); switch (cmd_ret) { case LTTNG_OK: DBG("Successfully stopped session `%s` on behalf of trigger `%s`", @@ -450,13 +462,7 @@ static int action_executor_stop_session_handler(struct action_executor *executor break; } -error_unlock_session: - session_unlock(session); - session_put(session); -error_unlock_list: - session_unlock_list(); -end: - return ret; + return 0; } static int action_executor_rotate_session_handler(struct action_executor *executor @@ -464,10 +470,8 @@ static int action_executor_rotate_session_handler(struct action_executor *execut const struct action_work_item *work_item, struct action_work_subitem *item) { - int ret = 0; const char *session_name; enum lttng_action_status action_status; - struct ltt_session *session; enum lttng_error_code cmd_ret; struct lttng_action *action = item->action; @@ -476,8 +480,7 @@ static int action_executor_rotate_session_handler(struct action_executor *execut action_status = lttng_action_rotate_session_get_session_name(action, &session_name); if (action_status != LTTNG_ACTION_STATUS_OK) { ERR("Failed to get session name from `%s` action", get_action_name(action)); - ret = -1; - goto end; + return -1; } /* @@ -490,21 +493,33 @@ static int action_executor_rotate_session_handler(struct action_executor *execut get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); - goto end; + return 0; } - session_lock_list(); - session = session_find_by_id(LTTNG_OPTIONAL_GET(item->context.session_id)); - if (!session) { - DBG("Failed to find session `%s` by name while executing `%s` action of trigger `%s`", - session_name, - get_action_name(action), - get_trigger_name(work_item->trigger)); + /* + * Mind the order of the declaration of list_lock vs target_session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + ltt_session::locked_ref session; + + try { + session = ltt_session::find_locked_session( + LTTNG_OPTIONAL_GET(item->context.session_id)); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + DBG_FMT("Failed to execution trigger action: {}, action=`{}`, trigger_name=`{}`, location='{}'", + ex.what(), + session_name, + get_action_name(action), + get_trigger_name(work_item->trigger), + ex.source_location); lttng_action_increase_execution_failure_count(action); - goto error_unlock_list; + return 0; } - session_lock(session); if (session->destroyed) { DBG("Session `%s` with id = %" PRIu64 " is flagged as destroyed. Skipping: action = `%s`, trigger = `%s`", @@ -512,15 +527,15 @@ static int action_executor_rotate_session_handler(struct action_executor *execut session->id, get_action_name(action), get_trigger_name(work_item->trigger)); - goto error_unlock_session; + return 0; } - if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_unlock_session; + if (!is_trigger_allowed_for_session(work_item->trigger, session.get())) { + return 0; } cmd_ret = (lttng_error_code) cmd_rotate_session( - session, nullptr, false, LTTNG_TRACE_CHUNK_COMMAND_TYPE_MOVE_TO_COMPLETED); + session.get(), nullptr, false, LTTNG_TRACE_CHUNK_COMMAND_TYPE_MOVE_TO_COMPLETED); switch (cmd_ret) { case LTTNG_OK: DBG("Successfully started rotation of session `%s` on behalf of trigger `%s`", @@ -548,13 +563,7 @@ static int action_executor_rotate_session_handler(struct action_executor *execut break; } -error_unlock_session: - session_unlock(session); - session_put(session); -error_unlock_list: - session_unlock_list(); -end: - return ret; + return 0; } static int action_executor_snapshot_session_handler(struct action_executor *executor @@ -562,10 +571,8 @@ static int action_executor_snapshot_session_handler(struct action_executor *exec const struct action_work_item *work_item, struct action_work_subitem *item) { - int ret = 0; const char *session_name; enum lttng_action_status action_status; - struct ltt_session *session; lttng_snapshot_output default_snapshot_output; const struct lttng_snapshot_output *snapshot_output = &default_snapshot_output; enum lttng_error_code cmd_ret; @@ -584,35 +591,45 @@ static int action_executor_snapshot_session_handler(struct action_executor *exec get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); - goto end; + return 0; } action_status = lttng_action_snapshot_session_get_session_name(action, &session_name); if (action_status != LTTNG_ACTION_STATUS_OK) { ERR("Failed to get session name from `%s` action", get_action_name(action)); - ret = -1; - goto end; + return -1; } action_status = lttng_action_snapshot_session_get_output(action, &snapshot_output); if (action_status != LTTNG_ACTION_STATUS_OK && action_status != LTTNG_ACTION_STATUS_UNSET) { ERR("Failed to get output from `%s` action", get_action_name(action)); - ret = -1; - goto end; + return -1; } - session_lock_list(); - session = session_find_by_id(LTTNG_OPTIONAL_GET(item->context.session_id)); - if (!session) { - DBG("Failed to find session `%s` by name while executing `%s` action of trigger `%s`", - session_name, - get_action_name(action), - get_trigger_name(work_item->trigger)); + /* + * Mind the order of the declaration of list_lock vs session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + ltt_session::locked_ref session; + + try { + session = ltt_session::find_locked_session( + LTTNG_OPTIONAL_GET(item->context.session_id)); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + DBG_FMT("Failed to execution trigger action: {}, action=`{}`, trigger_name=`{}`, location='{}'", + ex.what(), + session_name, + get_action_name(action), + get_trigger_name(work_item->trigger), + ex.source_location); lttng_action_increase_execution_failure_count(action); - goto error_unlock_list; + return 0; } - session_lock(session); if (session->destroyed) { DBG("Session `%s` with id = %" PRIu64 " is flagged as destroyed. Skipping: action = `%s`, trigger = `%s`", @@ -620,14 +637,14 @@ static int action_executor_snapshot_session_handler(struct action_executor *exec session->id, get_action_name(action), get_trigger_name(work_item->trigger)); - goto error_unlock_session; + return 0; } - if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_unlock_session; + if (!is_trigger_allowed_for_session(work_item->trigger, session.get())) { + return 0; } - cmd_ret = (lttng_error_code) cmd_snapshot_record(session, snapshot_output, 0); + cmd_ret = (lttng_error_code) cmd_snapshot_record(session.get(), snapshot_output, 0); switch (cmd_ret) { case LTTNG_OK: DBG("Successfully recorded snapshot of session `%s` on behalf of trigger `%s`", @@ -643,13 +660,7 @@ static int action_executor_snapshot_session_handler(struct action_executor *exec break; } -error_unlock_session: - session_unlock(session); - session_put(session); -error_unlock_list: - session_unlock_list(); -end: - return ret; + return 0; } static int action_executor_list_handler(struct action_executor *executor __attribute__((unused)), diff --git a/src/bin/lttng-sessiond/agent-thread.cpp b/src/bin/lttng-sessiond/agent-thread.cpp index 23a26a806..036b3dc60 100644 --- a/src/bin/lttng-sessiond/agent-thread.cpp +++ b/src/bin/lttng-sessiond/agent-thread.cpp @@ -486,7 +486,7 @@ static void *thread_agent_management(void *data) * the agent application's configuration is * updated. */ - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); /* * Update the newly registered applications's @@ -500,7 +500,6 @@ static void *thread_agent_management(void *data) /* Removing from the poll set. */ ret = lttng_poll_del(&events, new_app_socket_fd); if (ret < 0) { - session_unlock_list(); goto error; } continue; @@ -508,8 +507,6 @@ static void *thread_agent_management(void *data) /* Publish the new agent app. */ agent_add_app(new_app); - - session_unlock_list(); } else if (revents & (LPOLLERR | LPOLLHUP | LPOLLRDHUP)) { /* Removing from the poll set */ ret = lttng_poll_del(&events, pollfd); diff --git a/src/bin/lttng-sessiond/client.cpp b/src/bin/lttng-sessiond/client.cpp index 5f9a1d0d7..fa368a90d 100644 --- a/src/bin/lttng-sessiond/client.cpp +++ b/src/bin/lttng-sessiond/client.cpp @@ -28,6 +28,8 @@ #include #include #include +#include +#include #include #include #include @@ -48,6 +50,8 @@ #include #include +namespace ls = lttng::sessiond; + namespace { bool is_root; @@ -82,18 +86,15 @@ static bool wait_thread_status() * Setup the outgoing data buffer for the response (llm) by allocating the * right amount of memory and copying the original information from the lsm * structure. - * - * Return 0 on success, negative value on error. */ -static int setup_lttng_msg(struct command_ctx *cmd_ctx, - const void *payload_buf, - size_t payload_len, - const void *cmd_header_buf, - size_t cmd_header_len) +static void setup_lttng_msg(struct command_ctx *cmd_ctx, + const void *payload_buf, + size_t payload_len, + const void *cmd_header_buf, + size_t cmd_header_len) { - int ret = 0; - const size_t header_len = sizeof(struct lttcomm_lttng_msg); - const size_t total_msg_size = header_len + cmd_header_len + payload_len; + const auto header_len = sizeof(struct lttcomm_lttng_msg); + const auto total_msg_size = header_len + cmd_header_len + payload_len; lttcomm_lttng_msg llm{}; llm.cmd_type = cmd_ctx->lsm.cmd_type; @@ -101,62 +102,53 @@ static int setup_lttng_msg(struct command_ctx *cmd_ctx, llm.cmd_header_size = (uint32_t) cmd_header_len; llm.data_size = (uint32_t) payload_len; - ret = lttng_dynamic_buffer_set_size(&cmd_ctx->reply_payload.buffer, 0); - if (ret) { - goto end; - } + const auto zero_ret = lttng_dynamic_buffer_set_size(&cmd_ctx->reply_payload.buffer, 0); + LTTNG_ASSERT(zero_ret == 0); lttng_dynamic_pointer_array_clear(&cmd_ctx->reply_payload._fd_handles); cmd_ctx->lttng_msg_size = total_msg_size; /* Append reply header. */ - ret = lttng_dynamic_buffer_append(&cmd_ctx->reply_payload.buffer, &llm, sizeof(llm)); - if (ret) { - goto end; + if (lttng_dynamic_buffer_append(&cmd_ctx->reply_payload.buffer, &llm, sizeof(llm))) { + LTTNG_THROW_ALLOCATION_FAILURE_ERROR( + "Failed to append the reply header to a client reply", sizeof(llm)); } /* Append command header. */ if (cmd_header_len) { - ret = lttng_dynamic_buffer_append( - &cmd_ctx->reply_payload.buffer, cmd_header_buf, cmd_header_len); - if (ret) { - goto end; + if (lttng_dynamic_buffer_append( + &cmd_ctx->reply_payload.buffer, cmd_header_buf, cmd_header_len)) { + LTTNG_THROW_ALLOCATION_FAILURE_ERROR( + "Failed to append the command header to a client reply", + cmd_header_len); } } /* Append payload. */ if (payload_len) { - ret = lttng_dynamic_buffer_append( - &cmd_ctx->reply_payload.buffer, payload_buf, payload_len); - if (ret) { - goto end; + if (lttng_dynamic_buffer_append( + &cmd_ctx->reply_payload.buffer, payload_buf, payload_len)) { + LTTNG_THROW_ALLOCATION_FAILURE_ERROR( + "Failed to append the payload to a client reply", payload_len); } } - -end: - return ret; } -static int setup_empty_lttng_msg(struct command_ctx *cmd_ctx) +static void setup_empty_lttng_msg(struct command_ctx *cmd_ctx) { - int ret; const struct lttcomm_lttng_msg llm = {}; - ret = lttng_dynamic_buffer_set_size(&cmd_ctx->reply_payload.buffer, 0); - if (ret) { - goto end; - } + const auto zero_ret = lttng_dynamic_buffer_set_size(&cmd_ctx->reply_payload.buffer, 0); + LTTNG_ASSERT(zero_ret == 0); /* Append place-holder reply header. */ - ret = lttng_dynamic_buffer_append(&cmd_ctx->reply_payload.buffer, &llm, sizeof(llm)); - if (ret) { - goto end; + if (lttng_dynamic_buffer_append(&cmd_ctx->reply_payload.buffer, &llm, sizeof(llm))) { + LTTNG_THROW_ALLOCATION_FAILURE_ERROR( + "Failed to append the reply header to a client reply", sizeof(llm)); } cmd_ctx->lttng_msg_size = sizeof(llm); -end: - return ret; } static void update_lttng_msg(struct command_ctx *cmd_ctx, size_t cmd_header_len, size_t payload_len) @@ -609,32 +601,29 @@ static unsigned int lttng_sessions_count(uid_t uid, gid_t gid __attribute__((unu return i; } -static enum lttng_error_code receive_lttng_trigger(struct command_ctx *cmd_ctx, - int sock, - int *sock_error, - struct lttng_trigger **_trigger) +static lttng::ctl::trigger +receive_lttng_trigger(struct command_ctx *cmd_ctx, int sock, int *sock_error) { int ret; size_t trigger_len; ssize_t sock_recv_len; - enum lttng_error_code ret_code; struct lttng_payload trigger_payload; struct lttng_trigger *trigger = nullptr; lttng_payload_init(&trigger_payload); + const auto reset_payload_on_exit = lttng::make_scope_exit( + [&trigger_payload]() noexcept { lttng_payload_reset(&trigger_payload); }); + trigger_len = (size_t) cmd_ctx->lsm.u.trigger.length; ret = lttng_dynamic_buffer_set_size(&trigger_payload.buffer, trigger_len); if (ret) { - ret_code = LTTNG_ERR_NOMEM; - goto end; + LTTNG_THROW_CTL("Failed to allocate buffer for trigger receptio", LTTNG_ERR_NOMEM); } sock_recv_len = lttcomm_recv_unix_sock(sock, trigger_payload.buffer.data, trigger_len); if (sock_recv_len < 0 || sock_recv_len != trigger_len) { - ERR("Failed to receive trigger in command payload"); *sock_error = 1; - ret_code = LTTNG_ERR_INVALID_PROTOCOL; - goto end; + LTTNG_THROW_PROTOCOL_ERROR("Failed to receive trigger in command payload"); } /* Receive fds, if any. */ @@ -642,19 +631,17 @@ static enum lttng_error_code receive_lttng_trigger(struct command_ctx *cmd_ctx, sock_recv_len = lttcomm_recv_payload_fds_unix_sock( sock, cmd_ctx->lsm.fd_count, &trigger_payload); if (sock_recv_len > 0 && sock_recv_len != cmd_ctx->lsm.fd_count * sizeof(int)) { - ERR("Failed to receive all file descriptors for trigger in command payload: expected fd count = %u, ret = %d", - cmd_ctx->lsm.fd_count, - (int) ret); - ret_code = LTTNG_ERR_INVALID_PROTOCOL; *sock_error = 1; - goto end; + LTTNG_THROW_PROTOCOL_ERROR(fmt::format( + "Failed to receive all file descriptors for trigger in command payload: expected_fd_count={}, ret={}", + [cmd_ctx]() { return cmd_ctx->lsm.fd_count; }(), + sock_recv_len)); } else if (sock_recv_len <= 0) { - ERR("Failed to receive file descriptors for trigger in command payload: expected fd count = %u, ret = %d", - cmd_ctx->lsm.fd_count, - (int) ret); - ret_code = LTTNG_ERR_FATAL; *sock_error = 1; - goto end; + LTTNG_THROW_PROTOCOL_ERROR(fmt::format( + "Failed to receive file descriptors for trigger in command payload: expected_fd_count={}, ret={}", + [cmd_ctx]() { return cmd_ctx->lsm.fd_count; }(), + sock_recv_len)); } } @@ -663,20 +650,20 @@ static enum lttng_error_code receive_lttng_trigger(struct command_ctx *cmd_ctx, struct lttng_payload_view view = lttng_payload_view_from_payload(&trigger_payload, 0, -1); - if (lttng_trigger_create_from_payload(&view, &trigger) != trigger_len) { - ERR("Invalid trigger received as part of command payload"); - ret_code = LTTNG_ERR_INVALID_TRIGGER; + const auto trigger_create_ret = lttng_trigger_create_from_payload(&view, &trigger); + + if (trigger_create_ret != trigger_len) { lttng_trigger_put(trigger); - goto end; + LTTNG_THROW_PROTOCOL_ERROR(fmt::format( + "Trigger of unexpected size received as part of command payload: expected_size={}, actual_size={}", + trigger_len, + trigger_create_ret)); + } else if (trigger_create_ret < 0) { + LTTNG_THROW_CTL("Failed to allocate trigger", LTTNG_ERR_NOMEM); } } - *_trigger = trigger; - ret_code = LTTNG_OK; - -end: - lttng_payload_reset(&trigger_payload); - return ret_code; + return lttng::ctl::trigger(trigger); } static enum lttng_error_code receive_lttng_error_query(struct command_ctx *cmd_ctx, @@ -924,10 +911,10 @@ end: /* * Version of setup_lttng_msg() without command header. */ -static int +static void setup_lttng_msg_no_cmd_header(struct command_ctx *cmd_ctx, void *payload_buf, size_t payload_len) { - return setup_lttng_msg(cmd_ctx, payload_buf, payload_len, nullptr, 0); + setup_lttng_msg(cmd_ctx, payload_buf, payload_len, nullptr, 0); } /* @@ -1003,15 +990,15 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ bool need_consumerd; if (!lttcomm_sessiond_command_is_valid((lttcomm_sessiond_command) cmd_ctx->lsm.cmd_type)) { - ERR("Unknown client command received: command id = %" PRIu32, - cmd_ctx->lsm.cmd_type); - ret = LTTNG_ERR_UND; - goto error; + /* The lambda is used since fmt can't bind a packed field. */ + LTTNG_THROW_CTL(fmt::format("Unknown client command: command_id={}", + [&cmd_ctx]() { return cmd_ctx->lsm.cmd_type; }()), + LTTNG_ERR_UND); } - DBG("Processing client command '%s\' (%d)", - lttcomm_sessiond_command_str((lttcomm_sessiond_command) cmd_ctx->lsm.cmd_type), - cmd_ctx->lsm.cmd_type); + DBG_FMT("Processing client command: name=`{}`, id={}", + lttcomm_sessiond_command_str((lttcomm_sessiond_command) cmd_ctx->lsm.cmd_type), + [&cmd_ctx]() { return cmd_ctx->lsm.cmd_type; }()); *sock_error = 0; @@ -1060,22 +1047,25 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ if (the_config.no_kernel && need_domain && cmd_ctx->lsm.domain.type == LTTNG_DOMAIN_KERNEL) { if (!is_root) { - ret = LTTNG_ERR_NEED_ROOT_SESSIOND; + LTTNG_THROW_CTL( + "Can't run a kernel-domain command since the session daemon is not running as root", + LTTNG_ERR_NEED_ROOT_SESSIOND); } else { - ret = LTTNG_ERR_KERN_NA; + LTTNG_THROW_CTL( + "Can't run a kernel-domain command since kernel tracing is disabled", + LTTNG_ERR_KERN_NA); } - goto error; } /* Deny register consumer if we already have a spawned consumer. */ if (cmd_ctx->lsm.cmd_type == LTTCOMM_SESSIOND_COMMAND_REGISTER_CONSUMER) { - pthread_mutex_lock(&the_kconsumer_data.pid_mutex); + const lttng::pthread::lock_guard kconsumer_lock(the_kconsumer_data.pid_mutex); + if (the_kconsumer_data.pid > 0) { - ret = LTTNG_ERR_KERN_CONSUMER_FAIL; - pthread_mutex_unlock(&the_kconsumer_data.pid_mutex); - goto error; + LTTNG_THROW_CTL( + "Can't register a consumer since a kernel-domain consumer was already launched", + LTTNG_ERR_KERN_CONSUMER_FAIL); } - pthread_mutex_unlock(&the_kconsumer_data.pid_mutex); } /* @@ -1103,13 +1093,29 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ break; default: /* Setup lttng message with no payload */ - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, nullptr, 0); - if (ret < 0) { - /* This label does not try to unlock the session */ - goto init_setup_error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, nullptr, 0); } + /* + * The list lock is only acquired when processing a command that is applied + * against a session. As such, a unique_lock that holds the list lock is move()'d to + * list_lock during the execution of those commands. The list lock is then released + * as the instance leaves this scope. + * + * Mind the order of the declaration of list_lock vs target_session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + std::unique_lock list_lock; + /* + * A locked_ref is typically "never null" (hence its name). However, due to the + * structure of this function, target_session remains null for commands that don't + * have a target session. + */ + ltt_session::locked_ref target_session; + /* Commands that DO NOT need a session. */ switch (cmd_ctx->lsm.cmd_type) { case LTTCOMM_SESSIOND_COMMAND_CREATE_SESSION_EXT: @@ -1138,15 +1144,21 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ * for now, because the per-session lock does not * handle teardown properly. */ - session_lock_list(); - cmd_ctx->session = session_find_by_name(cmd_ctx->lsm.session.name); - if (cmd_ctx->session == nullptr) { - ret = LTTNG_ERR_SESS_NOT_FOUND; - goto error; - } else { - /* Acquire lock for the session */ - session_lock(cmd_ctx->session); - } + list_lock = lttng::sessiond::lock_session_list(); + try { + target_session = + ltt_session::find_locked_session(cmd_ctx->lsm.session.name); + } catch (...) { + std::throw_with_nested(lttng::ctl::error( + fmt::format( + "Target session of command doesn't exist: command='{}'", + lttcomm_sessiond_command_str( + (lttcomm_sessiond_command) cmd_ctx->lsm.cmd_type)), + LTTNG_ERR_SESS_NOT_FOUND, + LTTNG_SOURCE_LOCATION())); + } + + LTTNG_ASSERT(target_session); break; } @@ -1161,17 +1173,16 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ case LTTCOMM_SESSIOND_COMMAND_DISABLE_EVENT: switch (cmd_ctx->lsm.domain.type) { case LTTNG_DOMAIN_KERNEL: - if (!cmd_ctx->session->kernel_session) { - ret = LTTNG_ERR_NO_CHANNEL; - goto error; + if (!target_session->kernel_session) { + return LTTNG_ERR_NO_CHANNEL; } break; case LTTNG_DOMAIN_JUL: case LTTNG_DOMAIN_LOG4J: case LTTNG_DOMAIN_PYTHON: case LTTNG_DOMAIN_UST: - if (!cmd_ctx->session->ust_session) { - ret = LTTNG_ERR_NO_CHANNEL; + if (!target_session->ust_session) { + return LTTNG_ERR_NO_CHANNEL; goto error; } break; @@ -1214,8 +1225,8 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ /* Need a session for kernel command */ if (need_tracing_session) { - if (cmd_ctx->session->kernel_session == nullptr) { - ret = create_kernel_session(cmd_ctx->session); + if (target_session->kernel_session == nullptr) { + ret = create_kernel_session(target_session.get()); if (ret != LTTNG_OK) { ret = LTTNG_ERR_KERN_SESS_FAIL; goto error; @@ -1242,7 +1253,7 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ * the consumer output of the session if exist. */ ret = consumer_create_socket(&the_kconsumer_data, - cmd_ctx->session->kernel_session->consumer); + target_session->kernel_session->consumer); if (ret < 0) { goto error; } @@ -1272,9 +1283,9 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ if (need_tracing_session) { /* Create UST session if none exist. */ - if (cmd_ctx->session->ust_session == nullptr) { + if (target_session->ust_session == nullptr) { lttng_domain domain = cmd_ctx->lsm.domain; - ret = create_ust_session(cmd_ctx->session, &domain); + ret = create_ust_session(target_session.get(), &domain); if (ret != LTTNG_OK) { goto error; } @@ -1306,7 +1317,7 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ * since it was set above and can ONLY be set in this thread. */ ret = consumer_create_socket(&the_ustconsumer64_data, - cmd_ctx->session->ust_session->consumer); + target_session->ust_session->consumer); if (ret < 0) { goto error; } @@ -1336,7 +1347,7 @@ static int process_client_msg(struct command_ctx *cmd_ctx, int *sock, int *sock_ * since it was set above and can ONLY be set in this thread. */ ret = consumer_create_socket(&the_ustconsumer32_data, - cmd_ctx->session->ust_session->consumer); + target_session->ust_session->consumer); if (ret < 0) { goto error; } @@ -1380,9 +1391,9 @@ skip_domain: * The root user can interact with all sessions. */ if (need_tracing_session) { - if (!session_access_ok(cmd_ctx->session, + if (!session_access_ok(target_session.get(), LTTNG_SOCK_GET_UID_CRED(&cmd_ctx->creds)) || - cmd_ctx->session->destroyed) { + target_session->destroyed) { ret = LTTNG_ERR_EPERM; goto error; } @@ -1392,12 +1403,12 @@ skip_domain: * Send relayd information to consumer as soon as we have a domain and a * session defined. */ - if (cmd_ctx->session && need_domain) { + if (target_session && need_domain) { /* * Setup relayd if not done yet. If the relayd information was already * sent to the consumer, this call will gracefully return. */ - ret = cmd_setup_relayd(cmd_ctx->session); + ret = cmd_setup_relayd(target_session.get()); if (ret != LTTNG_OK) { goto error; } @@ -1416,20 +1427,21 @@ skip_domain: goto error; } - ret = cmd_add_context(cmd_ctx, event_context, the_kernel_poll_pipe[1]); + ret = cmd_add_context( + cmd_ctx, target_session, event_context, the_kernel_poll_pipe[1]); lttng_event_context_destroy(event_context); break; } case LTTCOMM_SESSIOND_COMMAND_DISABLE_CHANNEL: { - ret = cmd_disable_channel(cmd_ctx->session, + ret = cmd_disable_channel(target_session.get(), cmd_ctx->lsm.domain.type, cmd_ctx->lsm.u.disable.channel_name); break; } case LTTCOMM_SESSIOND_COMMAND_ENABLE_CHANNEL: { - ret = cmd_enable_channel(cmd_ctx, *sock, the_kernel_poll_pipe[1]); + ret = cmd_enable_channel(cmd_ctx, target_session, *sock, the_kernel_poll_pipe[1]); break; } case LTTCOMM_SESSIOND_COMMAND_PROCESS_ATTR_TRACKER_ADD_INCLUDE_VALUE: @@ -1526,10 +1538,10 @@ skip_domain: if (add_value) { ret = cmd_process_attr_tracker_inclusion_set_add_value( - cmd_ctx->session, domain_type, process_attr, value); + target_session.get(), domain_type, process_attr, value); } else { ret = cmd_process_attr_tracker_inclusion_set_remove_value( - cmd_ctx->session, domain_type, process_attr, value); + target_session.get(), domain_type, process_attr, value); } process_attr_value_destroy(value); error_add_remove_tracker_value: @@ -1546,18 +1558,14 @@ skip_domain: .process_attr_tracker_get_tracking_policy.process_attr; ret = cmd_process_attr_tracker_get_tracking_policy( - cmd_ctx->session, domain_type, process_attr, &tracking_policy); + target_session.get(), domain_type, process_attr, &tracking_policy); if (ret != LTTNG_OK) { goto error; } uint32_t tracking_policy_u32 = tracking_policy; - ret = setup_lttng_msg_no_cmd_header( - cmd_ctx, &tracking_policy_u32, sizeof(uint32_t)); - if (ret < 0) { - ret = LTTNG_ERR_NOMEM; - goto error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, &tracking_policy_u32, sizeof(uint32_t)); + ret = LTTNG_OK; break; } @@ -1573,7 +1581,7 @@ skip_domain: .process_attr_tracker_set_tracking_policy.process_attr; ret = cmd_process_attr_tracker_set_tracking_policy( - cmd_ctx->session, domain_type, process_attr, tracking_policy); + target_session.get(), domain_type, process_attr, tracking_policy); if (ret != LTTNG_OK) { goto error; } @@ -1590,7 +1598,7 @@ skip_domain: cmd_ctx->lsm.u.process_attr_tracker_get_inclusion_set.process_attr; ret = cmd_process_attr_tracker_get_inclusion_set( - cmd_ctx->session, domain_type, process_attr, &values); + target_session.get(), domain_type, process_attr, &values); if (ret != LTTNG_OK) { goto error; } @@ -1601,11 +1609,7 @@ skip_domain: goto error_tracker_get_inclusion_set; } - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, reply.data, reply.size); - if (ret < 0) { - ret = LTTNG_ERR_NOMEM; - goto error_tracker_get_inclusion_set; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, reply.data, reply.size); ret = LTTNG_OK; error_tracker_get_inclusion_set: @@ -1639,12 +1643,18 @@ skip_domain: */ ret = cmd_ctx->lsm.cmd_type == LTTCOMM_SESSIOND_COMMAND_ENABLE_EVENT ? cmd_enable_event(cmd_ctx, + target_session, event, filter_expression, exclusions, bytecode, the_kernel_poll_pipe[1]) : - cmd_disable_event(cmd_ctx, event, filter_expression, bytecode, exclusions); + cmd_disable_event(cmd_ctx, + target_session, + event, + filter_expression, + bytecode, + exclusions); lttng_event_destroy(event); break; } @@ -1655,17 +1665,12 @@ skip_domain: size_t payload_size; const size_t command_header_size = sizeof(struct lttcomm_list_command_header); - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); original_payload_size = cmd_ctx->reply_payload.buffer.size; - session_lock_list(); + list_lock = lttng::sessiond::lock_session_list(); ret_code = cmd_list_tracepoints(cmd_ctx->lsm.domain.type, &cmd_ctx->reply_payload); - session_unlock_list(); if (ret_code != LTTNG_OK) { ret = (int) ret_code; goto error; @@ -1685,18 +1690,14 @@ skip_domain: size_t payload_size; const size_t command_header_size = sizeof(struct lttcomm_list_command_header); - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); original_payload_size = cmd_ctx->reply_payload.buffer.size; - session_lock_list(); + list_lock = lttng::sessiond::lock_session_list(); ret_code = cmd_list_tracepoint_fields(cmd_ctx->lsm.domain.type, &cmd_ctx->reply_payload); - session_unlock_list(); + if (ret_code != LTTNG_OK) { ret = (int) ret_code; goto error; @@ -1716,11 +1717,7 @@ skip_domain: size_t payload_size; const size_t command_header_size = sizeof(struct lttcomm_list_command_header); - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); original_payload_size = cmd_ctx->reply_payload.buffer.size; @@ -1767,7 +1764,7 @@ skip_domain: goto error; } - ret = cmd_set_consumer_uri(cmd_ctx->session, nb_uri, uris); + ret = cmd_set_consumer_uri(target_session.get(), nb_uri, uris); free(uris); if (ret != LTTNG_OK) { goto error; @@ -1782,24 +1779,24 @@ skip_domain: * enabled time or size-based rotations, we have to make sure * the kernel tracer supports it. */ - if (!cmd_ctx->session->has_been_started && cmd_ctx->session->kernel_session && - (cmd_ctx->session->rotate_timer_period || cmd_ctx->session->rotate_size) && + if (!target_session->has_been_started && target_session->kernel_session && + (target_session->rotate_timer_period || target_session->rotate_size) && !check_rotate_compatible()) { DBG("Kernel tracer version is not compatible with the rotation feature"); ret = LTTNG_ERR_ROTATION_WRONG_VERSION; goto error; } - ret = cmd_start_trace(cmd_ctx->session); + ret = cmd_start_trace(target_session.get()); break; } case LTTCOMM_SESSIOND_COMMAND_STOP_TRACE: { - ret = cmd_stop_trace(cmd_ctx->session); + ret = cmd_stop_trace(target_session.get()); break; } case LTTCOMM_SESSIOND_COMMAND_DESTROY_SESSION: { - ret = cmd_destroy_session(cmd_ctx->session, sock); + ret = cmd_destroy_session(target_session.get(), sock); break; } case LTTCOMM_SESSIOND_COMMAND_LIST_DOMAINS: @@ -1807,21 +1804,17 @@ skip_domain: ssize_t nb_dom; struct lttng_domain *domains = nullptr; - nb_dom = cmd_list_domains(cmd_ctx->session, &domains); + nb_dom = cmd_list_domains(target_session.get(), &domains); if (nb_dom < 0) { /* Return value is a negative lttng_error_code. */ ret = -nb_dom; goto error; } - ret = setup_lttng_msg_no_cmd_header( + setup_lttng_msg_no_cmd_header( cmd_ctx, domains, nb_dom * sizeof(struct lttng_domain)); free(domains); - if (ret < 0) { - goto setup_error; - } - ret = LTTNG_OK; break; } @@ -1832,16 +1825,12 @@ skip_domain: size_t payload_size; const size_t command_header_size = sizeof(struct lttcomm_list_command_header); - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); original_payload_size = cmd_ctx->reply_payload.buffer.size; ret_code = cmd_list_channels( - cmd_ctx->lsm.domain.type, cmd_ctx->session, &cmd_ctx->reply_payload); + cmd_ctx->lsm.domain.type, target_session.get(), &cmd_ctx->reply_payload); if (ret_code != LTTNG_OK) { ret = (int) ret_code; goto error; @@ -1861,16 +1850,12 @@ skip_domain: size_t payload_size; const size_t command_header_size = sizeof(struct lttcomm_list_command_header); - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); original_payload_size = cmd_ctx->reply_payload.buffer.size; ret_code = cmd_list_events(cmd_ctx->lsm.domain.type, - cmd_ctx->session, + target_session.get(), cmd_ctx->lsm.u.list.channel_name, &cmd_ctx->reply_payload); if (ret_code != LTTNG_OK) { @@ -1891,7 +1876,7 @@ skip_domain: lttng_session *sessions_payload = nullptr; size_t payload_len = 0; - session_lock_list(); + list_lock = lttng::sessiond::lock_session_list(); nr_sessions = lttng_sessions_count(LTTNG_SOCK_GET_UID_CRED(&cmd_ctx->creds), LTTNG_SOCK_GET_GID_CRED(&cmd_ctx->creds)); @@ -1900,9 +1885,9 @@ skip_domain: (sizeof(struct lttng_session_extended) * nr_sessions); sessions_payload = zmalloc(payload_len); if (!sessions_payload) { - session_unlock_list(); - ret = -ENOMEM; - goto setup_error; + LTTNG_THROW_ALLOCATION_FAILURE_ERROR( + "Failed to allocate session list reply payload", + payload_len); } cmd_list_lttng_sessions(sessions_payload, @@ -1911,15 +1896,9 @@ skip_domain: LTTNG_SOCK_GET_GID_CRED(&cmd_ctx->creds)); } - session_unlock_list(); - - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, sessions_payload, payload_len); + setup_lttng_msg_no_cmd_header(cmd_ctx, sessions_payload, payload_len); free(sessions_payload); - if (ret < 0) { - goto setup_error; - } - ret = LTTNG_OK; break; } @@ -1936,8 +1915,10 @@ skip_domain: goto error; } - ret = cmd_register_consumer( - cmd_ctx->session, cmd_ctx->lsm.domain.type, cmd_ctx->lsm.u.reg.path, cdata); + ret = cmd_register_consumer(target_session.get(), + cmd_ctx->lsm.domain.type, + cmd_ctx->lsm.u.reg.path, + cdata); break; } case LTTCOMM_SESSIOND_COMMAND_KERNEL_TRACER_STATUS: @@ -1951,10 +1932,7 @@ skip_domain: } u_status = (uint32_t) status; - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, &u_status, 4); - if (ret < 0) { - goto error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, &u_status, 4); ret = LTTNG_OK; break; @@ -1964,7 +1942,7 @@ skip_domain: int pending_ret; uint8_t pending_ret_byte; - pending_ret = cmd_data_pending(cmd_ctx->session); + pending_ret = cmd_data_pending(target_session.get()); /* * FIXME @@ -1991,11 +1969,7 @@ skip_domain: pending_ret_byte = (uint8_t) pending_ret; /* 1 byte to return whether or not data is pending */ - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, &pending_ret_byte, 1); - - if (ret < 0) { - goto setup_error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, &pending_ret_byte, 1); ret = LTTNG_OK; break; @@ -2006,16 +1980,13 @@ skip_domain: struct lttcomm_lttng_output_id reply; lttng_snapshot_output output = cmd_ctx->lsm.u.snapshot_output.output; - ret = cmd_snapshot_add_output(cmd_ctx->session, &output, &snapshot_id); + ret = cmd_snapshot_add_output(target_session.get(), &output, &snapshot_id); if (ret != LTTNG_OK) { goto error; } reply.id = snapshot_id; - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, &reply, sizeof(reply)); - if (ret < 0) { - goto setup_error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, &reply, sizeof(reply)); /* Copy output list into message payload */ ret = LTTNG_OK; @@ -2024,7 +1995,7 @@ skip_domain: case LTTCOMM_SESSIOND_COMMAND_SNAPSHOT_DEL_OUTPUT: { lttng_snapshot_output output = cmd_ctx->lsm.u.snapshot_output.output; - ret = cmd_snapshot_del_output(cmd_ctx->session, &output); + ret = cmd_snapshot_del_output(target_session.get(), &output); break; } case LTTCOMM_SESSIOND_COMMAND_SNAPSHOT_LIST_OUTPUT: @@ -2032,58 +2003,55 @@ skip_domain: ssize_t nb_output; struct lttng_snapshot_output *outputs = nullptr; - nb_output = cmd_snapshot_list_outputs(cmd_ctx->session, &outputs); + nb_output = cmd_snapshot_list_outputs(target_session.get(), &outputs); if (nb_output < 0) { ret = -nb_output; goto error; } LTTNG_ASSERT((nb_output > 0 && outputs) || nb_output == 0); - ret = setup_lttng_msg_no_cmd_header( + setup_lttng_msg_no_cmd_header( cmd_ctx, outputs, nb_output * sizeof(struct lttng_snapshot_output)); free(outputs); - if (ret < 0) { - goto setup_error; - } - ret = LTTNG_OK; break; } case LTTCOMM_SESSIOND_COMMAND_SNAPSHOT_RECORD: { lttng_snapshot_output output = cmd_ctx->lsm.u.snapshot_record.output; - ret = cmd_snapshot_record(cmd_ctx->session, &output, 0); // RFC: set to zero since - // it's ignored by - // cmd_snapshot_record + ret = cmd_snapshot_record(target_session.get(), &output, 0); // RFC: set to zero + // since it's ignored + // by + // cmd_snapshot_record break; } case LTTCOMM_SESSIOND_COMMAND_CREATE_SESSION_EXT: { struct lttng_dynamic_buffer payload; - struct lttng_session_descriptor *return_descriptor = nullptr; lttng_dynamic_buffer_init(&payload); - ret = cmd_create_session(cmd_ctx, *sock, &return_descriptor); - if (ret != LTTNG_OK) { - goto error; - } - ret = lttng_session_descriptor_serialize(return_descriptor, &payload); - if (ret) { - ERR("Failed to serialize session descriptor in reply to \"create session\" command"); - lttng_session_descriptor_destroy(return_descriptor); - ret = LTTNG_ERR_NOMEM; - goto error; - } - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, payload.data, payload.size); + lttng::ctl::session_descriptor reply_session_descriptor = [cmd_ctx, sock]() { + lttng_session_descriptor *raw_descriptor; + const auto create_ret = cmd_create_session(cmd_ctx, *sock, &raw_descriptor); + if (create_ret != LTTNG_OK) { + LTTNG_THROW_CTL("Failed to create session", create_ret); + } + + return lttng::ctl::session_descriptor(raw_descriptor); + }(); + + ret = lttng_session_descriptor_serialize(reply_session_descriptor.get(), &payload); if (ret) { - lttng_session_descriptor_destroy(return_descriptor); - ret = LTTNG_ERR_NOMEM; - goto error; + LTTNG_THROW_CTL( + "Failed to serialize session descriptor in reply to \"create session\" command", + LTTNG_ERR_NOMEM); } + + setup_lttng_msg_no_cmd_header(cmd_ctx, payload.data, payload.size); + lttng_dynamic_buffer_reset(&payload); - lttng_session_descriptor_destroy(return_descriptor); ret = LTTNG_OK; break; } @@ -2094,24 +2062,22 @@ skip_domain: } case LTTCOMM_SESSIOND_COMMAND_SET_SESSION_SHM_PATH: { - ret = cmd_set_session_shm_path(cmd_ctx->session, + ret = cmd_set_session_shm_path(target_session.get(), cmd_ctx->lsm.u.set_shm_path.shm_path); break; } case LTTCOMM_SESSIOND_COMMAND_REGENERATE_METADATA: { - ret = cmd_regenerate_metadata(cmd_ctx->session); + ret = cmd_regenerate_metadata(target_session.get()); break; } case LTTCOMM_SESSIOND_COMMAND_REGENERATE_STATEDUMP: { - ret = cmd_regenerate_statedump(cmd_ctx->session); + ret = cmd_regenerate_statedump(target_session.get()); break; } case LTTCOMM_SESSIOND_COMMAND_REGISTER_TRIGGER: { - struct lttng_trigger *payload_trigger; - struct lttng_trigger *return_trigger; size_t original_reply_payload_size; size_t reply_payload_size; const struct lttng_credentials cmd_creds = { @@ -2119,36 +2085,26 @@ skip_domain: .gid = LTTNG_OPTIONAL_INIT_VALUE(cmd_ctx->creds.gid), }; - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); - ret = receive_lttng_trigger(cmd_ctx, *sock, sock_error, &payload_trigger); + auto payload_trigger = receive_lttng_trigger(cmd_ctx, *sock, sock_error); if (ret != LTTNG_OK) { goto error; } original_reply_payload_size = cmd_ctx->reply_payload.buffer.size; - ret = cmd_register_trigger(&cmd_creds, - payload_trigger, - cmd_ctx->lsm.u.trigger.is_trigger_anonymous, - the_notification_thread_handle, - &return_trigger); - if (ret != LTTNG_OK) { - lttng_trigger_put(payload_trigger); - goto error; - } + auto return_trigger = + cmd_register_trigger(&cmd_creds, + payload_trigger.get(), + cmd_ctx->lsm.u.trigger.is_trigger_anonymous, + the_notification_thread_handle); - ret = lttng_trigger_serialize(return_trigger, &cmd_ctx->reply_payload); - lttng_trigger_put(payload_trigger); - lttng_trigger_put(return_trigger); + ret = lttng_trigger_serialize(return_trigger.get(), &cmd_ctx->reply_payload); if (ret) { - ERR("Failed to serialize trigger in reply to \"register trigger\" command"); - ret = LTTNG_ERR_NOMEM; - goto error; + LTTNG_THROW_CTL( + "Failed to serialize trigger in reply to \"register trigger\" command", + LTTNG_ERR_NOMEM); } reply_payload_size = @@ -2161,36 +2117,31 @@ skip_domain: } case LTTCOMM_SESSIOND_COMMAND_UNREGISTER_TRIGGER: { - struct lttng_trigger *payload_trigger; const struct lttng_credentials cmd_creds = { .uid = LTTNG_OPTIONAL_INIT_VALUE(cmd_ctx->creds.uid), .gid = LTTNG_OPTIONAL_INIT_VALUE(cmd_ctx->creds.gid), }; - ret = receive_lttng_trigger(cmd_ctx, *sock, sock_error, &payload_trigger); - if (ret != LTTNG_OK) { - goto error; - } + auto payload_trigger = receive_lttng_trigger(cmd_ctx, *sock, sock_error); ret = cmd_unregister_trigger( - &cmd_creds, payload_trigger, the_notification_thread_handle); - lttng_trigger_put(payload_trigger); + &cmd_creds, payload_trigger.get(), the_notification_thread_handle); break; } case LTTCOMM_SESSIOND_COMMAND_ROTATE_SESSION: { struct lttng_rotate_session_return rotate_return; - DBG("Client rotate session \"%s\"", cmd_ctx->session->name); + DBG("Client rotate session \"%s\"", target_session->name); memset(&rotate_return, 0, sizeof(rotate_return)); - if (cmd_ctx->session->kernel_session && !check_rotate_compatible()) { + if (target_session->kernel_session && !check_rotate_compatible()) { DBG("Kernel tracer version is not compatible with the rotation feature"); ret = LTTNG_ERR_ROTATION_WRONG_VERSION; goto error; } - ret = cmd_rotate_session(cmd_ctx->session, + ret = cmd_rotate_session(target_session.get(), &rotate_return, false, LTTNG_TRACE_CHUNK_COMMAND_TYPE_MOVE_TO_COMPLETED); @@ -2199,11 +2150,7 @@ skip_domain: goto error; } - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, &rotate_return, sizeof(rotate_return)); - if (ret < 0) { - ret = -ret; - goto error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, &rotate_return, sizeof(rotate_return)); ret = LTTNG_OK; break; @@ -2213,7 +2160,7 @@ skip_domain: struct lttng_rotation_get_info_return get_info_return; memset(&get_info_return, 0, sizeof(get_info_return)); - ret = cmd_rotate_get_info(cmd_ctx->session, + ret = cmd_rotate_get_info(target_session.get(), &get_info_return, cmd_ctx->lsm.u.get_rotation_info.rotation_id); if (ret < 0) { @@ -2221,12 +2168,7 @@ skip_domain: goto error; } - ret = setup_lttng_msg_no_cmd_header( - cmd_ctx, &get_info_return, sizeof(get_info_return)); - if (ret < 0) { - ret = -ret; - goto error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, &get_info_return, sizeof(get_info_return)); ret = LTTNG_OK; break; @@ -2237,7 +2179,7 @@ skip_domain: enum lttng_rotation_schedule_type schedule_type; uint64_t value; - if (cmd_ctx->session->kernel_session && !check_rotate_compatible()) { + if (target_session->kernel_session && !check_rotate_compatible()) { DBG("Kernel tracer version does not support session rotations"); ret = LTTNG_ERR_ROTATION_WRONG_VERSION; goto error; @@ -2249,7 +2191,7 @@ skip_domain: value = cmd_ctx->lsm.u.rotation_set_schedule.value; ret = cmd_rotation_set_schedule( - cmd_ctx->session, set_schedule, schedule_type, value); + target_session.get(), set_schedule, schedule_type, value); if (ret != LTTNG_OK) { goto error; } @@ -2260,23 +2202,19 @@ skip_domain: { lttng_session_list_schedules_return schedules; - schedules.periodic.set = !!cmd_ctx->session->rotate_timer_period; - schedules.periodic.value = cmd_ctx->session->rotate_timer_period; - schedules.size.set = !!cmd_ctx->session->rotate_size; - schedules.size.value = cmd_ctx->session->rotate_size; + schedules.periodic.set = !!target_session->rotate_timer_period; + schedules.periodic.value = target_session->rotate_timer_period; + schedules.size.set = !!target_session->rotate_size; + schedules.size.value = target_session->rotate_size; - ret = setup_lttng_msg_no_cmd_header(cmd_ctx, &schedules, sizeof(schedules)); - if (ret < 0) { - ret = -ret; - goto error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, &schedules, sizeof(schedules)); ret = LTTNG_OK; break; } case LTTCOMM_SESSIOND_COMMAND_CLEAR_SESSION: { - ret = cmd_clear_session(cmd_ctx->session, sock); + ret = cmd_clear_session(target_session.get(), sock); break; } case LTTCOMM_SESSIOND_COMMAND_LIST_TRIGGERS: @@ -2285,11 +2223,7 @@ skip_domain: size_t original_payload_size; size_t payload_size; - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); original_payload_size = cmd_ctx->reply_payload.buffer.size; @@ -2325,11 +2259,7 @@ skip_domain: size_t original_payload_size; size_t payload_size; - ret = setup_empty_lttng_msg(cmd_ctx); - if (ret) { - ret = LTTNG_ERR_NOMEM; - goto setup_error; - } + setup_empty_lttng_msg(cmd_ctx); original_payload_size = cmd_ctx->reply_payload.buffer.size; @@ -2370,23 +2300,10 @@ skip_domain: error: if (cmd_ctx->reply_payload.buffer.size == 0) { DBG("Missing llm header, creating one."); - if (setup_lttng_msg_no_cmd_header(cmd_ctx, nullptr, 0) < 0) { - goto setup_error; - } + setup_lttng_msg_no_cmd_header(cmd_ctx, nullptr, 0); } command_ctx_set_status_code(*cmd_ctx, static_cast(ret)); - -setup_error: - if (cmd_ctx->session) { - session_unlock(cmd_ctx->session); - session_put(cmd_ctx->session); - cmd_ctx->session = nullptr; - } - if (need_tracing_session) { - session_unlock_list(); - } -init_setup_error: LTTNG_ASSERT(!rcu_read_ongoing()); return ret; } @@ -2439,6 +2356,51 @@ static void thread_init_cleanup(void *data __attribute__((unused))) set_thread_status(false); } +// Helper function to log the source_location if the exception is derived from lttng::runtime_error +template +typename std::enable_if::value, + std::string>::type +formatted_source_location(const ExceptionType& ex) +{ + return fmt::format("{}", ex.source_location); +} + +template +typename std::enable_if::value, + std::string>::type +formatted_source_location(const ExceptionType&) +{ + return ""; +} + +template +static void log_nested_exceptions(const ExceptionType& ex, unsigned int level = 0) +{ + const auto location = formatted_source_location(ex); + + if (level == 0) { + if (location.size()) { + WARN_FMT("Client request failed: {}, location='{}'", ex.what(), location); + } else { + WARN_FMT("Client request failed: {}", ex.what()); + } + } else { + if (location.size()) { + WARN_FMT("\t{}, location='{}'", ex.what(), location); + } else { + WARN_FMT("\t{}", ex.what()); + } + } + + try { + std::rethrow_if_nested(ex); + } catch (const lttng::runtime_error& nested_ex) { + log_nested_exceptions(nested_ex, level + 1); + } catch (const std::exception& nested_ex) { + log_nested_exceptions(nested_ex, level + 1); + } +} + /* * This thread manage all clients request using the unix client socket for * communication. @@ -2515,7 +2477,6 @@ static void *thread_manage_clients(void *data) cmd_ctx.creds.uid = UINT32_MAX; cmd_ctx.creds.gid = UINT32_MAX; cmd_ctx.creds.pid = 0; - cmd_ctx.session = nullptr; lttng_payload_clear(&cmd_ctx.reply_payload); cmd_ctx.lttng_msg_size = 0; @@ -2620,48 +2581,24 @@ static void *thread_manage_clients(void *data) try { ret = process_client_msg(&cmd_ctx, &sock, &sock_error); rcu_thread_offline(); - if (ret < 0) { - if (sock >= 0) { - ret = close(sock); - if (ret) { - PERROR("close"); - } - } - sock = -1; - /* - * TODO: Inform client somehow of the fatal error. At - * this point, ret < 0 means that a zmalloc failed - * (ENOMEM). Error detected but still accept - * command, unless a socket error has been - * detected. - */ - continue; - } } catch (const std::bad_alloc& ex) { - WARN_FMT("Failed to allocate memory while handling client request: {}", - ex.what()); - - /* - * Reset the payload contents as the command may have left them in an - * inconsistent state. - */ - (void) setup_empty_lttng_msg(&cmd_ctx); - command_ctx_set_status_code(cmd_ctx, LTTNG_ERR_NOMEM); + log_nested_exceptions(ex); + ret = LTTNG_ERR_NOMEM; } catch (const lttng::ctl::error& ex) { - WARN_FMT("Client request failed: {}", ex.what()); - - (void) setup_empty_lttng_msg(&cmd_ctx); - command_ctx_set_status_code(cmd_ctx, ex.code()); + log_nested_exceptions(ex); + ret = ex.code(); } catch (const lttng::invalid_argument_error& ex) { - WARN_FMT("Client request failed: {}", ex.what()); - - (void) setup_empty_lttng_msg(&cmd_ctx); - command_ctx_set_status_code(cmd_ctx, LTTNG_ERR_INVALID); + log_nested_exceptions(ex); + ret = LTTNG_ERR_INVALID; + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + log_nested_exceptions(ex); + ret = LTTNG_ERR_SESS_NOT_FOUND; + } catch (const lttng::runtime_error& ex) { + log_nested_exceptions(ex); + ret = LTTNG_ERR_UNK; } catch (const std::exception& ex) { - WARN_FMT("Client request failed: {}", ex.what()); - - (void) setup_empty_lttng_msg(&cmd_ctx); - command_ctx_set_status_code(cmd_ctx, LTTNG_ERR_UNK); + log_nested_exceptions(ex); + ret = LTTNG_ERR_UNK; } if (ret < LTTNG_OK || ret >= LTTNG_ERR_NR) { @@ -2674,6 +2611,16 @@ static void *thread_manage_clients(void *data) ret = LTTNG_ERR_UNK; } + if (ret != LTTNG_OK) { + /* + * Reset the payload contents as the command may have left them in an + * inconsistent state. + */ + setup_empty_lttng_msg(&cmd_ctx); + } + + command_ctx_set_status_code(cmd_ctx, static_cast(ret)); + cmd_completion_handler = cmd_pop_completion_handler(); if (cmd_completion_handler) { enum lttng_error_code completion_code; diff --git a/src/bin/lttng-sessiond/cmd.cpp b/src/bin/lttng-sessiond/cmd.cpp index b1e0c0985..3f79ee2bb 100644 --- a/src/bin/lttng-sessiond/cmd.cpp +++ b/src/bin/lttng-sessiond/cmd.cpp @@ -116,7 +116,7 @@ uint64_t relayd_net_seq_idx; static struct cmd_completion_handler *current_completion_handler; static int validate_ust_event_name(const char *); -static int cmd_enable_event_internal(struct ltt_session *session, +static int cmd_enable_event_internal(ltt_session::locked_ref& session, const struct lttng_domain *domain, char *channel_name, struct lttng_event *event, @@ -124,7 +124,7 @@ static int cmd_enable_event_internal(struct ltt_session *session, struct lttng_bytecode *filter, struct lttng_event_exclusion *exclusion, int wpipe); -static enum lttng_error_code cmd_enable_channel_internal(struct ltt_session *session, +static enum lttng_error_code cmd_enable_channel_internal(ltt_session::locked_ref& session, const struct lttng_domain *domain, const struct lttng_channel *_attr, int wpipe); @@ -1281,7 +1281,7 @@ error: * * The wpipe arguments is used as a notifier for the kernel thread. */ -int cmd_enable_channel(struct command_ctx *cmd_ctx, int sock, int wpipe) +int cmd_enable_channel(command_ctx *cmd_ctx, ltt_session::locked_ref& session, int sock, int wpipe) { int ret; size_t channel_len; @@ -1318,7 +1318,7 @@ int cmd_enable_channel(struct command_ctx *cmd_ctx, int sock, int wpipe) goto end; } - ret = cmd_enable_channel_internal(cmd_ctx->session, &command_domain, channel, wpipe); + ret = cmd_enable_channel_internal(session, &command_domain, channel, wpipe); end: lttng_dynamic_buffer_reset(&channel_buffer); @@ -1326,18 +1326,17 @@ end: return ret; } -static enum lttng_error_code cmd_enable_channel_internal(struct ltt_session *session, +static enum lttng_error_code cmd_enable_channel_internal(ltt_session::locked_ref& session, const struct lttng_domain *domain, const struct lttng_channel *_attr, int wpipe) { enum lttng_error_code ret_code; - struct ltt_ust_session *usess = session->ust_session; + struct ltt_ust_session *usess = session.get()->ust_session; struct lttng_ht *chan_ht; size_t len; struct lttng_channel *attr = nullptr; - LTTNG_ASSERT(session); LTTNG_ASSERT(_attr); LTTNG_ASSERT(domain); @@ -1364,8 +1363,8 @@ static enum lttng_error_code cmd_enable_channel_internal(struct ltt_session *ses * live timer does the same thing but sends also synchronisation * beacons for inactive streams. */ - if (session->live_timer > 0) { - attr->attr.live_timer_interval = session->live_timer; + if (session.get()->live_timer > 0) { + attr->attr.live_timer_interval = session.get()->live_timer; attr->attr.switch_timer_interval = 0; } @@ -1379,7 +1378,7 @@ static enum lttng_error_code cmd_enable_channel_internal(struct ltt_session *ses "Setting the monitor interval timer to 0 " "(disabled) for channel '%s' of session '%s'", attr->name, - session->name); + session.get()->name); lttng_channel_set_monitor_timer_interval(attr, 0); } break; @@ -1714,14 +1713,15 @@ end: * Command LTTNG_DISABLE_EVENT processed by the client thread. */ int cmd_disable_event(struct command_ctx *cmd_ctx, + ltt_session::locked_ref& locked_session, struct lttng_event *event, char *filter_expression, struct lttng_bytecode *bytecode, struct lttng_event_exclusion *exclusion) { int ret; + ltt_session& session = *locked_session.get(); const char *event_name; - const struct ltt_session *session = cmd_ctx->session; const char *channel_name = cmd_ctx->lsm.u.disable.channel_name; const enum lttng_domain_type domain = cmd_ctx->lsm.domain.type; @@ -1757,7 +1757,7 @@ int cmd_disable_event(struct command_ctx *cmd_ctx, struct ltt_kernel_channel *kchan; struct ltt_kernel_session *ksess; - ksess = session->kernel_session; + ksess = session.kernel_session; /* * If a non-default channel has been created in the @@ -1804,7 +1804,7 @@ int cmd_disable_event(struct command_ctx *cmd_ctx, struct ltt_ust_channel *uchan; struct ltt_ust_session *usess; - usess = session->ust_session; + usess = session.ust_session; if (validate_ust_event_name(event_name)) { ret = LTTNG_ERR_INVALID_EVENT_NAME; @@ -1855,7 +1855,7 @@ int cmd_disable_event(struct command_ctx *cmd_ctx, case LTTNG_DOMAIN_PYTHON: { struct agent *agt; - struct ltt_ust_session *usess = session->ust_session; + struct ltt_ust_session *usess = session.ust_session; LTTNG_ASSERT(usess); @@ -1906,12 +1906,13 @@ error: * Command LTTNG_ADD_CONTEXT processed by the client thread. */ int cmd_add_context(struct command_ctx *cmd_ctx, + ltt_session::locked_ref& locked_session, const struct lttng_event_context *event_context, int kwpipe) { int ret, chan_kern_created = 0, chan_ust_created = 0; const enum lttng_domain_type domain = cmd_ctx->lsm.domain.type; - const struct ltt_session *session = cmd_ctx->session; + const struct ltt_session& session = *locked_session.get(); const char *channel_name = cmd_ctx->lsm.u.context.channel_name; /* @@ -1919,25 +1920,25 @@ int cmd_add_context(struct command_ctx *cmd_ctx, * some point in time before. The tracer does not allow it and would * result in a corrupted trace. */ - if (cmd_ctx->session->has_been_started) { + if (session.has_been_started) { ret = LTTNG_ERR_TRACE_ALREADY_STARTED; goto end; } switch (domain) { case LTTNG_DOMAIN_KERNEL: - LTTNG_ASSERT(session->kernel_session); + LTTNG_ASSERT(session.kernel_session); - if (session->kernel_session->channel_count == 0) { + if (session.kernel_session->channel_count == 0) { /* Create default channel */ - ret = channel_kernel_create(session->kernel_session, nullptr, kwpipe); + ret = channel_kernel_create(session.kernel_session, nullptr, kwpipe); if (ret != LTTNG_OK) { goto error; } chan_kern_created = 1; } /* Add kernel context to kernel tracer */ - ret = context_kernel_add(session->kernel_session, event_context, channel_name); + ret = context_kernel_add(session.kernel_session, event_context, channel_name); if (ret != LTTNG_OK) { goto error; } @@ -1965,7 +1966,7 @@ int cmd_add_context(struct command_ctx *cmd_ctx, /* fall through */ case LTTNG_DOMAIN_UST: { - struct ltt_ust_session *usess = session->ust_session; + struct ltt_ust_session *usess = session.ust_session; unsigned int chan_count; LTTNG_ASSERT(usess); @@ -2006,7 +2007,7 @@ int cmd_add_context(struct command_ctx *cmd_ctx, error: if (chan_kern_created) { struct ltt_kernel_channel *kchan = trace_kernel_get_channel_by_name( - DEFAULT_CHANNEL_NAME, session->kernel_session); + DEFAULT_CHANNEL_NAME, session.kernel_session); /* Created previously, this should NOT fail. */ LTTNG_ASSERT(kchan); kernel_destroy_channel(kchan); @@ -2014,11 +2015,11 @@ error: if (chan_ust_created) { struct ltt_ust_channel *uchan = trace_ust_find_channel_by_name( - session->ust_session->domain_global.channels, DEFAULT_CHANNEL_NAME); + session.ust_session->domain_global.channels, DEFAULT_CHANNEL_NAME); /* Created previously, this should NOT fail. */ LTTNG_ASSERT(uchan); /* Remove from the channel list of the session. */ - trace_ust_delete_channel(session->ust_session->domain_global.channels, uchan); + trace_ust_delete_channel(session.ust_session->domain_global.channels, uchan); trace_ust_destroy_channel(uchan); } end: @@ -2062,7 +2063,7 @@ end: * be hidden from clients. Such events are used in the agent implementation to * enable the events through which all "agent" events are funeled. */ -static int _cmd_enable_event(struct ltt_session *session, +static int _cmd_enable_event(ltt_session::locked_ref& locked_session, const struct lttng_domain *domain, char *channel_name, struct lttng_event *event, @@ -2074,8 +2075,8 @@ static int _cmd_enable_event(struct ltt_session *session, { int ret = 0, channel_created = 0; struct lttng_channel *attr = nullptr; + ltt_session& session = *locked_session.get(); - LTTNG_ASSERT(session); LTTNG_ASSERT(event); LTTNG_ASSERT(channel_name); @@ -2108,12 +2109,12 @@ static int _cmd_enable_event(struct ltt_session *session, * session, explicitely require that -c chan_name needs * to be provided. */ - if (session->kernel_session->has_non_default_channel && channel_name[0] == '\0') { + if (session.kernel_session->has_non_default_channel && channel_name[0] == '\0') { ret = LTTNG_ERR_NEED_CHANNEL_NAME; goto error; } - kchan = trace_kernel_get_channel_by_name(channel_name, session->kernel_session); + kchan = trace_kernel_get_channel_by_name(channel_name, session.kernel_session); if (kchan == nullptr) { attr = channel_new_default_attr(LTTNG_DOMAIN_KERNEL, LTTNG_BUFFER_GLOBAL); if (attr == nullptr) { @@ -2125,7 +2126,7 @@ static int _cmd_enable_event(struct ltt_session *session, goto error; } - ret = cmd_enable_channel_internal(session, domain, attr, wpipe); + ret = cmd_enable_channel_internal(locked_session, domain, attr, wpipe); if (ret != LTTNG_OK) { goto error; } @@ -2133,7 +2134,7 @@ static int _cmd_enable_event(struct ltt_session *session, } /* Get the newly created kernel channel pointer */ - kchan = trace_kernel_get_channel_by_name(channel_name, session->kernel_session); + kchan = trace_kernel_get_channel_by_name(channel_name, session.kernel_session); if (kchan == nullptr) { /* This sould not happen... */ ret = LTTNG_ERR_FATAL; @@ -2229,7 +2230,7 @@ static int _cmd_enable_event(struct ltt_session *session, case LTTNG_DOMAIN_UST: { struct ltt_ust_channel *uchan; - struct ltt_ust_session *usess = session->ust_session; + struct ltt_ust_session *usess = session.ust_session; LTTNG_ASSERT(usess); @@ -2257,7 +2258,7 @@ static int _cmd_enable_event(struct ltt_session *session, goto error; } - ret = cmd_enable_channel_internal(session, domain, attr, wpipe); + ret = cmd_enable_channel_internal(locked_session, domain, attr, wpipe); if (ret != LTTNG_OK) { goto error; } @@ -2313,7 +2314,7 @@ static int _cmd_enable_event(struct ltt_session *session, struct agent *agt; struct lttng_event uevent; struct lttng_domain tmp_dom; - struct ltt_ust_session *usess = session->ust_session; + struct ltt_ust_session *usess = session.ust_session; LTTNG_ASSERT(usess); @@ -2396,7 +2397,7 @@ static int _cmd_enable_event(struct ltt_session *session, } } - ret = cmd_enable_event_internal(session, + ret = cmd_enable_event_internal(locked_session, &tmp_dom, (char *) default_chan_name, &uevent, @@ -2447,6 +2448,7 @@ error: * We own filter, exclusion, and filter_expression. */ int cmd_enable_event(struct command_ctx *cmd_ctx, + ltt_session::locked_ref& locked_session, struct lttng_event *event, char *filter_expression, struct lttng_event_exclusion *exclusion, @@ -2467,7 +2469,7 @@ int cmd_enable_event(struct command_ctx *cmd_ctx, * - bytecode, * - exclusion */ - ret = _cmd_enable_event(cmd_ctx->session, + ret = _cmd_enable_event(locked_session, &command_domain, cmd_ctx->lsm.u.enable.channel_name, event, @@ -2487,7 +2489,7 @@ int cmd_enable_event(struct command_ctx *cmd_ctx, * never be made visible to clients and are immune to checks such as * reserved names. */ -static int cmd_enable_event_internal(struct ltt_session *session, +static int cmd_enable_event_internal(ltt_session::locked_ref& locked_session, const struct lttng_domain *domain, char *channel_name, struct lttng_event *event, @@ -2496,7 +2498,7 @@ static int cmd_enable_event_internal(struct ltt_session *session, struct lttng_event_exclusion *exclusion, int wpipe) { - return _cmd_enable_event(session, + return _cmd_enable_event(locked_session, domain, channel_name, event, @@ -3122,7 +3124,7 @@ cmd_create_session_from_descriptor(struct lttng_session_descriptor *descriptor, struct ltt_session *new_session = nullptr; enum lttng_session_descriptor_status descriptor_status; - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); if (home_path) { if (*home_path != '/') { ERR("Home path provided by client is not absolute"); @@ -3223,7 +3225,7 @@ end: /* Release the global reference on error. */ session_destroy(new_session); } - session_unlock_list(); + return ret_code; } @@ -3955,8 +3957,7 @@ end: * Using the session list, filled a lttng_session array to send back to the * client for session listing. * - * The session list lock MUST be acquired before calling this function. Use - * session_lock_list() and session_unlock_list(). + * The session list lock MUST be acquired before calling this function. */ void cmd_list_lttng_sessions(struct lttng_session *sessions, size_t session_count, @@ -4470,7 +4471,7 @@ synchronize_tracer_notifier_register(struct notification_thread_handle *notifica trigger_status = lttng_trigger_get_name(trigger, &trigger_name); trigger_name = trigger_status == LTTNG_TRIGGER_STATUS_OK ? trigger_name : "(anonymous)"; - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); switch (trigger_domain) { case LTTNG_DOMAIN_KERNEL: { @@ -4490,7 +4491,7 @@ synchronize_tracer_notifier_register(struct notification_thread_handle *notifica ret_code); } - goto end_unlock_session_list; + return ret_code; } break; } @@ -4508,7 +4509,7 @@ synchronize_tracer_notifier_register(struct notification_thread_handle *notifica agt = agent_create(trigger_domain); if (!agt) { ret_code = LTTNG_ERR_NOMEM; - goto end_unlock_session_list; + return ret_code; } agent_add(agt, the_trigger_agents_ht_by_domain); @@ -4516,7 +4517,7 @@ synchronize_tracer_notifier_register(struct notification_thread_handle *notifica ret_code = (lttng_error_code) trigger_agent_enable(trigger, agt); if (ret_code != LTTNG_OK) { - goto end_unlock_session_list; + return ret_code; } break; @@ -4526,17 +4527,13 @@ synchronize_tracer_notifier_register(struct notification_thread_handle *notifica abort(); } - ret_code = LTTNG_OK; -end_unlock_session_list: - session_unlock_list(); - return ret_code; + return LTTNG_OK; } -enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_creds, - struct lttng_trigger *trigger, - bool is_trigger_anonymous, - struct notification_thread_handle *notification_thread, - struct lttng_trigger **return_trigger) +lttng::ctl::trigger cmd_register_trigger(const struct lttng_credentials *cmd_creds, + struct lttng_trigger *trigger, + bool is_trigger_anonymous, + struct notification_thread_handle *notification_thread) { enum lttng_error_code ret_code; const char *trigger_name; @@ -4561,12 +4558,13 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c */ if (!lttng_credentials_is_equal_uid(lttng_trigger_get_credentials(trigger), cmd_creds)) { if (lttng_credentials_get_uid(cmd_creds) != 0) { - ERR("Trigger credentials do not match the command credentials: trigger name = '%s', trigger owner uid = %d, command creds uid = %d", - trigger_name, - (int) trigger_owner, - (int) lttng_credentials_get_uid(cmd_creds)); - ret_code = LTTNG_ERR_INVALID_TRIGGER; - goto end; + LTTNG_THROW_CTL( + fmt::format( + "Trigger credentials do not match the command credentials: trigger_name = `{}`, trigger_owner_uid={}, command_creds_uid={}", + trigger_name, + trigger_owner, + lttng_credentials_get_uid(cmd_creds)), + LTTNG_ERR_INVALID_TRIGGER); } } @@ -4576,11 +4574,12 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c */ ret_code = lttng_trigger_generate_bytecode(trigger, cmd_creds); if (ret_code != LTTNG_OK) { - ERR("Failed to generate bytecode of trigger: trigger name = '%s', trigger owner uid = %d, error code = %d", - trigger_name, - (int) trigger_owner, - ret_code); - goto end; + LTTNG_THROW_CTL( + fmt::format( + "Failed to generate bytecode of trigger: trigger_name=`{}`, trigger_owner_uid={}", + trigger_name, + trigger_owner), + ret_code); } /* @@ -4596,11 +4595,12 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c ret_code = notification_thread_command_register_trigger( notification_thread, trigger, is_trigger_anonymous); if (ret_code != LTTNG_OK) { - DBG("Failed to register trigger to notification thread: trigger name = '%s', trigger owner uid = %d, error code = %d", - trigger_name, - (int) trigger_owner, - ret_code); - goto end; + LTTNG_THROW_CTL( + fmt::format( + "Failed to register trigger to notification thread: trigger_name=`{}`, trigger_owner_uid={}", + trigger_name, + trigger_owner), + ret_code); } trigger_status = lttng_trigger_get_name(trigger, &trigger_name); @@ -4613,8 +4613,7 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c ret_code = synchronize_tracer_notifier_register( notification_thread, trigger, cmd_creds); if (ret_code != LTTNG_OK) { - ERR("Error registering tracer notifier: %s", lttng_strerror(-ret_code)); - goto end; + LTTNG_THROW_CTL("Failed to register tracer notifier", ret_code); } } @@ -4625,14 +4624,9 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c * reference to the trigger so the caller doesn't have to care if those * are distinct instances or not. */ - if (ret_code == LTTNG_OK) { - lttng_trigger_get(trigger); - *return_trigger = trigger; - /* Ownership of trigger was transferred to caller. */ - trigger = nullptr; - } -end: - return ret_code; + LTTNG_ASSERT(ret_code == LTTNG_OK); + lttng_trigger_get(trigger); + return lttng::ctl::trigger(trigger); } static enum lttng_error_code @@ -4647,12 +4641,12 @@ synchronize_tracer_notifier_unregister(const struct lttng_trigger *trigger) LTTNG_ASSERT(lttng_condition_get_type(condition) == LTTNG_CONDITION_TYPE_EVENT_RULE_MATCHES); - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); switch (trigger_domain) { case LTTNG_DOMAIN_KERNEL: ret_code = kernel_unregister_event_notifier(trigger); if (ret_code != LTTNG_OK) { - goto end_unlock_session_list; + return ret_code; } break; @@ -4673,7 +4667,7 @@ synchronize_tracer_notifier_unregister(const struct lttng_trigger *trigger) LTTNG_ASSERT(agt); ret_code = (lttng_error_code) trigger_agent_disable(trigger, agt); if (ret_code != LTTNG_OK) { - goto end_unlock_session_list; + return ret_code; } break; @@ -4683,11 +4677,7 @@ synchronize_tracer_notifier_unregister(const struct lttng_trigger *trigger) abort(); } - ret_code = LTTNG_OK; - -end_unlock_session_list: - session_unlock_list(); - return ret_code; + return LTTNG_OK; } enum lttng_error_code cmd_unregister_trigger(const struct lttng_credentials *cmd_creds, diff --git a/src/bin/lttng-sessiond/cmd.hpp b/src/bin/lttng-sessiond/cmd.hpp index a470a0881..7e5184a21 100644 --- a/src/bin/lttng-sessiond/cmd.hpp +++ b/src/bin/lttng-sessiond/cmd.hpp @@ -9,6 +9,7 @@ #define CMD_H #include "context.hpp" +#include "ctl-utils.hpp" #include "lttng-sessiond.hpp" #include "lttng/tracker.h" #include "session.hpp" @@ -50,7 +51,10 @@ int cmd_destroy_session(struct ltt_session *session, int *sock_fd); int cmd_disable_channel(struct ltt_session *session, enum lttng_domain_type domain, char *channel_name); -int cmd_enable_channel(struct command_ctx *cmd_ctx, int sock, int wpipe); +int cmd_enable_channel(struct command_ctx *cmd_ctx, + ltt_session::locked_ref& session, + int sock, + int wpipe); /* Process attribute tracker commands */ enum lttng_error_code @@ -81,11 +85,13 @@ cmd_process_attr_tracker_get_inclusion_set(struct ltt_session *session, /* Event commands */ int cmd_disable_event(struct command_ctx *cmd_ctx, + ltt_session::locked_ref& locked_session, struct lttng_event *event, char *filter_expression, struct lttng_bytecode *filter, struct lttng_event_exclusion *exclusion); int cmd_add_context(struct command_ctx *cmd_ctx, + ltt_session::locked_ref& locked_session, const struct lttng_event_context *event_context, int kwpipe); int cmd_set_filter(struct ltt_session *session, @@ -94,6 +100,7 @@ int cmd_set_filter(struct ltt_session *session, struct lttng_event *event, struct lttng_bytecode *bytecode); int cmd_enable_event(struct command_ctx *cmd_ctx, + ltt_session::locked_ref& session, struct lttng_event *event, char *filter_expression, struct lttng_event_exclusion *exclusion, @@ -150,12 +157,11 @@ int cmd_set_session_shm_path(struct ltt_session *session, const char *shm_path); int cmd_regenerate_metadata(struct ltt_session *session); int cmd_regenerate_statedump(struct ltt_session *session); -enum lttng_error_code +lttng::ctl::trigger cmd_register_trigger(const struct lttng_credentials *cmd_creds, struct lttng_trigger *trigger, bool is_anonymous_trigger, - struct notification_thread_handle *notification_thread_handle, - struct lttng_trigger **return_trigger); + struct notification_thread_handle *notification_thread_handle); enum lttng_error_code cmd_unregister_trigger(const struct lttng_credentials *cmd_creds, const struct lttng_trigger *trigger, diff --git a/src/bin/lttng-sessiond/ctl-utils.hpp b/src/bin/lttng-sessiond/ctl-utils.hpp new file mode 100644 index 000000000..13c21605a --- /dev/null +++ b/src/bin/lttng-sessiond/ctl-utils.hpp @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2024 Jérémie Galarneau + * + * SPDX-License-Identifier: GPL-2.0-only + * + */ + +#ifndef LTTNG_SESSIOND_CTL_UTILS_H +#define LTTNG_SESSIOND_CTL_UTILS_H + +#include + +#include + +namespace lttng { +namespace ctl { +/* + * The 'session_descriptor' alias, based on unique_ptr, manages lttng_session_descriptor resources + * with automatic cleanup. + */ +using session_descriptor = std::unique_ptr< + lttng_session_descriptor, + lttng::memory::create_deleter_class::deleter>; + +/* + * The 'trigger' alias, based on unique_ptr, manages lttng_trigger resources + * with automatic cleanup. + */ +using trigger = std::unique_ptr< + lttng_trigger, + lttng::memory::create_deleter_class::deleter>; + +} /* namespace ctl */ +} /* namespace lttng */ + +#endif /* LTTNG_SESSIOND_CTL_UTILS_H */ diff --git a/src/bin/lttng-sessiond/dispatch.cpp b/src/bin/lttng-sessiond/dispatch.cpp index 141d8b4d0..9d53896df 100644 --- a/src/bin/lttng-sessiond/dispatch.cpp +++ b/src/bin/lttng-sessiond/dispatch.cpp @@ -378,7 +378,7 @@ static void *thread_dispatch_ust_registration(void *data) * registration done message, no thread can see the application * and change its state. */ - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); lttng::urcu::read_lock_guard read_lock; /* @@ -397,7 +397,6 @@ static void *thread_dispatch_ust_registration(void *data) ret = send_socket_to_thread( notifiers->apps_cmd_notify_pipe_write_fd, app->notify_sock); if (ret < 0) { - session_unlock_list(); /* * No notify thread, stop the UST tracing. However, this is * not an internal error of the this thread thus setting @@ -427,7 +426,6 @@ static void *thread_dispatch_ust_registration(void *data) ret = send_socket_to_thread(notifiers->apps_cmd_pipe_write_fd, app->sock); if (ret < 0) { - session_unlock_list(); /* * No apps. thread, stop the UST tracing. However, this is * not an internal error of the this thread thus setting @@ -437,7 +435,6 @@ static void *thread_dispatch_ust_registration(void *data) goto error; } - session_unlock_list(); } } while (node != nullptr); diff --git a/src/bin/lttng-sessiond/kernel-consumer.cpp b/src/bin/lttng-sessiond/kernel-consumer.cpp index b5ccd47dd..1ea81f801 100644 --- a/src/bin/lttng-sessiond/kernel-consumer.cpp +++ b/src/bin/lttng-sessiond/kernel-consumer.cpp @@ -92,11 +92,11 @@ static int kernel_consumer_add_channel(struct consumer_socket *sock, struct lttcomm_consumer_msg lkm; struct consumer_output *consumer; enum lttng_error_code status; - struct ltt_session *session = nullptr; struct lttng_channel_extended *channel_attr_extended; bool is_local_trace; size_t consumer_path_offset = 0; lttng::urcu::read_lock_guard read_lock; + ltt_session::ref session; /* Safety net */ LTTNG_ASSERT(channel); @@ -166,9 +166,17 @@ static int kernel_consumer_add_channel(struct consumer_socket *sock, } health_code_update(); - session = session_find_by_id(ksession->id); - LTTNG_ASSERT(session); - ASSERT_LOCKED(session->lock); + + try { + session = ltt_session::find_session(ksession->id); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + ERR_FMT("Fatal error during the creation of a kernel channel: {}, location='{}'", + ex.what(), + ex.source_location); + abort(); + } + + ASSERT_LOCKED(session->_lock); ASSERT_SESSION_LIST_LOCKED(); status = notification_thread_command_add_channel(the_notification_thread_handle, @@ -186,9 +194,6 @@ static int kernel_consumer_add_channel(struct consumer_socket *sock, channel->published_to_notification_thread = true; error: - if (session) { - session_put(session); - } free(pathname); return ret; } diff --git a/src/bin/lttng-sessiond/lttng-sessiond.hpp b/src/bin/lttng-sessiond/lttng-sessiond.hpp index a10586bb8..ac455230a 100644 --- a/src/bin/lttng-sessiond/lttng-sessiond.hpp +++ b/src/bin/lttng-sessiond/lttng-sessiond.hpp @@ -77,7 +77,6 @@ extern lttng::sessiond::rotation_thread::uptr the_rotation_thread_handle; */ struct command_ctx { unsigned int lttng_msg_size; - struct ltt_session *session; /* Input message */ struct lttcomm_session_msg lsm; /* Reply content, starts with an lttcomm_lttng_msg header. */ diff --git a/src/bin/lttng-sessiond/main.cpp b/src/bin/lttng-sessiond/main.cpp index 16d0721c1..6451899cd 100644 --- a/src/bin/lttng-sessiond/main.cpp +++ b/src/bin/lttng-sessiond/main.cpp @@ -1282,14 +1282,15 @@ static void destroy_all_sessions_and_wait() struct ltt_session *session, *tmp; struct ltt_session_list *session_list; - session_list = session_get_list(); DBG("Initiating destruction of all sessions"); + auto list_lock = lttng::sessiond::lock_session_list(); + + session_list = session_get_list(); if (!session_list) { return; } - session_lock_list(); /* Initiate the destruction of all sessions. */ cds_list_for_each_entry_safe (session, tmp, &session_list->head, list) { if (!session_get(session)) { @@ -1306,11 +1307,10 @@ static void destroy_all_sessions_and_wait() session_unlock(session); session_put(session); } - session_unlock_list(); /* Wait for the destruction of all sessions to complete. */ DBG("Waiting for the destruction of all sessions to complete"); - session_list_wait_empty(); + session_list_wait_empty(std::move(list_lock)); DBG("Destruction of all sessions completed"); } diff --git a/src/bin/lttng-sessiond/manage-kernel.cpp b/src/bin/lttng-sessiond/manage-kernel.cpp index fe7f21c6f..520583cc2 100644 --- a/src/bin/lttng-sessiond/manage-kernel.cpp +++ b/src/bin/lttng-sessiond/manage-kernel.cpp @@ -37,11 +37,12 @@ static int update_kernel_poll(struct lttng_poll_event *events) int ret; struct ltt_kernel_channel *channel; struct ltt_session *session; - const struct ltt_session_list *session_list = session_get_list(); DBG("Updating kernel poll set"); - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); + const struct ltt_session_list *session_list = session_get_list(); + cds_list_for_each_entry (session, &session_list->head, list) { if (!session_get(session)) { continue; @@ -60,20 +61,15 @@ static int update_kernel_poll(struct lttng_poll_event *events) if (ret < 0) { session_unlock(session); session_put(session); - goto error; + return -1; } DBG("Channel fd %d added to kernel set", channel->fd); } session_unlock(session); session_put(session); } - session_unlock_list(); return 0; - -error: - session_unlock_list(); - return -1; } /* @@ -88,11 +84,12 @@ static int update_kernel_stream(int fd) struct ltt_session *session; struct ltt_kernel_session *ksess; struct ltt_kernel_channel *channel; - const struct ltt_session_list *session_list = session_get_list(); DBG("Updating kernel streams for channel fd %d", fd); - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); + const struct ltt_session_list *session_list = session_get_list(); + cds_list_for_each_entry (session, &session_list->head, list) { if (!session_get(session)) { continue; @@ -154,13 +151,12 @@ static int update_kernel_stream(int fd) session_unlock(session); session_put(session); } - session_unlock_list(); + return ret; error: session_unlock(session); session_put(session); - session_unlock_list(); return ret; } diff --git a/src/bin/lttng-sessiond/rotation-thread.cpp b/src/bin/lttng-sessiond/rotation-thread.cpp index 418a2d62b..ca7237802 100644 --- a/src/bin/lttng-sessiond/rotation-thread.cpp +++ b/src/bin/lttng-sessiond/rotation-thread.cpp @@ -315,7 +315,7 @@ int launch_session_rotation(ltt_session& session) DBG("Launching scheduled time-based rotation on session \"%s\"", session.name); ASSERT_SESSION_LIST_LOCKED(); - ASSERT_LOCKED(session.lock); + ASSERT_LOCKED(session._lock); ret = cmd_rotate_session(&session, &rotation_return, @@ -539,9 +539,7 @@ void ls::rotation_thread::_handle_job_queue() cds_list_del(&job->head); } - session_lock_list(); - const auto unlock_list = - lttng::make_scope_exit([]() noexcept { session_unlock_list(); }); + const auto list_lock = lttng::sessiond::lock_session_list(); /* locked_ref will unlock the session and release the ref held by the job. */ session_lock(job->session); @@ -585,19 +583,18 @@ void ls::rotation_thread::_handle_notification(const lttng_notification& notific condition_session_name, consumed); - session_lock_list(); - const auto unlock_list = lttng::make_scope_exit([]() noexcept { session_unlock_list(); }); - - ltt_session::locked_ref session{ [&condition_session_name]() { - auto raw_session_ptr = session_find_by_name(condition_session_name); - - if (raw_session_ptr) { - session_lock(raw_session_ptr); - } - - return raw_session_ptr; - }() }; - if (!session) { + /* + * Mind the order of the declaration of list_lock vs session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + ltt_session::locked_ref session; + try { + session = ltt_session::find_locked_session(condition_session_name); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { DBG_FMT("Failed to find session while handling notification: notification_type={}, session name=`{}`", lttng_condition_type_str(condition_type), condition_session_name); @@ -839,7 +836,7 @@ void ls::rotation_thread::subscribe_session_consumed_size_rotation(ltt_session& .gid = LTTNG_OPTIONAL_INIT_VALUE(session.gid), }; - ASSERT_LOCKED(session.lock); + ASSERT_LOCKED(session._lock); auto rotate_condition = lttng::make_unique_wrapper( lttng_condition_session_consumed_size_create()); diff --git a/src/bin/lttng-sessiond/save.cpp b/src/bin/lttng-sessiond/save.cpp index b4ca4c080..484bf7c90 100644 --- a/src/bin/lttng-sessiond/save.cpp +++ b/src/bin/lttng-sessiond/save.cpp @@ -2737,27 +2737,36 @@ int cmd_save_sessions(struct lttng_save_session_attr *attr, lttng_sock_cred *cre { int ret; const char *session_name; - struct ltt_session *session; - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); session_name = lttng_save_session_attr_get_session_name(attr); if (session_name) { - session = session_find_by_name(session_name); - if (!session) { + + /* + * Mind the order of the declaration of list_lock vs session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + ltt_session::locked_ref session; + + try { + session = ltt_session::find_locked_session(session_name); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + WARN_FMT("Failed to save session: {} {}", ex.what(), ex.source_location); ret = LTTNG_ERR_SESS_NOT_FOUND; - goto end; + return ret; } - session_lock(session); - ret = save_session(session, attr, creds); - session_unlock(session); - session_put(session); + ret = save_session(session.get(), attr, creds); if (ret != LTTNG_OK) { - goto end; + return ret; } } else { struct ltt_session_list *list = session_get_list(); + struct ltt_session *session; cds_list_for_each_entry (session, &list->head, list) { if (!session_get(session)) { @@ -2769,13 +2778,10 @@ int cmd_save_sessions(struct lttng_save_session_attr *attr, lttng_sock_cred *cre session_put(session); /* Don't abort if we don't have the required permissions. */ if (ret != LTTNG_OK && ret != LTTNG_ERR_EPERM) { - goto end; + return ret; } } } - ret = LTTNG_OK; -end: - session_unlock_list(); - return ret; + return LTTNG_OK; } diff --git a/src/bin/lttng-sessiond/session.cpp b/src/bin/lttng-sessiond/session.cpp index 3c9f787dd..b588e6055 100644 --- a/src/bin/lttng-sessiond/session.cpp +++ b/src/bin/lttng-sessiond/session.cpp @@ -69,6 +69,65 @@ struct lttng_ht *ltt_sessions_ht_by_name = nullptr; * Please see session.h for more explanation and correct usage of the list. */ struct ltt_session_list the_session_list; + +/* + * Return a ltt_session structure ptr that matches name. If no session found, + * NULL is returned. This must be called with the session list lock held using + * session_lock_list and session_unlock_list. + * A reference to the session is implicitly acquired by this function. + */ +struct ltt_session *session_find_by_name(const char *name) +{ + struct ltt_session *iter; + + LTTNG_ASSERT(name); + ASSERT_SESSION_LIST_LOCKED(); + + DBG2("Trying to find session by name %s", name); + + cds_list_for_each_entry (iter, &the_session_list.head, list) { + if (!strncmp(iter->name, name, NAME_MAX) && !iter->destroyed) { + goto found; + } + } + + return nullptr; +found: + return session_get(iter) ? iter : nullptr; +} + +/* + * Return an ltt_session that matches the id. If no session is found, + * NULL is returned. This must be called with rcu_read_lock and + * session list lock held (to guarantee the lifetime of the session). + */ +struct ltt_session *session_find_by_id(uint64_t id) +{ + struct lttng_ht_node_u64 *node; + struct lttng_ht_iter iter; + struct ltt_session *ls; + + ASSERT_RCU_READ_LOCKED(); + ASSERT_SESSION_LIST_LOCKED(); + + if (!ltt_sessions_ht_by_id) { + goto end; + } + + lttng_ht_lookup(ltt_sessions_ht_by_id, &id, &iter); + node = lttng_ht_iter_get_node_u64(&iter); + if (node == nullptr) { + goto end; + } + ls = lttng::utils::container_of(node, <t_session::node); + + DBG3("Session %" PRIu64 " found by id.", id); + return session_get(ls) ? ls : nullptr; + +end: + DBG3("Session %" PRIu64 " NOT found by id", id); + return nullptr; +} } /* namespace */ /* @@ -141,23 +200,13 @@ struct ltt_session_list *session_get_list() /* * Returns once the session list is empty. */ -void session_list_wait_empty() +void session_list_wait_empty(std::unique_lock list_lock) { - std::unique_lock list_lock(the_session_list.lock); - /* Keep waiting until the session list is empty. */ the_session_list.removal_cond.wait(list_lock, [] { return cds_list_empty(&the_session_list.head); }); } -/* - * Acquire session list lock - */ -void session_lock_list() noexcept -{ - the_session_list.lock.lock(); -} - /* * Try to acquire session list lock */ @@ -167,14 +216,6 @@ int session_trylock_list() noexcept return the_session_list.lock.try_lock() ? 0 : 1; } -/* - * Release session list lock - */ -void session_unlock_list() noexcept -{ - the_session_list.lock.unlock(); -} - /* * Get the session's consumer destination type. * @@ -445,8 +486,17 @@ static void del_session_ht(struct ltt_session *ls) void session_lock(struct ltt_session *session) { LTTNG_ASSERT(session); + session->lock(); +} - pthread_mutex_lock(&session->lock); +void ltt_session::lock() const noexcept +{ + pthread_mutex_lock(&_lock); +} + +void ltt_session::unlock() const noexcept +{ + ltt_session::_const_session_unlock(*this); } /* @@ -455,8 +505,12 @@ void session_lock(struct ltt_session *session) void session_unlock(struct ltt_session *session) { LTTNG_ASSERT(session); + session->unlock(); +} - pthread_mutex_unlock(&session->lock); +void ltt_session::_const_session_unlock(const ltt_session &session) +{ + pthread_mutex_unlock(&session._lock); } static int _session_set_trace_chunk_no_lock_check(struct ltt_session *session, @@ -902,7 +956,7 @@ int session_set_trace_chunk(struct ltt_session *session, struct lttng_trace_chunk *new_trace_chunk, struct lttng_trace_chunk **current_trace_chunk) { - ASSERT_LOCKED(session->lock); + ASSERT_LOCKED(session->_lock); return _session_set_trace_chunk_no_lock_check( session, new_trace_chunk, current_trace_chunk); } @@ -944,7 +998,7 @@ static void session_release(struct urcu_ref *ref) int ret; struct ltt_ust_session *usess; struct ltt_kernel_session *ksess; - struct ltt_session *session = lttng::utils::container_of(ref, <t_session::ref); + struct ltt_session *session = lttng::utils::container_of(ref, <t_session::ref_count); const bool session_published = session->published; LTTNG_ASSERT(!session->chunk_being_archived); @@ -983,7 +1037,7 @@ static void session_release(struct urcu_ref *ref) snapshot_destroy(&session->snapshot); - pthread_mutex_destroy(&session->lock); + pthread_mutex_destroy(&session->_lock); if (session_published) { ASSERT_SESSION_LIST_LOCKED(); @@ -1022,7 +1076,7 @@ static void session_release(struct urcu_ref *ref) */ bool session_get(struct ltt_session *session) { - return urcu_ref_get_unless_zero(&session->ref); + return urcu_ref_get_unless_zero(&session->ref_count); } /* @@ -1038,8 +1092,8 @@ void session_put(struct ltt_session *session) * may cause the removal of the session from the session_list. */ ASSERT_SESSION_LIST_LOCKED(); - LTTNG_ASSERT(session->ref.refcount); - urcu_ref_put(&session->ref, session_release); + LTTNG_ASSERT(session->ref_count.refcount); + urcu_ref_put(&session->ref_count, session_release); } /* @@ -1095,65 +1149,6 @@ int session_add_clear_notifier(struct ltt_session *session, return lttng_dynamic_array_add_element(&session->clear_notifiers, &element); } -/* - * Return a ltt_session structure ptr that matches name. If no session found, - * NULL is returned. This must be called with the session list lock held using - * session_lock_list and session_unlock_list. - * A reference to the session is implicitly acquired by this function. - */ -struct ltt_session *session_find_by_name(const char *name) -{ - struct ltt_session *iter; - - LTTNG_ASSERT(name); - ASSERT_SESSION_LIST_LOCKED(); - - DBG2("Trying to find session by name %s", name); - - cds_list_for_each_entry (iter, &the_session_list.head, list) { - if (!strncmp(iter->name, name, NAME_MAX) && !iter->destroyed) { - goto found; - } - } - - return nullptr; -found: - return session_get(iter) ? iter : nullptr; -} - -/* - * Return an ltt_session that matches the id. If no session is found, - * NULL is returned. This must be called with rcu_read_lock and - * session list lock held (to guarantee the lifetime of the session). - */ -struct ltt_session *session_find_by_id(uint64_t id) -{ - struct lttng_ht_node_u64 *node; - struct lttng_ht_iter iter; - struct ltt_session *ls; - - ASSERT_RCU_READ_LOCKED(); - ASSERT_SESSION_LIST_LOCKED(); - - if (!ltt_sessions_ht_by_id) { - goto end; - } - - lttng_ht_lookup(ltt_sessions_ht_by_id, &id, &iter); - node = lttng_ht_iter_get_node_u64(&iter); - if (node == nullptr) { - goto end; - } - ls = lttng::utils::container_of(node, <t_session::node); - - DBG3("Session %" PRIu64 " found by id.", id); - return session_get(ls) ? ls : nullptr; - -end: - DBG3("Session %" PRIu64 " NOT found by id", id); - return nullptr; -} - /* * Create a new session and add it to the session list. * Session list lock must be held by the caller. @@ -1189,8 +1184,8 @@ session_create(const char *name, uid_t uid, gid_t gid, struct ltt_session **out_ lttng_dynamic_array_init(&new_session->clear_notifiers, sizeof(struct ltt_session_clear_notifier_element), nullptr); - urcu_ref_init(&new_session->ref); - pthread_mutex_init(&new_session->lock, nullptr); + urcu_ref_init(&new_session->ref_count); + pthread_mutex_init(&new_session->_lock, nullptr); new_session->creation_time = time(nullptr); if (new_session->creation_time == (time_t) -1) { @@ -1361,7 +1356,7 @@ int session_reset_rotation_state(ltt_session& session, enum lttng_rotation_state int ret = 0; ASSERT_SESSION_LIST_LOCKED(); - ASSERT_LOCKED(session.lock); + ASSERT_LOCKED(session._lock); session.rotation_state = result; if (session.rotation_pending_check_timer_enabled) { @@ -1424,7 +1419,7 @@ end: return found; } -void ls::details::locked_session_release(ltt_session *session) +void ltt_session::_locked_session_release(ltt_session *session) { if (!session) { return; @@ -1434,13 +1429,23 @@ void ls::details::locked_session_release(ltt_session *session) session_put(session); } -ltt_session::locked_ref ls::find_locked_session_by_id(ltt_session::id_t id) +void ltt_session::_locked_const_session_release(const ltt_session *session) +{ + if (!session) { + return; + } + + ltt_session::_const_session_unlock(*session); + ltt_session::_const_session_put(session); +} + +ltt_session::locked_ref ltt_session::find_locked_session(ltt_session::id_t id) { lttng::urcu::read_lock_guard rcu_lock; auto session = session_find_by_id(id); if (!session) { - return nullptr; + LTTNG_THROW_SESSION_NOT_FOUND_BY_ID_ERROR(id); } /* @@ -1451,14 +1456,105 @@ ltt_session::locked_ref ls::find_locked_session_by_id(ltt_session::id_t id) return ltt_session::locked_ref(session); } -ltt_session::sptr ls::find_session_by_id(ltt_session::id_t id) +ltt_session::locked_ref ltt_session::find_locked_session(lttng::c_string_view name) +{ + lttng::urcu::read_lock_guard rcu_lock; + auto session = session_find_by_name(name.data()); + + if (!session) { + LTTNG_THROW_SESSION_NOT_FOUND_BY_NAME_ERROR(name.data()); + } + + session_lock(session); + return ltt_session::locked_ref(session); +} + +ltt_session::const_locked_ref ltt_session::find_locked_const_session(ltt_session::id_t id) +{ + lttng::urcu::read_lock_guard rcu_lock; + auto session = session_find_by_id(id); + + if (!session) { + LTTNG_THROW_SESSION_NOT_FOUND_BY_ID_ERROR(id); + } + + session_lock(session); + return ltt_session::const_locked_ref(session); +} + +ltt_session::const_locked_ref ltt_session::find_locked_const_session(lttng::c_string_view name) +{ + lttng::urcu::read_lock_guard rcu_lock; + auto session = session_find_by_name(name.data()); + + if (!session) { + LTTNG_THROW_SESSION_NOT_FOUND_BY_NAME_ERROR(name.data()); + } + + session_lock(session); + return ltt_session::const_locked_ref(session); +} + +ltt_session::ref ltt_session::find_session(ltt_session::id_t id) +{ + lttng::urcu::read_lock_guard rcu_lock; + auto session = session_find_by_id(id); + + if (!session) { + LTTNG_THROW_SESSION_NOT_FOUND_BY_ID_ERROR(id); + } + + return ltt_session::ref(session); +} + +ltt_session::ref ltt_session::find_session(lttng::c_string_view name) +{ + lttng::urcu::read_lock_guard rcu_lock; + auto session = session_find_by_name(name.data()); + + if (!session) { + LTTNG_THROW_SESSION_NOT_FOUND_BY_NAME_ERROR(name.data()); + } + + return ltt_session::ref(session); +} + +ltt_session::const_ref ltt_session::find_const_session(ltt_session::id_t id) { lttng::urcu::read_lock_guard rcu_lock; auto session = session_find_by_id(id); if (!session) { - return nullptr; + LTTNG_THROW_SESSION_NOT_FOUND_BY_ID_ERROR(id); + } + + return ltt_session::const_ref(session); +} + +ltt_session::const_ref ltt_session::find_const_session(lttng::c_string_view name) +{ + lttng::urcu::read_lock_guard rcu_lock; + auto session = session_find_by_name(name.data()); + + if (!session) { + LTTNG_THROW_SESSION_NOT_FOUND_BY_NAME_ERROR(name.data()); } - return { session, session_put }; + return ltt_session::const_ref(session); +} + +void ltt_session::_const_session_put(const ltt_session *session) +{ + /* + * The session list lock must be held as any session_put() + * may cause the removal of the session from the session_list. + */ + ASSERT_SESSION_LIST_LOCKED(); + LTTNG_ASSERT(session->ref_count.refcount); + urcu_ref_put(&session->ref_count, session_release); +} + +std::unique_lock ls::lock_session_list() +{ + return std::unique_lock(the_session_list.lock); } diff --git a/src/bin/lttng-sessiond/session.hpp b/src/bin/lttng-sessiond/session.hpp index 689b4c1e5..e4d62a7c4 100644 --- a/src/bin/lttng-sessiond/session.hpp +++ b/src/bin/lttng-sessiond/session.hpp @@ -13,6 +13,7 @@ #include "trace-kernel.hpp" #include +#include #include #include #include @@ -34,13 +35,102 @@ struct ltt_ust_session; using ltt_session_destroy_notifier = void (*)(const struct ltt_session *, void *); using ltt_session_clear_notifier = void (*)(const struct ltt_session *, void *); -namespace lttng { -namespace sessiond { -namespace details { -void locked_session_release(ltt_session *session); -} /* namespace details */ -} /* namespace sessiond */ -} /* namespace lttng */ +struct ltt_session; +struct ltt_session_list; + +enum lttng_error_code +session_create(const char *name, uid_t uid, gid_t gid, struct ltt_session **out_session); +void session_lock(struct ltt_session *session); +void session_unlock(struct ltt_session *session); + +bool session_get(struct ltt_session *session); +void session_put(struct ltt_session *session); + +/* + * The session list lock covers more ground than its name implies. While + * it does protect against concurent mutations of the session list, it is + * also used as a multi-session lock when synchronizing newly-registered + * 'user space tracer' and 'agent' applications. + * + * In other words, it prevents tracer configurations from changing while they + * are being transmitted to the various applications. + */ +int session_trylock_list() noexcept; + +#define LTTNG_THROW_SESSION_NOT_FOUND_BY_NAME_ERROR(session_name) \ + throw lttng::sessiond::exceptions::session_not_found_error(session_name, \ + LTTNG_SOURCE_LOCATION()) +#define LTTNG_THROW_SESSION_NOT_FOUND_BY_ID_ERROR(id) \ + throw lttng::sessiond::exceptions::session_not_found_error(id, LTTNG_SOURCE_LOCATION()) + +void session_destroy(struct ltt_session *session); +int session_add_destroy_notifier(struct ltt_session *session, + ltt_session_destroy_notifier notifier, + void *user_data); + +int session_add_clear_notifier(struct ltt_session *session, + ltt_session_clear_notifier notifier, + void *user_data); +void session_notify_clear(ltt_session& session); + +enum consumer_dst_type session_get_consumer_destination_type(const struct ltt_session *session); +const char *session_get_net_consumer_hostname(const struct ltt_session *session); +void session_get_net_consumer_ports(const struct ltt_session *session, + uint16_t *control_port, + uint16_t *data_port); +struct lttng_trace_archive_location * +session_get_trace_archive_location(const struct ltt_session *session); + +struct ltt_session_list *session_get_list(); +void session_list_wait_empty(std::unique_lock list_lock); + +bool session_access_ok(struct ltt_session *session, uid_t uid); + +int session_reset_rotation_state(ltt_session& session, enum lttng_rotation_state result); + +/* Create a new trace chunk object from the session's configuration. */ +struct lttng_trace_chunk * +session_create_new_trace_chunk(const struct ltt_session *session, + const struct consumer_output *consumer_output_override, + const char *session_base_path_override, + const char *chunk_name_override); + +/* + * Set `new_trace_chunk` as the session's current trace chunk. A reference + * to `new_trace_chunk` is acquired by the session. The chunk is created + * on remote peers (consumer and relay daemons). + * + * A reference to the session's current trace chunk is returned through + * `current_session_trace_chunk` on success. + */ +int session_set_trace_chunk(struct ltt_session *session, + struct lttng_trace_chunk *new_trace_chunk, + struct lttng_trace_chunk **current_session_trace_chunk); + +/* + * Close a chunk on the remote peers of a session. Has no effect on the + * ltt_session itself. + */ +int session_close_trace_chunk(struct ltt_session *session, + struct lttng_trace_chunk *trace_chunk, + enum lttng_trace_chunk_command_type close_command, + char *path); + +/* Open a packet in all channels of a given session. */ +enum lttng_error_code session_open_packets(struct ltt_session *session); + +bool session_output_supports_trace_chunks(const struct ltt_session *session); + +/* + * Sample the id of a session looked up via its name. + * Here the term "sampling" hint the caller that this return the id at a given + * point in time with no guarantee that the session for which the id was + * sampled still exist at that point. + * + * Return 0 when the session is not found, + * Return 1 when the session is found and set `id`. + */ +bool sample_session_id_by_name(const char *name, uint64_t *id); /* * Tracing session list @@ -79,12 +169,50 @@ struct ltt_session_list { */ struct ltt_session { using id_t = uint64_t; + +private: + static void _locked_session_release(ltt_session *session); + static void _locked_const_session_release(const ltt_session *session); + static void _const_session_put(const ltt_session *session); + static void _const_session_unlock(const ltt_session &session); + +public: using locked_ref = std::unique_ptr::deleter>; - using sptr = std::shared_ptr; + ltt_session::_locked_session_release>::deleter>; + using ref = std::unique_ptr< + ltt_session, + lttng::memory::create_deleter_class::deleter>; + using const_locked_ref = + std::unique_ptr::deleter>; + using const_ref = std::unique_ptr< + const ltt_session, + lttng::memory::create_deleter_class::deleter>; + + void lock() const noexcept; + void unlock() const noexcept; + + /* + * Session list lock must be acquired by the caller. + * + * The caller must not keep the ownership of the returned locked session + * for longer than strictly necessary. If your intention is to acquire + * a reference to an ltt_session, see `find_session()`. + */ + static locked_ref find_locked_session(ltt_session::id_t id); + static locked_ref find_locked_session(lttng::c_string_view name); + static const_locked_ref find_locked_const_session(ltt_session::id_t id); + static const_locked_ref find_locked_const_session(lttng::c_string_view name); + + static ref find_session(ltt_session::id_t id); + static ref find_session(lttng::c_string_view name); + static const_ref find_const_session(ltt_session::id_t id); + static const_ref find_const_session(lttng::c_string_view name); char name[NAME_MAX]; bool has_auto_generated_name; @@ -95,13 +223,13 @@ struct ltt_session { time_t creation_time; struct ltt_kernel_session *kernel_session; struct ltt_ust_session *ust_session; - struct urcu_ref ref; + mutable struct urcu_ref ref_count; /* * Protect any read/write on this session data structure. This lock must be * acquired *before* using any public functions declared below. Use * session_lock() and session_unlock() for that. */ - pthread_mutex_t lock; + mutable pthread_mutex_t _lock; struct cds_list_head list; /* session unique identifier */ id_t id; @@ -215,114 +343,113 @@ struct ltt_session { struct lttng_dynamic_array clear_notifiers; /* Session base path override. Set non-null. */ char *base_path; -}; - -enum lttng_error_code -session_create(const char *name, uid_t uid, gid_t gid, struct ltt_session **out_session); -void session_lock(struct ltt_session *session); -void session_unlock(struct ltt_session *session); - -/* - * The session list lock covers more ground than its name implies. While - * it does protect against concurent mutations of the session list, it is - * also used as a multi-session lock when synchronizing newly-registered - * 'user space tracer' and 'agent' applications. - * - * In other words, it prevents tracer configurations from changing while they - * are being transmitted to the various applications. - */ -void session_lock_list() noexcept; -int session_trylock_list() noexcept; -void session_unlock_list() noexcept; - -void session_destroy(struct ltt_session *session); -int session_add_destroy_notifier(struct ltt_session *session, - ltt_session_destroy_notifier notifier, - void *user_data); - -int session_add_clear_notifier(struct ltt_session *session, - ltt_session_clear_notifier notifier, - void *user_data); -void session_notify_clear(ltt_session& session); - -bool session_get(struct ltt_session *session); -void session_put(struct ltt_session *session); - -enum consumer_dst_type session_get_consumer_destination_type(const struct ltt_session *session); -const char *session_get_net_consumer_hostname(const struct ltt_session *session); -void session_get_net_consumer_ports(const struct ltt_session *session, - uint16_t *control_port, - uint16_t *data_port); -struct lttng_trace_archive_location * -session_get_trace_archive_location(const struct ltt_session *session); - -struct ltt_session *session_find_by_name(const char *name); -struct ltt_session *session_find_by_id(ltt_session::id_t id); - -struct ltt_session_list *session_get_list(); -void session_list_wait_empty(); - -bool session_access_ok(struct ltt_session *session, uid_t uid); - -int session_reset_rotation_state(ltt_session& session, enum lttng_rotation_state result); -/* Create a new trace chunk object from the session's configuration. */ -struct lttng_trace_chunk * -session_create_new_trace_chunk(const struct ltt_session *session, - const struct consumer_output *consumer_output_override, - const char *session_base_path_override, - const char *chunk_name_override); - -/* - * Set `new_trace_chunk` as the session's current trace chunk. A reference - * to `new_trace_chunk` is acquired by the session. The chunk is created - * on remote peers (consumer and relay daemons). - * - * A reference to the session's current trace chunk is returned through - * `current_session_trace_chunk` on success. - */ -int session_set_trace_chunk(struct ltt_session *session, - struct lttng_trace_chunk *new_trace_chunk, - struct lttng_trace_chunk **current_session_trace_chunk); - -/* - * Close a chunk on the remote peers of a session. Has no effect on the - * ltt_session itself. - */ -int session_close_trace_chunk(struct ltt_session *session, - struct lttng_trace_chunk *trace_chunk, - enum lttng_trace_chunk_command_type close_command, - char *path); - -/* Open a packet in all channels of a given session. */ -enum lttng_error_code session_open_packets(struct ltt_session *session); -bool session_output_supports_trace_chunks(const struct ltt_session *session); - -/* - * Sample the id of a session looked up via its name. - * Here the term "sampling" hint the caller that this return the id at a given - * point in time with no guarantee that the session for which the id was - * sampled still exist at that point. - * - * Return 0 when the session is not found, - * Return 1 when the session is found and set `id`. - */ -bool sample_session_id_by_name(const char *name, uint64_t *id); +}; namespace lttng { namespace sessiond { -/* - * Session list lock must be acquired by the caller. - * The caller must not keep the ownership of the returned locked session - * for longer than strictly necessary. If your intention is to acquire - * a reference to an ltt_session, see `find_session_by_id()`. - */ -ltt_session::locked_ref find_locked_session_by_id(ltt_session::id_t id); - -ltt_session::sptr find_session_by_id(ltt_session::id_t id); +std::unique_lock lock_session_list(); +namespace exceptions { +/** + * @class session_not_found_error + * @brief Represents a session-not-found error and provides the parameters used to query the session + * for use by error-reporting code. + */ +class session_not_found_error : public lttng::runtime_error { +public: + class query_parameter { + public: + enum class query_type : std::uint8_t { BY_NAME, BY_ID }; + + /* + * Intentionally not explicit to allow construction from c-style strings, + * std::string, and lttng::c_string_view. + * + * NOLINTBEGIN(google-explicit-constructor) + */ + query_parameter(const std::string& session_name) : + type(query_type::BY_NAME), parameter(session_name) + { + } + /* NOLINTEND(google-explicit-constructor) */ + + explicit query_parameter(ltt_session::id_t id_) : type(query_type::BY_ID), parameter(id_) + { + } + + query_parameter(const query_parameter& other) : type(other.type) + { + if (type == query_type::BY_NAME) { + new (¶meter.name) std::string(other.parameter.name); + } else { + parameter.id = other.parameter.id; + } + } + + ~query_parameter() + { + if (type == query_type::BY_NAME) { + parameter.name.~basic_string(); + } + } + + query_type type; + union parameter { + explicit parameter(std::string name_) : name(std::move(name_)) + { + } + + explicit parameter(ltt_session::id_t id_) : id(id_) + { + } + + /* + * parameter doesn't have enough information to do this safely; it it + * delegated to its parent which uses placement new. + */ + parameter() + { + } + + parameter(const parameter&) = delete; + parameter(const parameter&&) = delete; + parameter& operator=(parameter&) = delete; + parameter& operator=(parameter&&) = delete; + + ~parameter() + { + } + + std::string name; + ltt_session::id_t id; + } parameter; + }; + + session_not_found_error(const std::string& session_name, + const lttng::source_location& source_location_) : + lttng::runtime_error(fmt::format("Session not found: name=`{}`", session_name), + source_location_), + query_parameter(session_name) + { + } + + session_not_found_error(ltt_session::id_t session_id, + const lttng::source_location& source_location_) : + lttng::runtime_error("Session not found: id=" + std::to_string(session_id), + source_location_), + query_parameter(session_id) + { + } + + session_not_found_error(const session_not_found_error& other) = default; + ~session_not_found_error() noexcept override = default; + + query_parameter query_parameter; +}; +} // namespace exceptions } /* namespace sessiond */ } /* namespace lttng */ diff --git a/src/bin/lttng-sessiond/ust-app.cpp b/src/bin/lttng-sessiond/ust-app.cpp index 2788ddbe0..2439d1e99 100644 --- a/src/bin/lttng-sessiond/ust-app.cpp +++ b/src/bin/lttng-sessiond/ust-app.cpp @@ -497,7 +497,6 @@ static void delete_ust_app_channel_rcu(struct rcu_head *head) static void save_per_pid_lost_discarded_counters(struct ust_app_channel *ua_chan) { uint64_t discarded = 0, lost = 0; - struct ltt_session *session; struct ltt_ust_channel *uchan; if (ua_chan->attr.type != LTTNG_UST_ABI_CHAN_PER_CPU) { @@ -505,7 +504,16 @@ static void save_per_pid_lost_discarded_counters(struct ust_app_channel *ua_chan } lttng::urcu::read_lock_guard read_lock; - session = session_find_by_id(ua_chan->session->tracing_id); + + ltt_session::ref session; + try { + session = ltt_session::find_session(ua_chan->session->tracing_id); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + DBG_FMT("Failed to save per-pid lost/discarded counters: {}, location='{}'", + ex.what(), + ex.source_location); + } + if (!session || !session->ust_session) { /* * Not finding the session is not an error because there are @@ -522,7 +530,7 @@ static void save_per_pid_lost_discarded_counters(struct ust_app_channel *ua_chan * events is done exactly once. The session is then unpublished * from the session list, resulting in this condition. */ - goto end; + return; } if (ua_chan->attr.overwrite) { @@ -540,16 +548,11 @@ static void save_per_pid_lost_discarded_counters(struct ust_app_channel *ua_chan ua_chan->name); if (!uchan) { ERR("Missing UST channel to store discarded counters"); - goto end; + return; } uchan->per_pid_closed_app_discarded += discarded; uchan->per_pid_closed_app_lost += lost; - -end: - if (session) { - session_put(session); - } } /* @@ -1024,7 +1027,7 @@ static void delete_ust_app(struct ust_app *app) * The session list lock must be held during this function to guarantee * the existence of ua_sess. */ - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); /* Delete ust app sessions info */ sock = app->sock; app->sock = -1; @@ -1114,7 +1117,6 @@ static void delete_ust_app(struct ust_app *app) DBG2("UST app pid %d deleted", app->pid); free(app); - session_unlock_list(); } /* @@ -3444,7 +3446,7 @@ static int create_channel_per_uid(struct ust_app *app, int ret; struct buffer_reg_uid *reg_uid; struct buffer_reg_channel *buf_reg_chan; - struct ltt_session *session = nullptr; + ltt_session::ref session; enum lttng_error_code notification_ret; LTTNG_ASSERT(app); @@ -3475,9 +3477,9 @@ static int create_channel_per_uid(struct ust_app *app, goto error; } - session = session_find_by_id(ua_sess->tracing_id); - LTTNG_ASSERT(session); - ASSERT_LOCKED(session->lock); + /* Guaranteed to exist; will not throw. */ + session = ltt_session::find_session(ua_sess->tracing_id); + ASSERT_LOCKED(session->_lock); ASSERT_SESSION_LIST_LOCKED(); /* @@ -3545,9 +3547,6 @@ send_channel: } error: - if (session) { - session_put(session); - } return ret; } @@ -3567,7 +3566,7 @@ static int create_channel_per_pid(struct ust_app *app, int ret; lsu::registry_session *registry; enum lttng_error_code cmd_ret; - struct ltt_session *session = nullptr; + ltt_session::ref session; uint64_t chan_reg_key; LTTNG_ASSERT(app); @@ -3594,9 +3593,9 @@ static int create_channel_per_pid(struct ust_app *app, goto error; } - session = session_find_by_id(ua_sess->tracing_id); - LTTNG_ASSERT(session); - ASSERT_LOCKED(session->lock); + /* Guaranteed to exist; will not throw. */ + session = ltt_session::find_session(ua_sess->tracing_id); + ASSERT_LOCKED(session->_lock); ASSERT_SESSION_LIST_LOCKED(); /* Create and get channel on the consumer side. */ @@ -3645,9 +3644,6 @@ error_remove_from_registry: } } error: - if (session) { - session_put(session); - } return ret; } @@ -3894,7 +3890,7 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess, int ret = 0; struct ust_app_channel *metadata; struct consumer_socket *socket; - struct ltt_session *session = nullptr; + ltt_session::ref session; LTTNG_ASSERT(ua_sess); LTTNG_ASSERT(app); @@ -3943,9 +3939,9 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess, */ locked_registry->_metadata_key = metadata->key; - session = session_find_by_id(ua_sess->tracing_id); - LTTNG_ASSERT(session); - ASSERT_LOCKED(session->lock); + /* Guaranteed to exist; will not throw. */ + session = ltt_session::find_session(ua_sess->tracing_id); + ASSERT_LOCKED(session->_lock); ASSERT_SESSION_LIST_LOCKED(); /* @@ -3985,9 +3981,6 @@ error_consumer: lttng_fd_put(LTTNG_FD_APPS, 1); delete_ust_app_channel(-1, metadata, app, locked_registry); error: - if (session) { - session_put(session); - } return ret; } diff --git a/src/bin/lttng-sessiond/ust-registry-session.cpp b/src/bin/lttng-sessiond/ust-registry-session.cpp index 501184724..7c091f7e6 100644 --- a/src/bin/lttng-sessiond/ust-registry-session.cpp +++ b/src/bin/lttng-sessiond/ust-registry-session.cpp @@ -539,10 +539,8 @@ void lsu::registry_session::accept( { /* The caller already holds the session and session list locks. */ ASSERT_SESSION_LIST_LOCKED(); - const auto session = lttng::sessiond::find_session_by_id(_tracing_id); - - LTTNG_ASSERT(session); - ASSERT_LOCKED(session->lock); + const auto session = ltt_session::find_session(_tracing_id); + ASSERT_LOCKED(session->_lock); visitor.visit(lst::environment_field( "trace_name", diff --git a/src/common/exception.cpp b/src/common/exception.cpp index 219e80fb1..50520c55c 100644 --- a/src/common/exception.cpp +++ b/src/common/exception.cpp @@ -32,6 +32,13 @@ lttng::runtime_error::runtime_error(const std::string& msg, { } +lttng::allocation_failure::allocation_failure(const std::string& msg, +std::size_t allocation_size_, + const lttng::source_location& location) : + lttng::runtime_error(msg, location), allocation_size(allocation_size_) +{ +} + lttng::unsupported_error::unsupported_error(const std::string& msg, const lttng::source_location& location) : lttng::runtime_error(msg, location) diff --git a/src/common/exception.hpp b/src/common/exception.hpp index f878b9cf5..340231734 100644 --- a/src/common/exception.hpp +++ b/src/common/exception.hpp @@ -23,6 +23,8 @@ #define LTTNG_THROW_POSIX(msg, errno_code) \ throw lttng::posix_error(msg, errno_code, LTTNG_SOURCE_LOCATION()) #define LTTNG_THROW_ERROR(msg) throw lttng::runtime_error(msg, LTTNG_SOURCE_LOCATION()) +#define LTTNG_THROW_ALLOCATION_FAILURE_ERROR(msg, allocation_size) \ + throw lttng::allocation_failure(msg, allocation_size, LTTNG_SOURCE_LOCATION()) #define LTTNG_THROW_UNSUPPORTED_ERROR(msg) \ throw lttng::unsupported_error(msg, LTTNG_SOURCE_LOCATION()) #define LTTNG_THROW_COMMUNICATION_ERROR(msg) \ @@ -92,6 +94,22 @@ public: lttng::source_location source_location; }; +/** + * @class allocation_failure + * @brief Represents an allocation failure. + * + * Thrown when an allocation fails. Differs from bad_alloc in that it offers a message and a + * source location. + */ +class allocation_failure : public lttng::runtime_error { +public: + explicit allocation_failure(const std::string& msg, + std::size_t allocation_size, + const lttng::source_location& source_location); + + std::size_t allocation_size; +}; + /** * @class unsupported_error * @brief Represents an error for unsupported features. diff --git a/tests/unit/test_session.cpp b/tests/unit/test_session.cpp index f6e58c8cb..af98c7b7e 100644 --- a/tests/unit/test_session.cpp +++ b/tests/unit/test_session.cpp @@ -88,11 +88,10 @@ static void empty_session_list() { struct ltt_session *iter, *tmp; - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); cds_list_for_each_entry_safe (iter, tmp, &session_list->head, list) { session_destroy(iter); } - session_unlock_list(); /* Session list must be 0 */ LTTNG_ASSERT(!session_list_count()); @@ -107,7 +106,7 @@ static int create_one_session(const char *name) enum lttng_error_code ret_code; struct ltt_session *session = nullptr; - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); ret_code = session_create(name, geteuid(), getegid(), &session); session_put(session); if (ret_code == LTTNG_OK) { @@ -128,7 +127,6 @@ static int create_one_session(const char *name) ret = -1; } - session_unlock_list(); return ret; } @@ -154,6 +152,7 @@ static int destroy_one_session(struct ltt_session *session) /* Fail */ ret = -1; } + return ret; } @@ -163,33 +162,28 @@ static int destroy_one_session(struct ltt_session *session) */ static int two_session_same_name() { - int ret; - struct ltt_session *sess; - - ret = create_one_session(SESSION1); + const auto ret = create_one_session(SESSION1); if (ret < 0) { /* Fail */ - ret = -1; - goto end; + return -1; } - session_lock_list(); - sess = session_find_by_name(SESSION1); - if (sess) { + /* + * Mind the order of the declaration of list_lock vs session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + try { + const auto session = ltt_session::find_session(SESSION1); /* Success */ - session_put(sess); - session_unlock_list(); - ret = 0; - goto end_unlock; - } else { + return 0; + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { /* Fail */ - ret = -1; - goto end_unlock; + return -1; } -end_unlock: - session_unlock_list(); -end: - return ret; } static void test_session_list() @@ -205,43 +199,61 @@ static void test_create_one_session() static void test_validate_session() { - struct ltt_session *tmp; - - session_lock_list(); - tmp = session_find_by_name(SESSION1); + /* + * Mind the order of the declaration of list_lock vs session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + ltt_session::ref session; + + try { + session = ltt_session::find_session(SESSION1); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + } - ok(tmp != nullptr, "Validating session: session found"); + ok(session, "Validating session: session found"); - if (tmp) { - ok(tmp->kernel_session == nullptr && strlen(tmp->name), + if (session) { + ok(session->kernel_session == nullptr && strlen(session->name), "Validating session: basic sanity check"); } else { skip(1, "Skipping session validation check as session was not found"); - goto end; + return; } - session_lock(tmp); - session_unlock(tmp); - session_put(tmp); -end: - session_unlock_list(); + session->lock(); + session->unlock(); } static void test_destroy_session() { - struct ltt_session *tmp; - - session_lock_list(); - tmp = session_find_by_name(SESSION1); + /* + * Mind the order of the declaration of list_lock vs session: + * the session list lock must always be released _after_ the release of + * a session's reference (the destruction of a ref/locked_ref) to ensure + * since the reference's release may unpublish the session from the list of + * sessions. + */ + const auto list_lock = lttng::sessiond::lock_session_list(); + ltt_session::ref session; + + try { + session = ltt_session::find_session(SESSION1); + } catch (const lttng::sessiond::exceptions::session_not_found_error& ex) { + } - ok(tmp != nullptr, "Destroying session: session found"); + ok(session, "Destroying session: session found"); - if (tmp) { - ok(destroy_one_session(tmp) == 0, "Destroying session: %s destroyed", SESSION1); + if (session) { + ok(destroy_one_session(session.release()) == 0, + "Destroying session: %s destroyed", + SESSION1); } else { skip(1, "Skipping session destruction as it was not found"); } - session_unlock_list(); } static void test_duplicate_session() @@ -255,7 +267,8 @@ static void test_session_name_generation() enum lttng_error_code ret_code; const char *expected_session_name_prefix = DEFAULT_SESSION_NAME; - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); + ret_code = session_create(nullptr, geteuid(), getegid(), &session); ok(ret_code == LTTNG_OK, "Create session with a NULL name (auto-generate a name)"); if (!session) { @@ -271,7 +284,6 @@ static void test_session_name_generation() DEFAULT_SESSION_NAME); end: session_put(session); - session_unlock_list(); } static void test_large_session_number() @@ -292,7 +304,7 @@ static void test_large_session_number() failed = 0; - session_lock_list(); + const auto list_lock = lttng::sessiond::lock_session_list(); for (i = 0; i < MAX_SESSIONS; i++) { cds_list_for_each_entry_safe (iter, tmp, &session_list->head, list) { LTTNG_ASSERT(session_get(iter)); @@ -303,7 +315,6 @@ static void test_large_session_number() } } } - session_unlock_list(); ok(failed == 0 && session_list_count() == 0, "Large sessions number: destroyed %u sessions", -- 2.34.1