trigger: use condition and action ref counting to ease internal objects management

Currently a trigger object has multiple relationship types with the action and
condition associated to it.

On the API client side, the trigger does not own the action and condition
associated to it. The user is responsible for managing the lifetime of the
associated action and condition object.

On the sessiond side, triggers created from `lttng_trigger_create_from_payload`
currently require that before calling lttng_trigger_destroy, the action and
condition be fetched and deleted using their respective destructor. This
operation cannot be done inside `lttng_trigger_destroy` since the exposed API is
clear that the trigger does no own the objects.

We can facilitate the lifetime/ownership management of the action and condition
objects using their respective reference counting mechanism.

On a trigger creation, the trigger get references on both object. On destroy,
the trigger put the references on the objects.

From an API client perspective, nothing changes. Even better, it prevents
premature freeing of these objects, since the trigger have references to these
objects.

On the sessiond side, we can now move the actual ownership of the action and
condition object to the trigger object. This is done in
`lttng_trigger_create_from_payload` forcing a put of the local references to the
object, effectively moving the ownership to the trigger object.

Note that the `lttng_trigger_get_{action, condition}` do not `get` a reference
to the object before returning it. This is done to comply with the API that was
introduced back in 2.11 which does expect a client to call lttng_{action,
condition}_destroy on the returned object.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I31d196d00c886fcf42c1b16c55e693949373568b

diff --git a/src/bin/lttng-sessiond/notification-thread-events.c b/src/bin/lttng-sessiond/notification-thread-events.c
index f7dd90cbe0aec976925dd0215af89d8b81648b10..6ccd1fe04abfdc54e3c84aa6e80ceb4edf58c096 100644 (file)
--- a/src/bin/lttng-sessiond/notification-thread-events.c
+++ b/src/bin/lttng-sessiond/notification-thread-events.c
@@ -2146,10 +2146,6 @@ error_free_ht_element:
        free(trigger_ht_element);
 error:
        if (free_trigger) {
        free(trigger_ht_element);
 error:
        if (free_trigger) {

-               struct lttng_action *action = lttng_trigger_get_action(trigger);
-
-               lttng_condition_destroy(condition);
-               lttng_action_destroy(action);

                lttng_trigger_destroy(trigger);
        }
        rcu_read_unlock();
                lttng_trigger_destroy(trigger);
        }
        rcu_read_unlock();


@@ -2184,7 +2180,6 @@ int handle_notification_thread_command_unregister_trigger(
        struct lttng_trigger_ht_element *trigger_ht_element = NULL;
        struct lttng_condition *condition = lttng_trigger_get_condition(
                        trigger);
        struct lttng_trigger_ht_element *trigger_ht_element = NULL;
        struct lttng_condition *condition = lttng_trigger_get_condition(
                        trigger);

-       struct lttng_action *action;

        enum lttng_error_code cmd_reply;
 
        rcu_read_lock();
        enum lttng_error_code cmd_reply;
 
        rcu_read_lock();


@@ -2246,10 +2241,7 @@ int handle_notification_thread_command_unregister_trigger(
                        struct lttng_trigger_ht_element, node);
        cds_lfht_del(state->triggers_ht, triggers_ht_node);
 
                        struct lttng_trigger_ht_element, node);
        cds_lfht_del(state->triggers_ht, triggers_ht_node);
 

-       condition = lttng_trigger_get_condition(trigger_ht_element->trigger);
-       lttng_condition_destroy(condition);
-       action = lttng_trigger_get_action(trigger_ht_element->trigger);
-       lttng_action_destroy(action);
+       /* Release the ownership of the trigger. */

        lttng_trigger_destroy(trigger_ht_element->trigger);
        call_rcu(&trigger_ht_element->rcu_node, free_lttng_trigger_ht_element_rcu);
 end:
        lttng_trigger_destroy(trigger_ht_element->trigger);
        call_rcu(&trigger_ht_element->rcu_node, free_lttng_trigger_ht_element_rcu);
 end:

diff --git a/src/common/trigger.c b/src/common/trigger.c
index ead0fc259a0eb09af3211df7038aeda00fb28f72..2ccf0073589a64d3f5fb8fc98bb5daea3b5bb848 100644 (file)
--- a/src/common/trigger.c
+++ b/src/common/trigger.c
@@ -46,13 +46,21 @@ struct lttng_trigger *lttng_trigger_create(
                goto end;
        }
 
                goto end;
        }
 

+       lttng_condition_get(condition);

        trigger->condition = condition;
        trigger->condition = condition;

+
+       lttng_action_get(action);

        trigger->action = action;
 
 end:
        return trigger;
 }
 
        trigger->action = action;
 
 end:
        return trigger;
 }
 

+/*
+ * Note: the lack of reference counting 'get' on the condition object is normal.
+ * This API was exposed as such in 2.11. The client is not expected to call
+ * lttng_condition_destroy on the returned object.
+ */

 struct lttng_condition *lttng_trigger_get_condition(
                struct lttng_trigger *trigger)
 {
 struct lttng_condition *lttng_trigger_get_condition(
                struct lttng_trigger *trigger)
 {


@@ -66,6 +74,12 @@ const struct lttng_condition *lttng_trigger_get_const_condition(
        return trigger->condition;
 }
 
        return trigger->condition;
 }
 

+
+/*
+ * Note: the lack of reference counting 'get' on the action object is normal.
+ * This API was exposed as such in 2.11. The client is not expected to call
+ * lttng_action_destroy on the returned object.
+ */

 struct lttng_action *lttng_trigger_get_action(
                struct lttng_trigger *trigger)
 {
 struct lttng_action *lttng_trigger_get_action(
                struct lttng_trigger *trigger)
 {


@@ -81,10 +95,21 @@ const struct lttng_action *lttng_trigger_get_const_action(
 
 void lttng_trigger_destroy(struct lttng_trigger *trigger)
 {
 
 void lttng_trigger_destroy(struct lttng_trigger *trigger)
 {

+       struct lttng_action *action = lttng_trigger_get_action(trigger);
+       struct lttng_condition *condition =
+                       lttng_trigger_get_condition(trigger);
+

        if (!trigger) {
                return;
        }
 
        if (!trigger) {
                return;
        }
 

+       assert(action);
+       assert(condition);
+
+       /* Release ownership. */
+       lttng_action_put(action);
+       lttng_condition_put(condition);
+

        free(trigger);
 }
 
        free(trigger);
 }
 


@@ -149,12 +174,22 @@ ssize_t lttng_trigger_create_from_payload(
                goto error;
        }
 
                goto error;
        }
 

+       /*
+        * The trigger object owns references to the action and condition
+        * objects.
+        */
+       lttng_condition_put(condition);
+       condition = NULL;
+
+       lttng_action_put(action);
+       action = NULL;
+

        ret = offset;
        ret = offset;

-end:
-       return ret;
+

 error:
        lttng_condition_destroy(condition);
        lttng_action_destroy(action);
 error:
        lttng_condition_destroy(condition);
        lttng_action_destroy(action);

+end:

        return ret;
 }
 
        return ret;
 }