lib: graph API: remove "listener removed" callback parameters

Before this patch, when you add a "port added" listener to a graph,
you can pass a callback which gets called when the listener is removed.
This only happens when the graph is destroyed.

This "listener removed" callback feature seems unnecessary as the graph
user, who adds the "port added" listener, has total control over the
graph object. Therefore she can ensure that anything needed by her "port
added" listeners exists as long as the graph exists.

Therefore this patch removes those parameters and everything related.

In Python, we used to keep a strong reference on the partial Python
object (listener's data) and release it when our "listener removed"
function was called. Now, the listener's data is a weak reference, but
we keep a list of strong partial references within the graph object
itself (`self._listener_partials`) to ensure that the partials exist as
long as the graph exists.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I4c06ff139740f887ae2ace7633d2edeb01fd2fa0
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2637
Tested-by: jenkins <jenkins@lttng.org>

diff --git a/include/babeltrace2/graph/graph.h b/include/babeltrace2/graph/graph.h
index f66b953887a87400875d37d8194cfdd0bb077cea..0a7484e475a854a9122e551fd9e32aa68824be57 100644 (file)
--- a/include/babeltrace2/graph/graph.h
+++ b/include/babeltrace2/graph/graph.h
@@ -89,8 +89,6 @@ typedef bt_graph_listener_func_status
                const bt_port_output *upstream_port,
                const bt_port_input *downstream_port, void *data);
 
-typedef void (* bt_graph_listener_removed_func)(void *data);
-
 typedef enum bt_graph_simple_sink_component_initialize_func_status {
        BT_GRAPH_SIMPLE_SINK_COMPONENT_INITIALIZE_FUNC_STATUS_OK                = __BT_FUNC_STATUS_OK,
        BT_GRAPH_SIMPLE_SINK_COMPONENT_INITIALIZE_FUNC_STATUS_ERROR             = __BT_FUNC_STATUS_ERROR,
@@ -214,29 +212,25 @@ extern bt_graph_add_listener_status
 bt_graph_add_filter_component_input_port_added_listener(
                bt_graph *graph,
                bt_graph_filter_component_input_port_added_listener_func listener,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *listener_id);
+               void *data, bt_listener_id *listener_id);
 
 extern bt_graph_add_listener_status
 bt_graph_add_sink_component_input_port_added_listener(
                bt_graph *graph,
                bt_graph_sink_component_input_port_added_listener_func listener,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *listener_id);
+               void *data, bt_listener_id *listener_id);
 
 extern bt_graph_add_listener_status
 bt_graph_add_source_component_output_port_added_listener(
                bt_graph *graph,
                bt_graph_source_component_output_port_added_listener_func listener,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *listener_id);
+               void *data, bt_listener_id *listener_id);
 
 extern bt_graph_add_listener_status
 bt_graph_add_filter_component_output_port_added_listener(
                bt_graph *graph,
                bt_graph_filter_component_output_port_added_listener_func listener,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *listener_id);
