Fix: only synchronize application configuration on tracing start

The UST configuration of applications is currently replicated as it is
changed from the ltt_ust_{session, channel, event} data structures to
their ust_app_* equivalent as they are modified.

While this worked correctly for the most part, it caused a problem in
per-PID mode since the buffers would get allocated
(and files created, in applicable tracing modes) even though tracing
was never started during some applications' lifetime.

A previous fix attempt, 0498a00, adressed this problem but
introduced a regression that caused configurations to become
mismatched between the sessiond and applications in cases where a
tracing session was started, stopped, modified, and started again
within the lifetime of a given application.

This change introduces an explicit "synchronize" set of operations
that ensures that a session's channels and events configurations, as
known by the application(s), match those of the session daemon
whenever a session is started.

Signed-off-by: Jérémie Galarneau <[email protected]>

src/bin/lttng-sessiond/channel.c

@@ -311,6 +311,18 @@ int channel_ust_enable(struct ltt_ust_session *usess,
 		DBG3("Channel %s already enabled. Skipping", uchan->name);
 		ret = LTTNG_ERR_UST_CHAN_EXIST;
 		goto end;
+	} else {
+		uchan->enabled = 1;
+		DBG2("Channel %s enabled successfully", uchan->name);
+	}
+
+	if (!usess->active) {
+		/*
+		 * The channel will be activated against the apps
+		 * when the session is started as part of the
+		 * application channel "synchronize" operation.
+		 */
+		goto end;
 	}
 
 	DBG2("Channel %s being enabled in UST domain", uchan->name);
