From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Tue, 11 May 2021 18:59:49 +0000 (-0400)
Subject: Fix: Java application context: pass application context argument to callbacks
X-Git-Tag: v2.13.0-rc2~11
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=577f6dfc5dc2cbd4d6d82b453dabca36c4b78d1d;p=lttng-ust.git

Fix: Java application context: pass application context argument to callbacks

The callbacks require the complete application context field names to
compare against the tables received from the TLS area.

We also need to free memory allocated by for the context event field and
context name in a destroy callback.

Therefore, allocate a data structure for each application context, and
use that data structure as private data. This allows callback to reach
the context name, and a destroy callback to free the allocated memory.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ia34fe00f5945fb58ef991f9d40dec36ac9ab322c
---

diff --git a/src/common/ust-context-provider.h b/src/common/ust-context-provider.h
index d8c9a138..43e75b4b 100644
--- a/src/common/ust-context-provider.h
+++ b/src/common/ust-context-provider.h
@@ -67,6 +67,27 @@ struct lttng_ust_context_provider {
 	/* End of base ABI. Fields below should be used after checking struct_size. */
 };
 
+/*
+ * Application context callback private data
+ *
+ * IMPORTANT: this structure is part of the ABI between the probe and
+ * UST. Fields need to be only added at the end, never reordered, never
+ * removed.
+ *
+ * The field @struct_size should be used to determine the size of the
+ * structure. It should be queried before using additional fields added
+ * at the end of the structure.
+ */
+
+struct lttng_ust_app_context {
+	uint32_t struct_size;
+
+	struct lttng_ust_event_field *event_field;
+	char *ctx_name;
+
+	/* End of base ABI. Fields below should be used after checking struct_size. */
+};
+
 /*
  * Returns an opaque pointer on success, which must be passed to
  * lttng_ust_context_provider_unregister for unregistration. Returns
diff --git a/src/lib/lttng-ust-java-agent/jni/common/lttng_ust_context.c b/src/lib/lttng-ust-java-agent/jni/common/lttng_ust_context.c
index 4f4fa9b7..5e6add28 100644
--- a/src/lib/lttng-ust-java-agent/jni/common/lttng_ust_context.c
+++ b/src/lib/lttng-ust-java-agent/jni/common/lttng_ust_context.c
@@ -82,10 +82,10 @@ static struct lttng_ust_jni_ctx_entry *lookup_ctx_by_name(const char *ctx_name)
 static size_t get_size_cb(void *priv, struct lttng_ust_probe_ctx *probe_ctx __attribute__((unused)),
 		size_t offset)
 {
+	const struct lttng_ust_app_context *app_ctx = (const struct lttng_ust_app_context *) priv;
+	const char *ctx_name = app_ctx->ctx_name;
 	struct lttng_ust_jni_ctx_entry *jctx;
 	size_t size = 0;
-	struct lttng_ust_jni_provider *jni_provider = (struct lttng_ust_jni_provider *) priv;
-	const char *ctx_name = jni_provider->name;
 	enum lttng_ust_jni_type jni_type;
 
 	size += lttng_ust_ring_buffer_align(offset, lttng_ust_rb_alignof(char));
@@ -147,9 +147,9 @@ static void record_cb(void *priv,
 		 struct lttng_ust_ring_buffer_ctx *ctx,
 		 struct lttng_ust_channel_buffer *lttng_chan_buf)
 {
+	const struct lttng_ust_app_context *app_ctx = (const struct lttng_ust_app_context *) priv;
+	const char *ctx_name = app_ctx->ctx_name;
 	struct lttng_ust_jni_ctx_entry *jctx;
-	struct lttng_ust_jni_provider *jni_provider = (struct lttng_ust_jni_provider *) priv;
-	const char *ctx_name = jni_provider->name;
 	enum lttng_ust_jni_type jni_type;
 	char sel_char;
 
@@ -252,9 +252,9 @@ static void record_cb(void *priv,
 static void get_value_cb(void *priv, struct lttng_ust_probe_ctx *probe_ctx __attribute__((unused)),
 		struct lttng_ust_ctx_value *value)
 {
-	struct lttng_ust_jni_provider *jni_provider = (struct lttng_ust_jni_provider *) priv;
+	const struct lttng_ust_app_context *app_ctx = (const struct lttng_ust_app_context *) priv;
+	const char *ctx_name = app_ctx->ctx_name;
 	struct lttng_ust_jni_ctx_entry *jctx;
-	const char *ctx_name = jni_provider->name;
 	enum lttng_ust_jni_type jni_type;
 
 	jctx = lookup_ctx_by_name(ctx_name);
diff --git a/src/lib/lttng-ust/context-provider-internal.h b/src/lib/lttng-ust/context-provider-internal.h
index 92de7dff..cf1a1306 100644
--- a/src/lib/lttng-ust/context-provider-internal.h
+++ b/src/lib/lttng-ust/context-provider-internal.h
@@ -17,8 +17,7 @@ void lttng_ust_context_set_event_notifier_group_provider(const char *name,
 			struct lttng_ust_ring_buffer_ctx *ctx,
 			struct lttng_ust_channel_buffer *chan),
 		void (*get_value)(void *priv, struct lttng_ust_probe_ctx *probe_ctx,
-			struct lttng_ust_ctx_value *value),
-		void *priv)
+			struct lttng_ust_ctx_value *value))
 	__attribute__((visibility("hidden")));
 
 #endif /* _LTTNG_UST_CONTEXT_PROVIDER_INTERNAL_H */
