bt2: free port user data when finalizing components

When creating component ports in Python, it is possible to pass a Python
object as user data:

    class MySource(
        bt2._UserSourceComponent, message_iterator_class=MyIter
    ):
        def __init__(self, config, params, obj):
            self._add_output_port('out', MyUserData())

The port takes a reference to this Python object, thanks to the `void *`
typemap in native_bt_port.i:

    %typemap(out) void * {
            Py_INCREF($1);
            $result = $1;
    }

However, this reference is never released.

This patches makes it so that when a component is finalized, it releases
the reference for the user data of all of its ports.

Change-Id: Ifebecc3b242c2ccf2bd65347a9087b90093f286c
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2734
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/src/bindings/python/bt2/bt2/native_bt_component_class.i.h b/src/bindings/python/bt2/bt2/native_bt_component_class.i.h
index f830afa3465cf5449309ba3e1b1d18de07b460be..6ea021dee6adf02fd86f2bb6bdc86e8adde9b795 100644 (file)
--- a/src/bindings/python/bt2/bt2/native_bt_component_class.i.h
+++ b/src/bindings/python/bt2/bt2/native_bt_component_class.i.h
@@ -524,25 +524,126 @@ end:
        Py_DECREF(py_comp);
 }
 
+/* Decref the Python object in the user data associated to `port`. */
+
+static
+void delete_port_user_data(bt_self_component_port *port)
+{
+       Py_DECREF(bt_self_component_port_get_data(port));
+}
+
+static
+void delete_port_input_user_data(bt_self_component_port_input *port_input)
+{
+       bt_self_component_port *port =
+               bt_self_component_port_input_as_self_component_port(port_input);
+
+       delete_port_user_data(port);
+}
+
+static
+void delete_port_output_user_data(bt_self_component_port_output *port_output)
+{
+       bt_self_component_port *port =
+               bt_self_component_port_output_as_self_component_port(port_output);
+
+       delete_port_user_data(port);
+}
+
 static
 void component_class_source_finalize(bt_self_component_source *self_component_source)
 {
-       bt_self_component *self_component = bt_self_component_source_as_self_component(self_component_source);
+       uint64_t i;
+       bt_self_component *self_component;
+       const bt_component_source *component_source;
+
+       self_component = bt_self_component_source_as_self_component(
+               self_component_source);
+       component_source = bt_self_component_source_as_component_source(
+               self_component_source);
+
        component_class_finalize(self_component);
+
+       /*
+        * Free the user data Python object attached to the port.  The
+        * corresponding incref was done by the `void *` typemap in
+        * native_bt_port.i.
+        */
+       for (i = 0; i < bt_component_source_get_output_port_count(component_source); i++) {
+               bt_self_component_port_output *port_output;
+
+               port_output = bt_self_component_source_borrow_output_port_by_index(
+                       self_component_source, i);
+
+               delete_port_output_user_data(port_output);
+       }
 }
 
 static
 void component_class_filter_finalize(bt_self_component_filter *self_component_filter)
 {
-       bt_self_component *self_component = bt_self_component_filter_as_self_component(self_component_filter);
+       uint64_t i;
+       bt_self_component *self_component;
+       const bt_component_filter *component_filter;
+
+       self_component = bt_self_component_filter_as_self_component(
+               self_component_filter);
+       component_filter = bt_self_component_filter_as_component_filter(
+               self_component_filter);
+
        component_class_finalize(self_component);
+
+       /*
+        * Free the user data Python object attached to the port.  The
+        * corresponding incref was done by the `void *` typemap in
+        * native_bt_port.i.
+        */
+       for (i = 0; i < bt_component_filter_get_input_port_count(component_filter); i++) {
+               bt_self_component_port_input *port_input;
+
+               port_input = bt_self_component_filter_borrow_input_port_by_index(
+                       self_component_filter, i);
+
+               delete_port_input_user_data(port_input);
+       }
+
+       for (i = 0; i < bt_component_filter_get_output_port_count(component_filter); i++) {
+               bt_self_component_port_output *port_output;
+
+               port_output = bt_self_component_filter_borrow_output_port_by_index(
+                       self_component_filter, i);
+
+               delete_port_output_user_data(port_output);
+       }
 }
 
 static
 void component_class_sink_finalize(bt_self_component_sink *self_component_sink)
 {
-       bt_self_component *self_component = bt_self_component_sink_as_self_component(self_component_sink);
+       uint64_t i;
+       bt_self_component *self_component;
+       const bt_component_sink *component_sink;
+
+       self_component = bt_self_component_sink_as_self_component(
+               self_component_sink);
+       component_sink = bt_self_component_sink_as_component_sink(
+               self_component_sink);
+
        component_class_finalize(self_component);
+
+       /*
+        * Free the user data Python object attached to the port.  The
+        * corresponding incref was done by the `void *` typemap in
+        * native_bt_port.i.
+        */
+       for (i = 0; i < bt_component_sink_get_input_port_count(component_sink); i++) {
+               bt_self_component_port_input *port_input;
+
+               port_input = bt_self_component_sink_borrow_input_port_by_index(
+                       self_component_sink, i);
+
+               delete_port_input_user_data(port_input);
+       }
 }
 
 static

