From f002c045dc8f7a2f077e2ec4fa8a168f39c77619 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 6 Jan 2020 20:47:20 -0500 Subject: [PATCH] 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/2734 Tested-by: jenkins Reviewed-by: Philippe Proulx --- .../bt2/bt2/native_bt_component_class.i.h | 107 +++++++++++++++++- tests/bindings/python/bt2/test_port.py | 73 ++++++++++-- 2 files changed, 166 insertions(+), 14 deletions(-) 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 f830afa3..6ea021de 100644 --- 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 68640a78..3ba46cdd 100644 --- 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__': -- 2.34.1