diff --git a/src/lib/lttng-ust/events.h b/src/lib/lttng-ust/events.h
index 3ef0dbcb..cfdbd2e0 100644
--- a/src/lib/lttng-ust/events.h
+++ b/src/lib/lttng-ust/events.h
@@ -316,8 +316,7 @@ int lttng_ust_context_set_provider_rcu(struct lttng_ust_ctx **_ctx,
 			struct lttng_ust_ring_buffer_ctx *ctx,
 			struct lttng_ust_channel_buffer *chan),
 		void (*get_value)(void *priv, struct lttng_ust_probe_ctx *probe_ctx,
-			struct lttng_ust_ctx_value *value),
-		void *priv)
+			struct lttng_ust_ctx_value *value))
 	__attribute__((visibility("hidden")));
 
 void lttng_ust_context_set_session_provider(const char *name,
@@ -327,8 +326,7 @@ void lttng_ust_context_set_session_provider(const char *name,
 			struct lttng_ust_ring_buffer_ctx *ctx,
 			struct lttng_ust_channel_buffer *chan),
 		void (*get_value)(void *priv, struct lttng_ust_probe_ctx *probe_ctx,
-			struct lttng_ust_ctx_value *value),
-		void *priv)
+			struct lttng_ust_ctx_value *value))
 	__attribute__((visibility("hidden")));
 
 #endif /* _LTTNG_UST_EVENTS_INTERNAL_H */
diff --git a/src/lib/lttng-ust/lttng-context-provider.c b/src/lib/lttng-ust/lttng-context-provider.c
index b4118122..4e7e429f 100644
--- a/src/lib/lttng-ust/lttng-context-provider.c
+++ b/src/lib/lttng-ust/lttng-context-provider.c
@@ -89,11 +89,11 @@ struct lttng_ust_registered_context_provider *lttng_ust_context_provider_registe
 
 	lttng_ust_context_set_session_provider(provider->name,
 		provider->get_size, provider->record,
-		provider->get_value, provider->priv);
+		provider->get_value);
 
 	lttng_ust_context_set_event_notifier_group_provider(provider->name,
 		provider->get_size, provider->record,
-		provider->get_value, provider->priv);
+		provider->get_value);
 end:
 	ust_unlock();
 	return reg_provider;