+               void *data, bt_listener_id *listener_id);
 
 typedef enum bt_graph_add_interrupter_status {
        BT_GRAPH_ADD_INTERRUPTER_STATUS_OK              = __BT_FUNC_STATUS_OK,

diff --git a/src/bindings/python/bt2/bt2/graph.py b/src/bindings/python/bt2/bt2/graph.py
index b1d59700c8cc100c50d3c28809d82edf7495cf04..a3926e5951ff14df902f47c5b5203281bad27cbd 100644 (file)
--- a/src/bindings/python/bt2/bt2/graph.py
+++ b/src/bindings/python/bt2/bt2/graph.py
@@ -57,6 +57,10 @@ class Graph(object._SharedObject):
 
         super().__init__(ptr)
 
+        # list of listener partials to keep a reference as long as
+        # this graph exists
+        self._listener_partials = []
+
     def add_component(
         self,
         component_class,
@@ -140,6 +144,9 @@ class Graph(object._SharedObject):
         if listener_ids is None:
             raise bt2._Error('cannot add listener to graph object')
 
+        # keep the partial's reference
+        self._listener_partials.append(listener_from_native)
+
     def run_once(self):
         status = native_bt.graph_run_once(self._ptr)
         utils._handle_func_status(status, 'graph object could not run once')

diff --git a/src/bindings/python/bt2/bt2/native_bt_graph.i.h b/src/bindings/python/bt2/bt2/native_bt_graph.i.h
index e45fea610df5a886b186c67bb90ab799eedd7704..74684b826839b9858ed29909a4a89cda324f1183 100644 (file)
--- a/src/bindings/python/bt2/bt2/native_bt_graph.i.h
+++ b/src/bindings/python/bt2/bt2/native_bt_graph.i.h
@@ -22,13 +22,6 @@
  * THE SOFTWARE.
  */
 
-static
-void graph_listener_removed(void *py_callable)
-{
-       BT_ASSERT(py_callable);
-       Py_DECREF(py_callable);
-}
-
 static bt_graph_listener_func_status port_added_listener(
        const void *component,
        swig_type_info *component_swig_type,
@@ -145,7 +138,7 @@ PyObject *bt_bt2_graph_add_port_added_listener(struct bt_graph *graph,
        /* source output port */
        status = bt_graph_add_source_component_output_port_added_listener(
                graph, source_component_output_port_added_listener,
-               graph_listener_removed, py_callable, &listener_id);
+               py_callable, &listener_id);
        if (status != __BT_FUNC_STATUS_OK) {
                /*
                 * bt_graph_add_source_component_output_port_added_listener has
@@ -167,7 +160,7 @@ PyObject *bt_bt2_graph_add_port_added_listener(struct bt_graph *graph,
        /* filter input port */
        status = bt_graph_add_filter_component_input_port_added_listener(
                graph, filter_component_input_port_added_listener,
-               graph_listener_removed, py_callable, &listener_id);
+               py_callable, &listener_id);
        if (status != __BT_FUNC_STATUS_OK) {
                /*
                 * bt_graph_add_filter_component_input_port_added_listener has
@@ -189,7 +182,7 @@ PyObject *bt_bt2_graph_add_port_added_listener(struct bt_graph *graph,
        /* filter output port */
        status = bt_graph_add_filter_component_output_port_added_listener(
                graph, filter_component_output_port_added_listener,
-               graph_listener_removed, py_callable, &listener_id);
+               py_callable, &listener_id);
        if (status != __BT_FUNC_STATUS_OK) {
                /*
                 * bt_graph_add_filter_component_output_port_added_listener has
@@ -211,7 +204,7 @@ PyObject *bt_bt2_graph_add_port_added_listener(struct bt_graph *graph,
        /* sink input port */
        status = bt_graph_add_sink_component_input_port_added_listener(
                graph, sink_component_input_port_added_listener,
-               graph_listener_removed, py_callable, &listener_id);
+               py_callable, &listener_id);
        if (status != __BT_FUNC_STATUS_OK) {
                /*
                 * bt_graph_add_sink_component_input_port_added_listener has
@@ -229,12 +222,6 @@ PyObject *bt_bt2_graph_add_port_added_listener(struct bt_graph *graph,
 
        PyTuple_SET_ITEM(py_listener_ids, 3, py_listener_id);
        py_listener_id = NULL;
-
-       Py_INCREF(py_callable);
-       Py_INCREF(py_callable);
-       Py_INCREF(py_callable);
-       Py_INCREF(py_callable);
-
        goto end;
 
 error:

diff --git a/src/cli/babeltrace2.c b/src/cli/babeltrace2.c
index ad211fa24f2b47e2d4d794a8b7d60f26f45c5442..ad8ec4b711d3593dbd2ae36252d910795ca8a73d 100644 (file)
--- a/src/cli/babeltrace2.c
+++ b/src/cli/babeltrace2.c
@@ -1899,7 +1899,7 @@ int cmd_run_ctx_init(struct cmd_run_ctx *ctx, struct bt_config *cfg)
 
        bt_graph_add_interrupter(ctx->graph, the_interrupter);
        add_listener_status = bt_graph_add_source_component_output_port_added_listener(
-               ctx->graph, graph_source_output_port_added_listener, NULL, ctx,
+               ctx->graph, graph_source_output_port_added_listener, ctx,
                NULL);
        if (add_listener_status != BT_GRAPH_ADD_LISTENER_STATUS_OK) {
                BT_CLI_LOGE_APPEND_CAUSE(
@@ -1908,7 +1908,7 @@ int cmd_run_ctx_init(struct cmd_run_ctx *ctx, struct bt_config *cfg)
        }
 
        add_listener_status = bt_graph_add_filter_component_output_port_added_listener(
-               ctx->graph, graph_filter_output_port_added_listener, NULL, ctx,
+               ctx->graph, graph_filter_output_port_added_listener, ctx,
                NULL);
        if (add_listener_status != BT_GRAPH_ADD_LISTENER_STATUS_OK) {
                BT_CLI_LOGE_APPEND_CAUSE(

diff --git a/src/lib/graph/graph.c b/src/lib/graph/graph.c
index 8416b1f6bbe1c6c06d788463d616fdbdd14e040b..70d00c3de1d70dd03d20576b8868cf4acd3d5a44 100644 (file)
--- a/src/lib/graph/graph.c
+++ b/src/lib/graph/graph.c
@@ -58,14 +58,9 @@ typedef enum bt_graph_listener_func_status
 typedef enum bt_component_class_initialize_method_status
 (*comp_init_method_t)(const void *, void *, const void *, void *);
 
-struct bt_graph_listener {
-       bt_graph_listener_removed_func removed;
-       void *data;
-};
-
 struct bt_graph_listener_port_added {
-       struct bt_graph_listener base;
        port_added_func_t func;
+       void *data;
 };
 
 #define INIT_LISTENERS_ARRAY(_type, _listeners)                                \
@@ -77,23 +72,6 @@ struct bt_graph_listener_port_added {
                }                                                       \
        } while (0)
 
-#define CALL_REMOVE_LISTENERS(_type, _listeners)                       \
-       do {                                                            \
-               size_t i;                                               \
-                                                                       \
-               if (!_listeners) {                                      \
-                       break;                                          \
-               }                                                       \
-               for (i = 0; i < (_listeners)->len; i++) {               \
-                       _type *listener =                               \
-                               &g_array_index((_listeners), _type, i); \
-                                                                       \
-                       if (listener->base.removed) {                   \
-                               listener->base.removed(listener->base.data); \
-                       }                                               \
-               }                                                       \
-       } while (0)
-
 static
 void destroy_graph(struct bt_object *obj)
 {
@@ -129,16 +107,6 @@ void destroy_graph(struct bt_object *obj)
        obj->ref_count++;
        graph->config_state = BT_GRAPH_CONFIGURATION_STATE_DESTROYING;
 
-       /* Call all remove listeners */
-       CALL_REMOVE_LISTENERS(struct bt_graph_listener_port_added,
-               graph->listeners.source_output_port_added);
-       CALL_REMOVE_LISTENERS(struct bt_graph_listener_port_added,
-               graph->listeners.filter_output_port_added);
-       CALL_REMOVE_LISTENERS(struct bt_graph_listener_port_added,
-               graph->listeners.filter_input_port_added);
-       CALL_REMOVE_LISTENERS(struct bt_graph_listener_port_added,
-               graph->listeners.sink_input_port_added);
-
        if (graph->messages) {
                g_ptr_array_free(graph->messages, TRUE);
                graph->messages = NULL;
@@ -710,25 +678,17 @@ enum bt_graph_add_listener_status
 bt_graph_add_source_component_output_port_added_listener(
                struct bt_graph *graph,
                bt_graph_source_component_output_port_added_listener_func func,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *out_listener_id)
+               void *data, bt_listener_id *out_listener_id)
 {
        struct bt_graph_listener_port_added listener = {
-               .base = {
-                       .removed = listener_removed,
-                       .data = data,
-               },
                .func = (port_added_func_t) func,
+               .data = data,
        };
        bt_listener_id listener_id;
 
        BT_ASSERT_PRE_NO_ERROR();
        BT_ASSERT_PRE_NON_NULL(graph, "Graph");
        BT_ASSERT_PRE_NON_NULL(func, "Listener");
-       BT_ASSERT_PRE_NON_NULL(func, "\"Listener removed\" listener");
-       BT_ASSERT_PRE(!graph->in_remove_listener,
-               "Graph currently executing a \"listener removed\" listener: "
-               "%!+g", graph);
        g_array_append_val(graph->listeners.source_output_port_added, listener);
        listener_id = graph->listeners.source_output_port_added->len - 1;
        BT_LIB_LOGD("Added \"source component output port added\" listener to graph: "
@@ -746,25 +706,17 @@ enum bt_graph_add_listener_status
 bt_graph_add_filter_component_output_port_added_listener(
                struct bt_graph *graph,
                bt_graph_filter_component_output_port_added_listener_func func,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *out_listener_id)
+               void *data, bt_listener_id *out_listener_id)
 {
        struct bt_graph_listener_port_added listener = {
-               .base = {
-                       .removed = listener_removed,
-                       .data = data,
-               },
                .func = (port_added_func_t) func,
+               .data = data,
        };
        bt_listener_id listener_id;
 
        BT_ASSERT_PRE_NO_ERROR();
        BT_ASSERT_PRE_NON_NULL(graph, "Graph");
        BT_ASSERT_PRE_NON_NULL(func, "Listener");
-       BT_ASSERT_PRE_NON_NULL(func, "\"Listener removed\" listener");
-       BT_ASSERT_PRE(!graph->in_remove_listener,
-               "Graph currently executing a \"listener removed\" listener: "
-               "%!+g", graph);
        g_array_append_val(graph->listeners.filter_output_port_added, listener);
        listener_id = graph->listeners.filter_output_port_added->len - 1;
        BT_LIB_LOGD("Added \"filter component output port added\" listener to graph: "
@@ -782,25 +734,17 @@ enum bt_graph_add_listener_status
 bt_graph_add_filter_component_input_port_added_listener(
                struct bt_graph *graph,
                bt_graph_filter_component_input_port_added_listener_func func,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *out_listener_id)
+               void *data, bt_listener_id *out_listener_id)
 {
        struct bt_graph_listener_port_added listener = {
-               .base = {
-                       .removed = listener_removed,
-                       .data = data,
-               },
                .func = (port_added_func_t) func,
+               .data = data,
        };
        bt_listener_id listener_id;
 
        BT_ASSERT_PRE_NO_ERROR();
        BT_ASSERT_PRE_NON_NULL(graph, "Graph");
        BT_ASSERT_PRE_NON_NULL(func, "Listener");
-       BT_ASSERT_PRE_NON_NULL(func, "\"Listener removed\" listener");
-       BT_ASSERT_PRE(!graph->in_remove_listener,
-               "Graph currently executing a \"listener removed\" listener: "
-               "%!+g", graph);
        g_array_append_val(graph->listeners.filter_input_port_added, listener);
        listener_id = graph->listeners.filter_input_port_added->len - 1;
        BT_LIB_LOGD("Added \"filter component input port added\" listener to graph: "
@@ -818,25 +762,17 @@ enum bt_graph_add_listener_status
 bt_graph_add_sink_component_input_port_added_listener(
                struct bt_graph *graph,
                bt_graph_sink_component_input_port_added_listener_func func,
-               bt_graph_listener_removed_func listener_removed, void *data,
-               bt_listener_id *out_listener_id)
+               void *data, bt_listener_id *out_listener_id)
 {
        struct bt_graph_listener_port_added listener = {
-               .base = {
-                       .removed = listener_removed,
-                       .data = data,
-               },
                .func = (port_added_func_t) func,
+               .data = data,
        };
        bt_listener_id listener_id;
 
        BT_ASSERT_PRE_NO_ERROR();
        BT_ASSERT_PRE_NON_NULL(graph, "Graph");
        BT_ASSERT_PRE_NON_NULL(func, "Listener");
-       BT_ASSERT_PRE_NON_NULL(func, "\"Listener removed\" listener");
-       BT_ASSERT_PRE(!graph->in_remove_listener,
-               "Graph currently executing a \"listener removed\" listener: "
-               "%!+g", graph);
        g_array_append_val(graph->listeners.sink_input_port_added, listener);
        listener_id = graph->listeners.sink_input_port_added->len - 1;
        BT_LIB_LOGD("Added \"sink component input port added\" listener to graph: "
@@ -917,7 +853,7 @@ enum bt_graph_listener_func_status bt_graph_notify_port_added(
 
 
                BT_ASSERT(listener->func);
-               status = listener->func(comp, port, listener->base.data);
+               status = listener->func(comp, port, listener->data);
                BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(status);
                if (status != BT_FUNC_STATUS_OK) {
                        goto end;

diff --git a/src/lib/graph/graph.h b/src/lib/graph/graph.h
index 672cb7e38518a463b4d9d690844fbf0fd3a0df90..dcc15260c6a2daef4f82a8d02c083c9ee794cffb 100644 (file)
--- a/src/lib/graph/graph.h
+++ b/src/lib/graph/graph.h
@@ -106,7 +106,6 @@ struct bt_graph {
         */
        struct bt_interrupter *default_interrupter;
 
-       bool in_remove_listener;
        bool has_sink;
 
        /*

diff --git a/tests/lib/test_graph_topo.c b/tests/lib/test_graph_topo.c
index 184528d1a1a7b6b8fa09a7ead957e1258ebf6069..91e12760f12b503b6c203c5ee88e8b50698366aa 100644 (file)
--- a/tests/lib/test_graph_topo.c
+++ b/tests/lib/test_graph_topo.c
@@ -401,10 +401,10 @@ bt_graph *create_graph(void)
 
        BT_ASSERT(graph);
        ret = bt_graph_add_source_component_output_port_added_listener(
-               graph, graph_src_output_port_added, NULL, NULL, NULL);
+               graph, graph_src_output_port_added, NULL, NULL);
        BT_ASSERT(ret >= 0);
        ret = bt_graph_add_sink_component_input_port_added_listener(
-               graph, graph_sink_input_port_added, NULL, NULL, NULL);
+               graph, graph_sink_input_port_added, NULL, NULL);
        BT_ASSERT(ret >= 0);
        return graph;
 }