From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Wed, 12 Nov 2014 23:18:32 +0000 (-0500)
Subject: Fix: filter attach vs event enable race
X-Git-Tag: v2.5.3~49
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=15febfee5392a7c56a8c51f9637be8ef87e84759;p=lttng-tools.git

Fix: filter attach vs event enable race

In order to correctly handle the use-case where events are enabled
_after_ trace is started, and _after_ applications are already being
traced, the event should be created in a "disabled" state, so that it
does not trace events until its filter is attached.

This fix needs to be done both in lttng-tools and lttng-ust. In order to
keep ABI compatibility between tools and ust within a stable release
cycle, we introduce a new "disabled" within struct lttng_ust_event
padding (previously zeroed). Newer LTTng-UST checks this flag, and
fallback on the old racy behavior (enabling the event on creation) if it
is unset.

Therefore, old session daemon works with newer lttng-ust of the same
stable release, and vice-versa. However, building lttng-tools requires
an upgraded lttng-ust, which contains the communication protocol with
the new "disabled" field.

This patch should be backported to stable-2.4, stable-2.5, stable-2.6
branches.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
---

diff --git a/src/bin/lttng-sessiond/trace-ust.c b/src/bin/lttng-sessiond/trace-ust.c
index 96225a21b..ddf5dc4ab 100644
--- a/src/bin/lttng-sessiond/trace-ust.c
+++ b/src/bin/lttng-sessiond/trace-ust.c
@@ -393,6 +393,11 @@ struct ltt_ust_event *trace_ust_create_event(struct lttng_event *ev,
 		ERR("Unknown ust loglevel type (%d)", ev->loglevel_type);
 		goto error_free_event;
 	}
+	/*
+	 * Fix for enabler race. Enable is now done explicitly by
+	 * sessiond after setting filter.
+	 */
+	lue->attr.disabled = 1;
 
 	/* Same layout. */
 	lue->filter_expression = filter_expression;
diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index 4a7fadaf5..bffceefd9 100644
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -1446,7 +1446,32 @@ int create_ust_event(struct ust_app *app, struct ust_app_session *ua_sess,
 	}
 
 	/* If event not enabled, disable it on the tracer */
-	if (ua_event->enabled == 0) {
+	if (ua_event->enabled) {
+		/*
+		 * We now need to explicitly enable the event, since it
+		 * is now disabled at creation.
+		 */
+		ret = enable_ust_event(app, ua_sess, ua_event);
+		if (ret < 0) {
+			/*
+			 * If we hit an EPERM, something is wrong with our enable call. If
+			 * we get an EEXIST, there is a problem on the tracer side since we
+			 * just created it.
+			 */
+			switch (ret) {
+			case -LTTNG_UST_ERR_PERM:
+				/* Code flow problem */
+				assert(0);
+			case -LTTNG_UST_ERR_EXIST:
+				/* It's OK for our use case. */
+				ret = 0;
+				break;
+			default:
+				break;
+			}
+			goto error;
+		}
+	} else {
 		ret = disable_ust_event(app, ua_sess, ua_event);
 		if (ret < 0) {
 			/*