@@ -107,11 +107,11 @@ void lttng_ust_context_provider_unregister(struct lttng_ust_registered_context_p
 		goto end;
 	lttng_ust_context_set_session_provider(reg_provider->provider->name,
 		lttng_ust_dummy_get_size, lttng_ust_dummy_record,
-		lttng_ust_dummy_get_value, NULL);
+		lttng_ust_dummy_get_value);
 
 	lttng_ust_context_set_event_notifier_group_provider(reg_provider->provider->name,
 		lttng_ust_dummy_get_size, lttng_ust_dummy_record,
-		lttng_ust_dummy_get_value, NULL);
+		lttng_ust_dummy_get_value);
 
 	cds_hlist_del(&reg_provider->node);
 end:
@@ -119,6 +119,20 @@ end:
 	free(reg_provider);
 }
 
+static
+void app_context_destroy(void *priv)
+{
+	struct lttng_ust_app_context *app_ctx = (struct lttng_ust_app_context *) priv;
+
+	free(app_ctx->ctx_name);
+	free(app_ctx->event_field);
+}
+
+static
+const struct lttng_ust_type_common app_ctx_type = {
+	.type = lttng_ust_type_dynamic,
+};
+
 /*
  * Called with ust mutex held.
  * Add application context to array of context, even if the application
@@ -133,7 +147,7 @@ int lttng_ust_add_app_context_to_ctx_rcu(const char *name,
 	const struct lttng_ust_context_provider *provider;
 	struct lttng_ust_ctx_field new_field = { 0 };
 	struct lttng_ust_event_field *event_field = NULL;
-	struct lttng_ust_type_common *type = NULL;
+	struct lttng_ust_app_context *app_ctx = NULL;
 	char *ctx_name;
 	int ret;
 
@@ -149,14 +163,17 @@ int lttng_ust_add_app_context_to_ctx_rcu(const char *name,
 		ret = -ENOMEM;
 		goto error_field_name_alloc;
 	}
-	type = zmalloc(sizeof(struct lttng_ust_type_common));
-	if (!type) {
+	app_ctx = zmalloc(sizeof(struct lttng_ust_app_context));
+	if (!app_ctx) {
 		ret = -ENOMEM;
-		goto error_field_type_alloc;
+		goto error_app_ctx_alloc;
 	}
+	app_ctx->struct_size = sizeof(struct lttng_ust_app_context);
+	app_ctx->event_field = event_field;
+	app_ctx->ctx_name = ctx_name;
+
 	event_field->name = ctx_name;
-	type->type = lttng_ust_type_dynamic;
-	event_field->type = type;
+	event_field->type = &app_ctx_type;
 	new_field.event_field = event_field;
 	/*
 	 * If provider is not found, we add the context anyway, but
@@ -167,13 +184,13 @@ int lttng_ust_add_app_context_to_ctx_rcu(const char *name,
 		new_field.get_size = provider->get_size;
 		new_field.record = provider->record;
 		new_field.get_value = provider->get_value;
-		new_field.priv = provider->priv;
 	} else {
 		new_field.get_size = lttng_ust_dummy_get_size;
 		new_field.record = lttng_ust_dummy_record;
 		new_field.get_value = lttng_ust_dummy_get_value;
-		new_field.priv = NULL;
 	}
+	new_field.priv = app_ctx;
+	new_field.destroy = app_context_destroy;
 	/*
 	 * For application context, add it by expanding
 	 * ctx array.
@@ -185,8 +202,8 @@ int lttng_ust_add_app_context_to_ctx_rcu(const char *name,
 	return 0;
 
 error_append:
-	free(type);
-error_field_type_alloc:
+	free(app_ctx);
+error_app_ctx_alloc:
 	free(ctx_name);
 error_field_name_alloc:
 	free(event_field);
diff --git a/src/lib/lttng-ust/lttng-context.c b/src/lib/lttng-ust/lttng-context.c
index 427afe56..fa209a2c 100644
--- a/src/lib/lttng-ust/lttng-context.c
+++ b/src/lib/lttng-ust/lttng-context.c
@@ -260,8 +260,7 @@ int lttng_ust_context_set_provider_rcu(struct lttng_ust_ctx **_ctx,
 			struct lttng_ust_ring_buffer_ctx *ctx,
 			struct lttng_ust_channel_buffer *chan),
 		void (*get_value)(void *priv, struct lttng_ust_probe_ctx *probe_ctx,
-			struct lttng_ust_ctx_value *value),
-		void *priv)
+			struct lttng_ust_ctx_value *value))
 {
 	int i, ret;
 	struct lttng_ust_ctx *ctx = *_ctx, *new_ctx;
@@ -291,7 +290,6 @@ int lttng_ust_context_set_provider_rcu(struct lttng_ust_ctx **_ctx,
 		new_fields[i].get_size = get_size;
 		new_fields[i].record = record;
 		new_fields[i].get_value = get_value;
-		new_fields[i].priv = priv;
 	}
 	new_ctx->fields = new_fields;
 	lttng_ust_rcu_assign_pointer(*_ctx, new_ctx);
diff --git a/src/lib/lttng-ust/lttng-events.c b/src/lib/lttng-ust/lttng-events.c
index 26071594..f8170f55 100644
--- a/src/lib/lttng-ust/lttng-events.c
+++ b/src/lib/lttng-ust/lttng-events.c
@@ -2002,8 +2002,7 @@ void lttng_ust_context_set_session_provider(const char *name,
 			struct lttng_ust_ring_buffer_ctx *ctx,
 			struct lttng_ust_channel_buffer *chan),
 		void (*get_value)(void *priv, struct lttng_ust_probe_ctx *probe_ctx,
-			struct lttng_ust_ctx_value *value),
-		void *priv)
+			struct lttng_ust_ctx_value *value))
 {
 	struct lttng_ust_session_private *session_priv;
 
@@ -2013,18 +2012,18 @@ void lttng_ust_context_set_session_provider(const char *name,
 		int ret;
 
 		ret = lttng_ust_context_set_provider_rcu(&session_priv->ctx,
-				name, get_size, record, get_value, priv);
+				name, get_size, record, get_value);
 		if (ret)
 			abort();
 		cds_list_for_each_entry(chan, &session_priv->chan_head, node) {
 			ret = lttng_ust_context_set_provider_rcu(&chan->ctx,
-					name, get_size, record, get_value, priv);
+					name, get_size, record, get_value);
 			if (ret)
 				abort();
 		}
 		cds_list_for_each_entry(event_recorder_priv, &session_priv->events_head, node) {
 			ret = lttng_ust_context_set_provider_rcu(&event_recorder_priv->ctx,
-					name, get_size, record, get_value, priv);
+					name, get_size, record, get_value);
 			if (ret)
 				abort();
 		}
@@ -2045,8 +2044,7 @@ void lttng_ust_context_set_event_notifier_group_provider(const char *name,
 			struct lttng_ust_ring_buffer_ctx *ctx,
 			struct lttng_ust_channel_buffer *chan),
 		void (*get_value)(void *priv, struct lttng_ust_probe_ctx *probe_ctx,
-			struct lttng_ust_ctx_value *value),
-		void *priv)
+			struct lttng_ust_ctx_value *value))
 {
 	struct lttng_event_notifier_group *event_notifier_group;
 
@@ -2055,7 +2053,7 @@ void lttng_ust_context_set_event_notifier_group_provider(const char *name,
 
 		ret = lttng_ust_context_set_provider_rcu(
 				&event_notifier_group->ctx,
-				name, get_size, record, get_value, priv);
+				name, get_size, record, get_value);
 		if (ret)
 			abort();
 	}