ir: trace: pass remove listeners when adding listeners

With this patch, when you call bt_ctf_trace_add_is_static_listener(),
you can also provide a "remove listener" which is called when the added
listener is removed, weither manually with
bt_ctf_trace_remove_is_static_listener(), or when the trace object is
destroyed. This is useful when you dynamically allocate data only for
the listener to add and you are not the only owner of the trace object:
the remove listener is where to free/release this data.

Existing plugins and tests are modified to match and test this change.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/include/babeltrace/ctf-ir/trace-internal.h b/include/babeltrace/ctf-ir/trace-internal.h
index 468b72f0f712c2cc752d65e8a8872594729b5467..f2275fb7e7a7d4d9ef730c58104fbe799f1d61cc 100644 (file)
--- a/include/babeltrace/ctf-ir/trace-internal.h
+++ b/include/babeltrace/ctf-ir/trace-internal.h
@@ -71,6 +71,7 @@ struct bt_ctf_trace {
        GPtrArray *listeners; /* Array of struct listener_wrapper */
        GArray *is_static_listeners;
        bt_bool is_static;
+       bt_bool in_remove_listener;
 };
 
 struct metadata_context {

diff --git a/include/babeltrace/ctf-ir/trace.h b/include/babeltrace/ctf-ir/trace.h
index c7fa1f31b5eb7d0d0798c47f94625f061ecdfc5e..34ba437622313b7a7c1cf1d8436e0a777ba81c11 100644 (file)
--- a/include/babeltrace/ctf-ir/trace.h
+++ b/include/babeltrace/ctf-ir/trace.h
@@ -161,6 +161,20 @@ struct bt_ctf_clock_class;
 typedef void (* bt_ctf_trace_is_static_listener)(
        struct bt_ctf_trace *trace_class, void *data);
 
+/**
+@brief User function type to use with
+       bt_ctf_trace_add_is_static_listener().
+
+@param[in] trace_class Trace class to which the listener was added.
+@param[in] data                User data as passed to
+                       bt_ctf_trace_add_is_static_listener() when
+                       you added the listener.
+
+@prenotnull{trace_class}
+*/
+typedef void (* bt_ctf_trace_listener_removed)(
+       struct bt_ctf_trace *trace_class, void *data);
+
 /**
 @name Creation function
 @{
@@ -873,6 +887,13 @@ extern int bt_ctf_trace_set_is_static(struct bt_ctf_trace *trace_class);
 \p listener is called with \p data, the user data, the first time
 bt_ctf_trace_set_is_static() is called on \p trace_class.
 
+When the trace is destroyed, or when you remove the added listener with
+bt_ctf_trace_remove_is_static_listener(), \p listener_removed is called
+if it's not \c NULL. You can use \p listener_removed to free any dynamic
+data which exists only for the added listener. You cannot call
+any function which modifies \p trace_class during the execution of
+\p listener_removed, including bt_ctf_trace_remove_is_static_listener().
+
 This function fails if \p trace_class is already static: you need to
 check the condition first with bt_ctf_trace_is_static().
 
@@ -881,12 +902,18 @@ listener within \p trace. You can use this identifier to remove the
 specific listener you added with
 bt_ctf_trace_remove_is_static_listener().
 
-@param[in] trace_class Trace class to which to add the listener.
-@param[in] listener    Listener to add to \p trace_class.
-@param[in] data                User data passed when \p listener is called.
-@returns               A unique numeric identifier for this listener
-                       on success (0 or greater), or a negative value
-                       on error.
+@param[in] trace_class         Trace class to which to add the
+                               listener.
+@param[in] listener            Listener to add to \p trace_class.
+@param[in] listener_removed    Remove listener called when \p listener
+                               is removed from \p trace_class, or
+                               \c NULL if you don't need a remove
+                               listener.
+@param[in] data                        User data passed when \p listener or
+                               \p listener_removed is called.
+@returns                       A unique numeric identifier for this
+                               listener on success (0 or greater), or a
+                               negative value on error.
 
 @prenotnull{trace_class}
 @prenotnull{listener}
@@ -902,7 +929,8 @@ bt_ctf_trace_remove_is_static_listener().
 */
 extern int bt_ctf_trace_add_is_static_listener(
                struct bt_ctf_trace *trace_class,
-               bt_ctf_trace_is_static_listener listener, void *data);
+               bt_ctf_trace_is_static_listener listener,
+               bt_ctf_trace_listener_removed listener_removed, void *data);
 
 /**
 @brief  Removes the "trace is static" listener identified by

diff --git a/lib/ctf-ir/trace.c b/lib/ctf-ir/trace.c
index 110c20d4ebff28950373fb12419c22a7db5373a5..9482e097fd56ee9a7cf986a6c9e0c55df9a2f8ad 100644 (file)
--- a/lib/ctf-ir/trace.c
+++ b/lib/ctf-ir/trace.c
@@ -64,6 +64,7 @@ struct listener_wrapper {
 
 struct bt_ctf_trace_is_static_listener_elem {
        bt_ctf_trace_is_static_listener func;
+       bt_ctf_trace_listener_removed removed;
        void *data;
 };
 
@@ -273,6 +274,30 @@ void bt_ctf_trace_destroy(struct bt_object *obj)
        BT_LOGD("Destroying trace object: addr=%p, name=\"%s\"",
                trace, bt_ctf_trace_get_name(trace));
 
+       /*
+        * Call remove listeners first so that everything else still
+        * exists in the trace.
+        */
+       if (trace->is_static_listeners) {
+               size_t i;
+
+               for (i = 0; i < trace->is_static_listeners->len; i++) {
+                       struct bt_ctf_trace_is_static_listener_elem elem =
+                               g_array_index(trace->is_static_listeners,
+                                       struct bt_ctf_trace_is_static_listener_elem, i);
+
+                       if (elem.removed) {
+                               elem.removed(trace, elem.data);
+                       }
+               }
+
+               g_array_free(trace->is_static_listeners, TRUE);
+       }
+
+       if (trace->listeners) {
+               g_ptr_array_free(trace->listeners, TRUE);
+       }
+
        if (trace->environment) {
                BT_LOGD_STR("Destroying environment attributes.");
                bt_ctf_attributes_destroy(trace->environment);
@@ -297,14 +322,6 @@ void bt_ctf_trace_destroy(struct bt_object *obj)
                g_ptr_array_free(trace->stream_classes, TRUE);
        }
 