diff --git a/tests/bindings/python/bt2/test_port.py b/tests/bindings/python/bt2/test_port.py
index 68640a78fe8bb5ced6341130b43c14ff91ea9cc2..3ba46cdd1772cc4ae3d72b206c758d62ed89256d 100644 (file)
--- a/tests/bindings/python/bt2/test_port.py
+++ b/tests/bindings/python/bt2/test_port.py
@@ -716,6 +716,11 @@ class PortTestCase(unittest.TestCase):
         self._create_comp(MySink)
 
     def test_source_self_port_user_data(self):
+        class MyUserData:
+            def __del__(self):
+                nonlocal objects_deleted
+                objects_deleted += 1
+
         class MySource(
             bt2._UserFilterComponent, message_iterator_class=bt2._UserMessageIterator
         ):
@@ -726,13 +731,30 @@ class PortTestCase(unittest.TestCase):
                 user_datas.append(p.user_data)
                 p = comp_self._add_output_port('port2', 2)
                 user_datas.append(p.user_data)
+                p = comp_self._add_output_port('port3', MyUserData())
+                user_datas.append(p.user_data)
 
         user_datas = []
+        objects_deleted = 0
 
-        self._create_comp(MySource)
-        self.assertEqual(user_datas, [None, 2])
+        comp = self._create_comp(MySource)
+        self.assertEqual(len(user_datas), 3)
+        self.assertIs(user_datas[0], None)
+        self.assertEqual(user_datas[1], 2)
+        self.assertIs(type(user_datas[2]), MyUserData)
+
+        # Verify that the user data gets freed.
+        self.assertEqual(objects_deleted, 0)
+        del user_datas
+        del comp
+        self.assertEqual(objects_deleted, 1)
 
     def test_filter_self_port_user_data(self):
+        class MyUserData:
+            def __del__(self):
+                nonlocal objects_deleted
+                objects_deleted += 1
+
         class MyFilter(
             bt2._UserFilterComponent, message_iterator_class=bt2._UserMessageIterator
         ):
@@ -743,36 +765,65 @@ class PortTestCase(unittest.TestCase):
                 user_datas.append(p.user_data)
                 p = comp_self._add_output_port('port2', 'user data string')
                 user_datas.append(p.user_data)
+                p = comp_self._add_output_port('port3', MyUserData())
+                user_datas.append(p.user_data)
 
-                p = comp_self._add_input_port('port3')
+                p = comp_self._add_input_port('port4')
                 user_datas.append(p.user_data)
-                p = comp_self._add_input_port('port4', user_data={'user data': 'dict'})
+                p = comp_self._add_input_port('port5', user_data={'user data': 'dict'})
+                user_datas.append(p.user_data)
+                p = comp_self._add_input_port('port6', MyUserData())
                 user_datas.append(p.user_data)
 
         user_datas = []
+        objects_deleted = 0
 
-        self._create_comp(MyFilter)
-        self.assertEqual(
-            user_datas, [None, 'user data string', None, {'user data': 'dict'}]
-        )
+        comp = self._create_comp(MyFilter)
+        self.assertEqual(len(user_datas), 6)
+        self.assertIs(user_datas[0], None)
+        self.assertEqual(user_datas[1], 'user data string')
+        self.assertIs(type(user_datas[2]), MyUserData)
+        self.assertIs(user_datas[3], None)
+        self.assertEqual(user_datas[4], {'user data': 'dict'})
+        self.assertIs(type(user_datas[5]), MyUserData)
+
+        # Verify that the user data gets freed.
+        self.assertEqual(objects_deleted, 0)
+        del user_datas
+        del comp
+        self.assertEqual(objects_deleted, 2)
 
     def test_sink_self_port_user_data(self):
+        class MyUserData:
+            def __del__(self):
+                nonlocal objects_deleted
+                objects_deleted += 1
+
         class MySink(bt2._UserSinkComponent):
             def __init__(comp_self, config, params, obj):
                 nonlocal user_datas
 
                 p = comp_self._add_input_port('port1')
                 user_datas.append(p.user_data)
-                p = comp_self._add_input_port('port2', set())
+                p = comp_self._add_input_port('port2', MyUserData())
                 user_datas.append(p.user_data)
 
             def _user_consume(self):
                 pass
 
         user_datas = []
+        objects_deleted = 0
 
-        self._create_comp(MySink)
-        self.assertEqual(user_datas, [None, set()])
+        comp = self._create_comp(MySink)
+        self.assertEqual(len(user_datas), 2)
+        self.assertIs(user_datas[0], None)
+        self.assertIs(type(user_datas[1]), MyUserData)
+
+        # Verify that the user data gets freed.
+        self.assertEqual(objects_deleted, 0)
+        del user_datas
+        del comp
+        self.assertEqual(objects_deleted, 1)
 
 
 if __name__ == '__main__':