Fix: sessiond: rotation trigger leak
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Fri, 15 Apr 2022 05:30:50 +0000 (01:30 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Wed, 18 May 2022 14:29:36 +0000 (10:29 -0400)
==1801304==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 224 byte(s) in 2 object(s) allocated from:
    #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x559fbeb64175 in zmalloc_internal ../../src/common/macros.hpp:60
    #2 0x559fbeb6a291 in lttng_trigger* zmalloc<lttng_trigger>() ../../src/common/macros.hpp:89
    #3 0x559fbeb64aa6 in lttng_trigger_create /home/jgalar/EfficiOS/src/lttng-tools/src/common/trigger.cpp:58
    #4 0x559fbe9dc417 in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:87
    #5 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993
    #6 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246
    #7 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624
    #8 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68
    #9 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1)

Indirect leak of 208 byte(s) in 2 object(s) allocated from:
    #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x559fbeb16e21 in zmalloc_internal ../../src/common/macros.hpp:60
    #2 0x559fbeb16e31 in lttng_action_notify* zmalloc<lttng_action_notify>() ../../src/common/macros.hpp:89
    #3 0x559fbeb168a0 in lttng_action_notify_create actions/notify.cpp:135
    #4 0x559fbe9dc34b in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:80
    #5 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993
    #6 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246
    #7 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624
    #8 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68
    #9 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1)

Indirect leak of 160 byte(s) in 2 object(s) allocated from:
    #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x559fbeb3d7a1 in zmalloc_internal ../../src/common/macros.hpp:60
    #2 0x559fbeb3fa35 in lttng_condition_session_consumed_size* zmalloc<lttng_condition_session_consumed_size>() ../../src/common/macros.hpp:89
    #3 0x559fbeb3e6fd in lttng_condition_session_consumed_size_create conditions/session-consumed-size.cpp:206
    #4 0x559fbe9dc0f1 in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:54
    #5 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993
    #6 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246
    #7 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624
    #8 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68
    #9 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1)

Indirect leak of 112 byte(s) in 2 object(s) allocated from:
    #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x559fbeb242ad in zmalloc_internal ../../src/common/macros.hpp:60
    #2 0x559fbeb27062 in zmalloc<(anonymous namespace)::lttng_rate_policy_every_n> ../../src/common/macros.hpp:89
    #3 0x559fbeb25e9f in lttng_rate_policy_every_n_create actions/rate-policy.cpp:492
    #4 0x559fbeb168b9 in lttng_action_notify_create actions/notify.cpp:141
    #5 0x559fbe9dc34b in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:80
    #6 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993
    #7 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246
    #8 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624
    #9 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68
    #10 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1)

Indirect leak of 34 byte(s) in 2 object(s) allocated from:
    #0 0x7fe0f4e19319 in __interceptor_strdup /usr/src/debug/gcc/libsanitizer/asan/asan_interceptors.cpp:454
    #1 0x559fbeb3f603 in lttng_condition_session_consumed_size_set_session_name conditions/session-consumed-size.cpp:442
    #2 0x559fbe9dc2c4 in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:71
    #3 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993
    #4 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246
    #5 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624
    #6 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68
    #7 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1)

The rotation trigger of a session (used for size-based rotations) is
never cleaned-up. It is now cleaned up every time its condition is
hit and whenever the session is destroyed.

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

include/lttng/notification/notification-internal.h
src/bin/lttng-sessiond/rotate.c
src/bin/lttng-sessiond/rotation-thread.c
src/bin/lttng-sessiond/session.c
src/bin/lttng-sessiond/session.h
src/common/notification.c

index dc3a0e1d67569711ef3a13076256c6453b2119eb..99a545642f0cf152439a7a80457981620230716a 100644 (file)
@@ -43,4 +43,12 @@ ssize_t lttng_notification_create_from_payload(
                struct lttng_payload_view *view,
                struct lttng_notification **notification);
 
+const struct lttng_condition *lttng_notification_get_const_condition(
+               const struct lttng_notification *notification);
+const struct lttng_evaluation *lttng_notification_get_const_evaluation(
+               const struct lttng_notification *notification);
+
+const struct lttng_trigger *lttng_notification_get_const_trigger(
+               const struct lttng_notification *notification);
+
 #endif /* LTTNG_NOTIFICATION_INTERNAL_H */
index cdf95f353dbbfa2cb0ba276cdba6292d26dc92ae..9e1126bdf03e6b86b523f659af3ef1359ba847b4 100644 (file)
@@ -25,6 +25,8 @@
 
 #include <lttng/notification/channel-internal.h>
 #include <lttng/rotate-internal.h>
+#include <lttng/condition/condition-internal.h>
+#include <lttng/action/action-internal.h>
 
 #include "session.h"
 #include "rotate.h"
@@ -45,21 +47,22 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64
        int ret;
        enum lttng_condition_status condition_status;
        enum lttng_notification_channel_status nc_status;
-       struct lttng_action *action;
+       struct lttng_condition *rotate_condition = NULL;
+       struct lttng_action *notify_action = NULL;
        const struct lttng_credentials session_creds = {
                .uid = LTTNG_OPTIONAL_INIT_VALUE(session->uid),
                .gid = LTTNG_OPTIONAL_INIT_VALUE(session->gid),
        };
 
-       session->rotate_condition = lttng_condition_session_consumed_size_create();
-       if (!session->rotate_condition) {
+       rotate_condition = lttng_condition_session_consumed_size_create();
+       if (!rotate_condition) {
                ERR("Failed to create session consumed size condition object");
                ret = -1;
                goto end;
        }
 
        condition_status = lttng_condition_session_consumed_size_set_threshold(
-                       session->rotate_condition, size);
+                       rotate_condition, size);
        if (condition_status != LTTNG_CONDITION_STATUS_OK) {
                ERR("Could not set session consumed size condition threshold (size = %" PRIu64 ")",
                                size);
@@ -69,7 +72,7 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64
 
        condition_status =
                        lttng_condition_session_consumed_size_set_session_name(
-                               session->rotate_condition, session->name);
+                               rotate_condition, session->name);
        if (condition_status != LTTNG_CONDITION_STATUS_OK) {
                ERR("Could not set session consumed size condition session name (name = %s)",
                                session->name);
@@ -77,15 +80,16 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64
                goto end;
        }
 
-       action = lttng_action_notify_create();
-       if (!action) {
+       notify_action = lttng_action_notify_create();
+       if (!notify_action) {
                ERR("Could not create notify action");
                ret = -1;
                goto end;
        }
 
-       session->rotate_trigger = lttng_trigger_create(session->rotate_condition,
-                       action);
+       assert(!session->rotate_trigger);
+       session->rotate_trigger = lttng_trigger_create(rotate_condition,
+                       notify_action);
        if (!session->rotate_trigger) {
                ERR("Could not create size-based rotation trigger");
                ret = -1;
@@ -98,7 +102,7 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64
                        session->rotate_trigger, &session_creds);
 
        nc_status = lttng_notification_channel_subscribe(
-                       rotate_notification_channel, session->rotate_condition);
+                       rotate_notification_channel, rotate_condition);
        if (nc_status != LTTNG_NOTIFICATION_CHANNEL_STATUS_OK) {
                ERR("Could not subscribe to session consumed size notification");
                ret = -1;
@@ -117,6 +121,11 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64
        ret = 0;
 
 end:
+       lttng_condition_put(rotate_condition);
+       lttng_action_put(notify_action);
+       if (ret) {
+               lttng_trigger_put(session->rotate_trigger);
+       }
        return ret;
 }
 
@@ -126,9 +135,10 @@ int unsubscribe_session_consumed_size_rotation(struct ltt_session *session,
        int ret = 0;
        enum lttng_notification_channel_status status;
 
+       assert(session->rotate_trigger);
        status = lttng_notification_channel_unsubscribe(
                        rotate_notification_channel,
-                       session->rotate_condition);
+                       lttng_trigger_get_const_condition(session->rotate_trigger));
        if (status != LTTNG_NOTIFICATION_CHANNEL_STATUS_OK) {
                ERR("Session unsubscribe error: %d", (int) status);
                ret = -1;
@@ -142,6 +152,9 @@ int unsubscribe_session_consumed_size_rotation(struct ltt_session *session,
                goto end;
        }
 
+       lttng_trigger_put(session->rotate_trigger);
+       session->rotate_trigger = NULL;
+
        ret = 0;
 end:
        return ret;
index 590b95dd224b0cc0f6e5351b4cd85f147a66d8ac..e4811bd44bd957e854c8a0be1250632cd1a07854 100644 (file)
@@ -26,6 +26,7 @@
 #include <lttng/rotate-internal.h>
 #include <lttng/location-internal.h>
 #include <lttng/condition/condition-internal.h>
+#include <lttng/notification/notification-internal.h>
 
 #include "rotation-thread.h"
 #include "lttng-sessiond.h"
@@ -619,8 +620,7 @@ end:
 }
 
 static
-int handle_condition(const struct lttng_condition *condition,
-               const struct lttng_evaluation *evaluation,
+int handle_condition(const struct lttng_notification *notification,
                struct notification_thread_handle *notification_thread_handle)
 {
        int ret = 0;
@@ -630,6 +630,10 @@ int handle_condition(const struct lttng_condition *condition,
        enum lttng_evaluation_status evaluation_status;
        uint64_t consumed;
        struct ltt_session *session;
+       const struct lttng_condition *condition =
+                       lttng_notification_get_const_condition(notification);
+       const struct lttng_evaluation *evaluation =
+                       lttng_notification_get_const_evaluation(notification);
 
        condition_type = lttng_condition_get_type(condition);
 
@@ -671,6 +675,13 @@ int handle_condition(const struct lttng_condition *condition,
        }
        session_lock(session);
 
+       if (!lttng_trigger_is_equal(session->rotate_trigger,
+                       lttng_notification_get_const_trigger(notification))) {
+               /* Notification does not originate from our rotation trigger. */
+               ret = 0;
+               goto end_unlock;
+       }
+
        ret = unsubscribe_session_consumed_size_rotation(session,
                        notification_thread_handle);
        if (ret) {
@@ -713,8 +724,6 @@ int handle_notification_channel(int fd,
        bool notification_pending;
        struct lttng_notification *notification = NULL;
        enum lttng_notification_channel_status status;
-       const struct lttng_evaluation *notification_evaluation;
-       const struct lttng_condition *notification_condition;
 
        status = lttng_notification_channel_has_pending_notification(
                        rotate_notification_channel, &notification_pending);
@@ -752,10 +761,7 @@ int handle_notification_channel(int fd,
                goto end;
        }
 
-       notification_condition = lttng_notification_get_condition(notification);
-       notification_evaluation = lttng_notification_get_evaluation(notification);
-
-       ret = handle_condition(notification_condition, notification_evaluation,
+       ret = handle_condition(notification,
                        handle->notification_thread_handle);
 
 end:
index 6f52bcf4fac62abb83baac3342de28f30f37a3ee..635192f2b23125242494c523791e4c6f6ce8c812 100644 (file)
@@ -1032,6 +1032,7 @@ void session_release(struct urcu_ref *ref)
        lttng_dynamic_array_reset(&session->clear_notifiers);
        free(session->last_archived_chunk_name);
        free(session->base_path);
+       lttng_trigger_put(session->rotate_trigger);
        free(session);
        if (session_published) {
                /*
index 1a68d8e2fdd72c2c73264126122a712950a3ca37..7b6e620ffe06fe332db4c03676b7a91cc7b0de25 100644 (file)
@@ -181,9 +181,8 @@ struct ltt_session {
         */
        bool rotated;
        /*
-        * Condition and trigger for size-based rotations.
+        * Trigger for size-based rotations.
         */
-       struct lttng_condition *rotate_condition;
        struct lttng_trigger *rotate_trigger;
        LTTNG_OPTIONAL(uint64_t) most_recent_chunk_id;
        struct lttng_trace_chunk *current_trace_chunk;
index be09c1fcc66249e34f66d16ac3de7212de360f29..a5d887007baf52c41c5fb2bf06a9af010cf5c27c 100644 (file)
@@ -179,6 +179,24 @@ const struct lttng_evaluation *lttng_notification_get_evaluation(
        return notification ? notification->evaluation : NULL;
 }
 
+const struct lttng_condition *lttng_notification_get_const_condition(
+               const struct lttng_notification *notification)
+{
+       return notification ? lttng_trigger_get_const_condition(notification->trigger) : NULL;
+}
+
+const struct lttng_evaluation *lttng_notification_get_const_evaluation(
+               const struct lttng_notification *notification)
+{
+       return notification ? notification->evaluation : NULL;
+}
+
+const struct lttng_trigger *lttng_notification_get_const_trigger(
+               const struct lttng_notification *notification)
+{
+       return notification ? notification->trigger : NULL;
+}
+
 const struct lttng_trigger *lttng_notification_get_trigger(
                struct lttng_notification *notification)
 {
This page took 0.03293 seconds and 4 git commands to generate.