From: Jonathan Rajotte Date: Mon, 12 Apr 2021 18:45:24 +0000 (-0400) Subject: trigger: keep state of if a trigger is currently registered X-Git-Tag: v2.13.0-rc1~85 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=9c374932629a1c25927dad13fce02ba1c4892cf7;hp=3e68c9e87ed6fd601e953b9bb10e507a6a92a8f0 trigger: keep state of if a trigger is currently registered Since a trigger can be referenced even when is was "unregistered" in other part of lttng-sessiond, namely the action executor queue, we must keep track of the registration state. This will allows us to easily skip any actions to be executed if the associated trigger is "unregistered" at the moment of execution. This is implemented in a following patch. Signed-off-by: Jonathan Rajotte Signed-off-by: Jérémie Galarneau Change-Id: I06c9d437fce975c9c8393e8d1be3e66a24618af3 --- diff --git a/include/lttng/trigger/trigger-internal.h b/include/lttng/trigger/trigger-internal.h index c5ee9c64b..f16b68d00 100644 --- a/include/lttng/trigger/trigger-internal.h +++ b/include/lttng/trigger/trigger-internal.h @@ -8,13 +8,14 @@ #ifndef LTTNG_TRIGGER_INTERNAL_H #define LTTNG_TRIGGER_INTERNAL_H -#include #include #include #include #include -#include +#include +#include #include +#include #include #include @@ -36,6 +37,25 @@ struct lttng_trigger { * notification. */ LTTNG_OPTIONAL(uint64_t) tracer_token; + + /* + * Is the trigger registered? + * + * This is necessary since a reference holder might be interested in the + * overall state of the trigger from the point of view of its owner. + * + * The main user is the action executor since we want to prevent the + * execution of actions related to a trigger that is unregistered. + * + * Not considered for `is_equal`. + */ + bool registered; + + /* + * The lock is used to protect against concurrent trigger execution and + * trigger removal. + */ + pthread_mutex_t lock; }; struct lttng_triggers { @@ -182,4 +202,28 @@ struct lttng_trigger *lttng_trigger_copy(const struct lttng_trigger *trigger); LTTNG_HIDDEN bool lttng_trigger_needs_tracer_notifier(const struct lttng_trigger *trigger); +LTTNG_HIDDEN +void lttng_trigger_set_as_registered(struct lttng_trigger *trigger); + +LTTNG_HIDDEN +void lttng_trigger_set_as_unregistered(struct lttng_trigger *trigger); + +/* + * The trigger must be locked before calling lttng_trigger_is_registered. + * + * The lock is necessary since a trigger can be unregistered at any time. + * + * Manipulations requiring that the trigger be registered must always acquire + * the trigger lock for the duration of the manipulation using + * `lttng_trigger_lock` and `lttng_trigger_unlock`. + */ +LTTNG_HIDDEN +bool lttng_trigger_is_registered(struct lttng_trigger *trigger); + +LTTNG_HIDDEN +void lttng_trigger_lock(struct lttng_trigger *trigger); + +LTTNG_HIDDEN +void lttng_trigger_unlock(struct lttng_trigger *trigger); + #endif /* LTTNG_TRIGGER_INTERNAL_H */ diff --git a/src/bin/lttng-sessiond/notification-thread-events.c b/src/bin/lttng-sessiond/notification-thread-events.c index 29318432f..ec4d56bce 100644 --- a/src/bin/lttng-sessiond/notification-thread-events.c +++ b/src/bin/lttng-sessiond/notification-thread-events.c @@ -2693,6 +2693,9 @@ int handle_notification_thread_command_register_trigger( goto error_free_ht_element; } + /* From this point consider the trigger registered. */ + lttng_trigger_set_as_registered(trigger); + /* * Some triggers might need a tracer notifier depending on its * condition and actions. @@ -2892,6 +2895,11 @@ error_free_ht_element: } error: if (free_trigger) { + /* + * Other objects might have a reference to the trigger, mark it + * as unregistered. + */ + lttng_trigger_set_as_unregistered(trigger); lttng_trigger_destroy(trigger); } end: @@ -2973,6 +2981,12 @@ int handle_notification_thread_command_unregister_trigger( cmd_reply = LTTNG_OK; } + trigger_ht_element = caa_container_of(triggers_ht_node, + struct lttng_trigger_ht_element, node); + + /* From this point, consider the trigger unregistered no matter what. */ + lttng_trigger_set_as_unregistered(trigger_ht_element->trigger); + /* Remove trigger from channel_triggers_ht. */ cds_lfht_for_each_entry(state->channel_triggers_ht, &iter, trigger_list, channel_triggers_ht_node) { @@ -2995,9 +3009,6 @@ int handle_notification_thread_command_unregister_trigger( teardown_tracer_notifier(state, trigger); } - trigger_ht_element = caa_container_of(triggers_ht_node, - struct lttng_trigger_ht_element, node); - if (is_trigger_action_notify(trigger)) { /* * Remove and release the client list from diff --git a/src/common/trigger.c b/src/common/trigger.c index 125c871ea..ec96fc80b 100644 --- a/src/common/trigger.c +++ b/src/common/trigger.c @@ -5,24 +5,24 @@ * */ -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include #include -#include -#include -#include -#include #include +#include #include -#include +#include +#include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include LTTNG_HIDDEN bool lttng_trigger_validate(const struct lttng_trigger *trigger) @@ -68,6 +68,9 @@ struct lttng_trigger *lttng_trigger_create( lttng_action_get(action); trigger->action = action; + pthread_mutex_init(&trigger->lock, NULL); + trigger->registered = false; + end: return trigger; } @@ -121,6 +124,8 @@ static void trigger_destroy_ref(struct urcu_ref *ref) lttng_action_put(action); lttng_condition_put(condition); + pthread_mutex_destroy(&trigger->lock); + free(trigger->name); free(trigger); } @@ -895,3 +900,45 @@ bool lttng_trigger_needs_tracer_notifier(const struct lttng_trigger *trigger) end: return needs_tracer_notifier; } + +LTTNG_HIDDEN +void lttng_trigger_set_as_registered(struct lttng_trigger *trigger) +{ + pthread_mutex_lock(&trigger->lock); + trigger->registered = true; + pthread_mutex_unlock(&trigger->lock); +} + +LTTNG_HIDDEN +void lttng_trigger_set_as_unregistered(struct lttng_trigger *trigger) +{ + pthread_mutex_lock(&trigger->lock); + trigger->registered = false; + pthread_mutex_unlock(&trigger->lock); +} + +/* + * The trigger must be locked before calling lttng_trigger_registered. + * The lock is necessary since a trigger can be unregistered at anytime. + * Manipulations requiring that the trigger be registered must always acquire + * the trigger lock for the duration of the manipulation using + * `lttng_trigger_lock` and `lttng_trigger_unlock`. + */ +LTTNG_HIDDEN +bool lttng_trigger_is_registered(struct lttng_trigger *trigger) +{ + ASSERT_LOCKED(trigger->lock); + return trigger->registered; +} + +LTTNG_HIDDEN +void lttng_trigger_lock(struct lttng_trigger *trigger) +{ + pthread_mutex_lock(&trigger->lock); +} + +LTTNG_HIDDEN +void lttng_trigger_unlock(struct lttng_trigger *trigger) +{ + pthread_mutex_unlock(&trigger->lock); +}