@@ -325,8 +337,6 @@ int channel_ust_enable(struct ltt_ust_session *usess,
 	 */
 	(void) ust_app_enable_channel_glb(usess, uchan);
 
-	uchan->enabled = 1;
-	DBG2("Channel %s enabled successfully", uchan->name);
 
 end:
 	return ret;
@@ -465,11 +475,13 @@ int channel_ust_create(struct ltt_ust_session *usess,
 		goto error_free_chan;
 	}
 
-	/* Enable channel for global domain */
-	ret = ust_app_create_channel_glb(usess, uchan);
-	if (ret < 0 && ret != -LTTNG_UST_ERR_EXIST) {
-		ret = LTTNG_ERR_UST_CHAN_FAIL;
-		goto error_free_chan;
+	if (usess->active) {
+		/* Enable channel for global domain */
+		ret = ust_app_create_channel_glb(usess, uchan);
+		if (ret < 0 && ret != -LTTNG_UST_ERR_EXIST) {
+			ret = LTTNG_ERR_UST_CHAN_FAIL;
+			goto error_free_chan;
+		}
 	}
 
 	/* Adding the channel to the channel hash table. */
@@ -533,6 +545,9 @@ int channel_ust_disable(struct ltt_ust_session *usess,
 		DBG2("Channel UST %s already disabled", uchan->name);
 		goto end;
 	}
+	if (!usess->active) {
+		goto end;
+	}
 
 	DBG2("Channel %s being disabled in UST global domain", uchan->name);
 	/* Disable channel for global domain */

src/bin/lttng-sessiond/context.c

@@ -169,18 +169,19 @@ static int add_uctx_to_channel(struct ltt_ust_session *usess,
 		goto error;
 	}
 
-	ret = ust_app_add_ctx_channel_glb(usess, uchan, uctx);
-	if (ret < 0) {
-		goto error;
-	}
-
-	rcu_read_lock();
-
 	/* Add ltt UST context node to ltt UST channel */
 	lttng_ht_add_ulong(uchan->ctx, &uctx->node);
-	rcu_read_unlock();
 	cds_list_add_tail(&uctx->list, &uchan->ctx_list);
 
+	if (!usess->active) {
+		goto end;
+	}
+
+	ret = ust_app_add_ctx_channel_glb(usess, uchan, uctx);
+	if (ret < 0) {
+		goto error;
+	}
+end:
 	DBG("Context UST %d added to channel %s", uctx->ctx.ctx, uchan->name);
 
 	return 0;

src/bin/lttng-sessiond/dispatch.c

@@ -61,7 +61,7 @@ static void update_ust_app(int app_sock)
 			continue;
 		}
 		session_lock(sess);
-		if (!sess->ust_session) {
+		if (!sess->active || !sess->ust_session) {
 			goto unlock_session;
 		}
 

src/bin/lttng-sessiond/event.c

@@ -162,7 +162,7 @@ int event_ust_enable_tracepoint(struct ltt_ust_session *usess,
 		struct lttng_event_exclusion *exclusion,
 		bool internal_event)
 {
-	int ret, to_create = 0;
+	int ret = LTTNG_OK, to_create = 0;
 	struct ltt_ust_event *uevent;
 
 	assert(usess);
@@ -195,6 +195,14 @@ int event_ust_enable_tracepoint(struct ltt_ust_session *usess,
 	}
 
 	uevent->enabled = 1;
+	if (to_create) {
+		/* Add ltt ust event to channel */
+		add_unique_ust_event(uchan->events, uevent);
+	}
+
+	if (!usess->active) {
+		goto end;
+	}
 
 	if (to_create) {
 		/* Create event on all UST registered apps for session */
@@ -214,11 +222,6 @@ int event_ust_enable_tracepoint(struct ltt_ust_session *usess,
 		}
 	}
 
-	if (to_create) {
-		/* Add ltt ust event to channel */
-		add_unique_ust_event(uchan->events, uevent);
-	}
-
 	DBG("Event UST %s %s in channel %s", uevent->attr.name,
 			to_create ? "created" : "enabled", uchan->name);
 
@@ -291,17 +294,18 @@ int event_ust_disable_tracepoint(struct ltt_ust_session *usess,
 			/* It's already disabled so everything is OK */
 			goto next;
 		}
+		uevent->enabled = 0;
+		DBG2("Event UST %s disabled in channel %s", uevent->attr.name,
+				uchan->name);
 
+		if (!usess->active) {
+			goto next;
+		}
 		ret = ust_app_disable_event_glb(usess, uchan, uevent);
 		if (ret < 0 && ret != -LTTNG_UST_ERR_EXIST) {
 			ret = LTTNG_ERR_UST_DISABLE_FAIL;
 			goto error;
 		}
-		uevent->enabled = 0;
-
-		DBG2("Event UST %s disabled in channel %s", uevent->attr.name,
-				uchan->name);
-
 next:
 		/* Get next duplicate event by name. */
 		cds_lfht_next_duplicate(ht->ht, trace_ust_ht_match_event_by_name,
@@ -506,18 +510,19 @@ int event_agent_enable(struct ltt_ust_session *usess,
 		created = 1;
 	}
 
-	/* Already enabled? */
-	if (aevent->enabled) {
-		goto end;
-	}
-
 	if (created && filter) {
 		ret = add_filter_app_ctx(filter, filter_expression, agt);
 		if (ret != LTTNG_OK) {
 			goto error;
 		}
 	}
 
+	/* Already enabled? */
+	if (aevent->enabled) {
+		ret = LTTNG_OK;
+		goto end;
+	}
+
 	ret = agent_enable_event(aevent, agt->domain);
 	if (ret != LTTNG_OK) {
 		goto error;
@@ -629,10 +634,12 @@ static int event_agent_disable_one(struct ltt_ust_session *usess,
 	/* If the agent event exists, it must be available on the UST side. */
 	assert(uevent);
 
-	ret = ust_app_disable_event_glb(usess, uchan, uevent);
-	if (ret < 0 && ret != -LTTNG_UST_ERR_EXIST) {
-		ret = LTTNG_ERR_UST_DISABLE_FAIL;
-		goto error;
+	if (usess->active) {
+		ret = ust_app_disable_event_glb(usess, uchan, uevent);
+		if (ret < 0 && ret != -LTTNG_UST_ERR_EXIST) {
+			ret = LTTNG_ERR_UST_DISABLE_FAIL;
+			goto error;
+		}
 	}
 
 	/*

src/bin/lttng-sessiond/trace-ust.c

@@ -827,13 +827,14 @@ int trace_ust_pid_tracker_lookup(struct ltt_ust_session *session, int pid)
 int trace_ust_track_pid(struct ltt_ust_session *session, int pid)
 {
 	int retval = LTTNG_OK;
+	bool should_update_apps = false;
 
 	if (pid == -1) {
 		/* Track all pids: destroy tracker if exists. */
 		if (session->pid_tracker.ht) {
 			fini_pid_tracker(&session->pid_tracker);
 			/* Ensure all apps have session. */
-			ust_app_global_update_all(session);
+			should_update_apps = true;
 		}
 	} else {
 		int ret;
@@ -852,7 +853,7 @@ int trace_ust_track_pid(struct ltt_ust_session *session, int pid)
 				goto end;
 			}
 			/* Remove all apps from session except pid. */
-			ust_app_global_update_all(session);
+			should_update_apps = true;
 		} else {
 			struct ust_app *app;
 
@@ -864,10 +865,13 @@ int trace_ust_track_pid(struct ltt_ust_session *session, int pid)
 			/* Add session to application */
 			app = ust_app_find_by_pid(pid);
 			if (app) {
-				ust_app_global_update(session, app);
+				should_update_apps = true;
 			}
 		}
 	}
+	if (should_update_apps && session->active) {
+		ust_app_global_update_all(session);
+	}
 end:
 	return retval;
 }