-       if (trace->listeners) {
-               g_ptr_array_free(trace->listeners, TRUE);
-       }
-
-       if (trace->is_static_listeners) {
-               g_array_free(trace->is_static_listeners, TRUE);
-       }
-
        BT_LOGD_STR("Putting packet header field type.");
        bt_put(trace->packet_header_type);
        g_free(trace);
@@ -2166,11 +2183,13 @@ end:
 }
 
 int bt_ctf_trace_add_is_static_listener(struct bt_ctf_trace *trace,
-               bt_ctf_trace_is_static_listener listener, void *data)
+               bt_ctf_trace_is_static_listener listener,
+               bt_ctf_trace_listener_removed listener_removed, void *data)
 {
        int i;
        struct bt_ctf_trace_is_static_listener_elem new_elem = {
                .func = listener,
+               .removed = listener_removed,
                .data = data,
        };
 
@@ -2194,6 +2213,14 @@ int bt_ctf_trace_add_is_static_listener(struct bt_ctf_trace *trace,
                goto end;
        }
 
+       if (trace->in_remove_listener) {
+               BT_LOGW("Cannot call this function during the execution of a remove listener: "
+                       "addr=%p, name=\"%s\"",
+                       trace, bt_ctf_trace_get_name(trace));
+               i = -1;
+               goto end;
+       }
+
        /* Find the next available spot */
        for (i = 0; i < trace->is_static_listeners->len; i++) {
                struct bt_ctf_trace_is_static_listener_elem elem =
@@ -2232,6 +2259,15 @@ int bt_ctf_trace_remove_is_static_listener(
                goto end;
        }
 
+       if (trace->in_remove_listener) {
+               BT_LOGW("Cannot call this function during the execution of a remove listener: "
+                       "addr=%p, name=\"%s\", listener-id=%d",
+                       trace, bt_ctf_trace_get_name(trace),
+                       listener_id);
+               ret = -1;
+               goto end;
+       }
+
        if (listener_id < 0) {
                BT_LOGW("Invalid listener ID: must be zero or positive: "
                        "listener-id=%d", listener_id);
@@ -2260,7 +2296,19 @@ int bt_ctf_trace_remove_is_static_listener(
                goto end;
        }
 
+       if (elem->removed) {
+               /* Call remove listener */
+               BT_LOGV("Calling remove listener: "
+                       "trace-addr=%p, trace-name=\"%s\", "
+                       "listener-id=%d", trace, bt_ctf_trace_get_name(trace),
+                       listener_id);
+               trace->in_remove_listener = BT_TRUE;
+               elem->removed(trace, elem->data);
+               trace->in_remove_listener = BT_FALSE;
+       }
+
        elem->func = NULL;
+       elem->removed = NULL;
        elem->data = NULL;
        BT_LOGV("Removed \"trace is static\" listener: "
                "trace-addr=%p, trace-name=\"%s\", "

diff --git a/plugins/ctf/fs-sink/write.c b/plugins/ctf/fs-sink/write.c
index 995d344b807d64c37cae0a0f3b6f3dee56d3a4e7..519eb2cb1654560be0785c37b54f3f66df3bc8c2 100644 (file)
--- a/plugins/ctf/fs-sink/write.c
+++ b/plugins/ctf/fs-sink/write.c
@@ -360,7 +360,7 @@ struct fs_writer *insert_new_writer(
                fs_writer->static_listener_id = -1;
        } else {
                ret = bt_ctf_trace_add_is_static_listener(trace,
-                               trace_is_static_listener, fs_writer);
+                               trace_is_static_listener, NULL, fs_writer);
                assert(ret >= 0);
                fs_writer->static_listener_id = ret;
        }

diff --git a/plugins/lttng-utils/copy.c b/plugins/lttng-utils/copy.c
index f99d38315bfe6c79016a81e46ea3397526fd84f0..3bbc0a178c674469c2ff0d66cb42959f66dbcdd6 100644 (file)
--- a/plugins/lttng-utils/copy.c
+++ b/plugins/lttng-utils/copy.c
@@ -753,7 +753,7 @@ struct debug_info_trace *insert_new_trace(struct debug_info_iterator *debug_it,
                bt_ctf_trace_set_is_static(writer_trace);
        } else {
                ret = bt_ctf_trace_add_is_static_listener(trace,
-                               trace_is_static_listener, di_trace);
+                               trace_is_static_listener, NULL, di_trace);
                assert(ret >= 0);
                di_trace->static_listener_id = ret;
        }

diff --git a/tests/lib/test_ctf_writer.c b/tests/lib/test_ctf_writer.c
index 5347afbf30cf2aa0154abca5c280ae7549f53d7c..31fcb6264edfbe14576ec216b540bc2f764dc2e3 100644 (file)
--- a/tests/lib/test_ctf_writer.c
+++ b/tests/lib/test_ctf_writer.c
@@ -60,7 +60,7 @@
 #define DEFAULT_CLOCK_TIME 0
 #define DEFAULT_CLOCK_VALUE 0
 
-#define NR_TESTS 622
+#define NR_TESTS 623
 
 static int64_t current_time = 42;
 static unsigned int packet_resize_test_length = PACKET_RESIZE_TEST_DEF_LENGTH;
@@ -2720,7 +2720,13 @@ void test_static_trace(void)
 static
 void trace_is_static_listener(struct bt_ctf_trace *trace, void *data)
 {
-       *((int *) data) = 1;
+       *((int *) data) |= 1;
+}
+
+static
+void trace_listener_removed(struct bt_ctf_trace *trace, void *data)
+{
+       *((int *) data) |= 2;
 }
 
 static
@@ -2740,18 +2746,19 @@ void test_trace_is_static_listener(void)
        trace = bt_ctf_trace_create();
        assert(trace);
        ret = bt_ctf_trace_add_is_static_listener(NULL,
-               trace_is_static_listener, &called1);
+               trace_is_static_listener, trace_listener_removed, &called1);
        ok(ret < 0, "bt_ctf_trace_add_is_static_listener() handles NULL (trace)");
-       ret = bt_ctf_trace_add_is_static_listener(trace, NULL, &called1);
+       ret = bt_ctf_trace_add_is_static_listener(trace, NULL,
+               trace_listener_removed, &called1);
        ok(ret < 0, "bt_ctf_trace_add_is_static_listener() handles NULL (listener)");
        listener1_id = bt_ctf_trace_add_is_static_listener(trace,
-               trace_is_static_listener, &called1);
+               trace_is_static_listener, trace_listener_removed, &called1);
        ok(listener1_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (1)");
        listener2_id = bt_ctf_trace_add_is_static_listener(trace,
-               trace_is_static_listener, &called2);
+               trace_is_static_listener, trace_listener_removed, &called2);
        ok(listener2_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (2)");
        listener3_id = bt_ctf_trace_add_is_static_listener(trace,
-               trace_is_static_listener, &called3);
+               trace_is_static_listener, trace_listener_removed, &called3);
        ok(listener3_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (3)");
        ret = bt_ctf_trace_remove_is_static_listener(NULL, 0);
        ok(ret < 0, "bt_ctf_trace_remove_is_static_listener() handles NULL (trace)");
@@ -2761,21 +2768,22 @@ void test_trace_is_static_listener(void)
        ok(ret < 0, "bt_ctf_trace_remove_is_static_listener() handles invalid ID (non existing)");
        ret = bt_ctf_trace_remove_is_static_listener(trace, listener2_id);
        ok(ret == 0, "bt_ctf_trace_remove_is_static_listener() succeeds");
+       ok(called2 == 2, "bt_ctf_trace_remove_is_static_listener() calls the remove listener");
        listener4_id = bt_ctf_trace_add_is_static_listener(trace,
-               trace_is_static_listener, &called4);
+               trace_is_static_listener, NULL, &called4);
        ok(listener4_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (4)");
        ok(called1 == 0, "\"trace is static\" listener not called before the trace is made static (1)");
-       ok(called2 == 0, "\"trace is static\" listener not called before the trace is made static (2)");
+       ok(called2 == 2, "\"trace is static\" listener not called before the trace is made static (2)");
        ok(called3 == 0, "\"trace is static\" listener not called before the trace is made static (3)");
        ok(called4 == 0, "\"trace is static\" listener not called before the trace is made static (4)");
        ret = bt_ctf_trace_set_is_static(trace);
        assert(ret == 0);
        ret = bt_ctf_trace_add_is_static_listener(trace,
-               trace_is_static_listener, &called1);
+               trace_is_static_listener, trace_listener_removed, &called1);
        ok(ret < 0,
                "bt_ctf_trace_add_is_static_listener() fails when the trace is static");
        ok(called1 == 1, "\"trace is static\" listener called when the trace is made static (1)");
-       ok(called2 == 0, "\"trace is static\" listener not called when the trace is made static (2)");
+       ok(called2 == 2, "\"trace is static\" listener not called when the trace is made static (2)");
        ok(called3 == 1, "\"trace is static\" listener called when the trace is made static (3)");
        ok(called4 == 1, "\"trace is static\" listener called when the trace is made static (4)");
        called1 = 0;
@@ -2783,9 +2791,9 @@ void test_trace_is_static_listener(void)
        called3 = 0;
        called4 = 0;
        bt_put(trace);
-       ok(called1 == 0, "\"trace is static\" listener not called after the trace is put (1)");
+       ok(called1 == 2, "\"trace is static\" listener not called after the trace is put (1)");
        ok(called2 == 0, "\"trace is static\" listener not called after the trace is put (2)");
-       ok(called3 == 0, "\"trace is static\" listener not called after the trace is put (3)");
+       ok(called3 == 2, "\"trace is static\" listener not called after the trace is put (3)");
        ok(called4 == 0, "\"trace is static\" listener not called after the trace is put (4)");
 }