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.

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

diff --git a/include/lttng/ust-abi.h b/include/lttng/ust-abi.h
index 8c2469e42a2a7194449e796f57095718d0db0232..797a06b6e5e33d1e94552bdb9dffdf4fd0ac9df2 100644 (file)
--- a/include/lttng/ust-abi.h
+++ b/include/lttng/ust-abi.h
@@ -98,7 +98,7 @@ struct lttng_ust_stream {
         */
 } LTTNG_PACKED;
 
-#define LTTNG_UST_EVENT_PADDING1       16
+#define LTTNG_UST_EVENT_PADDING1       15
 #define LTTNG_UST_EVENT_PADDING2       (LTTNG_UST_SYM_NAME_LEN + 32)
 struct lttng_ust_event {
        enum lttng_ust_instrumentation instrumentation;
@@ -106,6 +106,7 @@ struct lttng_ust_event {
 
        enum lttng_ust_loglevel_type loglevel_type;
        int loglevel;   /* value, -1: all */
+       char disabled;
        char padding[LTTNG_UST_EVENT_PADDING1];
 
        /* Per instrumentation type configuration */

diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
index 49ae3e6cf94dfc12f6ae32c176841d8a8e71cc79..195f29c03cd211eac3a3fc354e56cd88a574dc4a 100644 (file)
--- a/liblttng-ust-ctl/ustctl.c
+++ b/liblttng-ust-ctl/ustctl.c
@@ -200,6 +200,7 @@ int ustctl_create_event(int sock, struct lttng_ust_event *ev,
        lum.u.event.instrumentation = ev->instrumentation;
        lum.u.event.loglevel_type = ev->loglevel_type;
        lum.u.event.loglevel = ev->loglevel;
+       lum.u.event.disabled = ev->disabled;
        ret = ustcomm_send_app_cmd(sock, &lum, &lur);
        if (ret) {
                free(event_data);

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 285b98e8d501181241c65fe5a46bff61a68b7782..8bdb743d93da21c0f8f1af62dcab2018956fa876 100644 (file)
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -746,7 +746,16 @@ struct lttng_enabler *lttng_enabler_create(enum lttng_enabler_type type,
                sizeof(enabler->event_param));
        enabler->chan = chan;
        /* ctx left NULL */
-       enabler->enabled = 1;
+       /*
+        * The "disable" event create comm field has been added to fix a
+        * race between event creation (of a started trace) and enabling
+        * filtering. New session daemon always set the "disable" field
+        * to 1, and are aware that they need to explicitly enable the
+        * event. Older session daemon (within same ABI) leave it at 0,
+        * and therefore we need to enable it here, keeping the original
+        * racy behavior.
+        */
+       enabler->enabled = !event_param->disabled;
        cds_list_add(&enabler->node, &enabler->chan->session->enablers_head);
        lttng_session_lazy_sync_enablers(enabler->chan->session);
        return enabler;