From fa9611b15d5090a6b88221ea1fccfef897f12544 Mon Sep 17 00:00:00 2001 From: Jonathan Rajotte Date: Thu, 11 Nov 2021 15:02:54 -0500 Subject: [PATCH] Fix: action executor: ref count imbalance for session object MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue ============== The following scenario leads to a hang on `lttng destroy`. # Start lttng-sessiond under gdb $ gdb lttng-sessiond set pagination off set non-stop start break action_executor_snapshot_session_handler $ lttng add-trigger --name my_trigger --condition=event-rule-matches --type=user:tracepoint --name=sample_component:message --action=snapshot-session my_snapshot $ lttng create --snapshot my_snapshot $ lttng enable-event -u -a $ lttng start $ start an app producing a single sample_component:message # gdb should break on thread 6 # inside gdb thread 6 $ lttng destroy my_snapshot $ lttng create --snapshot my_snapshot $ lttng enable-event -u -a $ lttng start # inside gdb use `continue` $ lttng destroy my_snapshot The destroy command hang: Destroying session my_snapshot.... .... Cause ===== The scenario forces the usage of the following code path: if (session->id != LTTNG_OPTIONAL_GET(item->context.session_id)) { 624├───────────────> DBG("Session id for session `%s` (id: %" PRIu64 625│ " is not the same that was sampled (id: %" PRIu64 626│ " at the moment the work item was enqueued for %s` action of trigger `%s`", 627│ session_name, session->id, 628│ LTTNG_OPTIONAL_GET(item->context.session_id), 629│ get_action_name(action), 630│ get_trigger_name(work_item->trigger)); 631│ ret = 0; 632│ goto error_unlock_list; 633│ } At that point a reference on the session object was taken on line: 610│ session = session_find_by_name(session_name); But the reference is never put on `error_unlock_list` resulting in a ref count problem. Solution ======== Use `session_put` for the code path. Note that most of the handler also have the same problem that was introduced by commit 72365501d3148ca977a09bad8de0ec51b427bdd8 [1] Known drawbacks ========= None. Refs ========= [1] https://github.com/lttng/lttng-tools/commit/72365501d3148ca977a09bad8de0ec51b427bdd8 Signed-off-by: Jonathan Rajotte Signed-off-by: Jérémie Galarneau Change-Id: I23c3c089866df74854bbfe64320310c4b28ee41d --- src/bin/lttng-sessiond/action-executor.c | 28 ++++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/bin/lttng-sessiond/action-executor.c b/src/bin/lttng-sessiond/action-executor.c index 14ac8103b..878680a70 100644 --- a/src/bin/lttng-sessiond/action-executor.c +++ b/src/bin/lttng-sessiond/action-executor.c @@ -331,12 +331,12 @@ static int action_executor_start_session_handler( get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_start_trace(session); @@ -357,8 +357,9 @@ static int action_executor_start_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); @@ -423,12 +424,12 @@ static int action_executor_stop_session_handler( get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_stop_trace(session); @@ -449,8 +450,9 @@ static int action_executor_stop_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); @@ -515,12 +517,12 @@ static int action_executor_rotate_session_handler( get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_rotate_session(session, NULL, false, @@ -548,8 +550,9 @@ static int action_executor_rotate_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); @@ -629,12 +632,12 @@ static int action_executor_snapshot_session_handler( get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_snapshot_record(session, snapshot_output, 0); @@ -651,8 +654,9 @@ static int action_executor_snapshot_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); -- 2.34.1