From 0361868a820211a562bc5b2cb915596ff2325fc2 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sat, 20 Jul 2019 23:10:41 -0400 Subject: [PATCH] Fix: bt2: incref Py_None in get_msg_range_common on error When get_msg_range_common processes a result that is not OK, it returns a tuple whose second element is None: (status, None) However, when setting the second element of the tuple to None, we are missing an incref of Py_None. This incref is necessary, because PyTuple_SET_ITEM steals the reference of the object we pass. This patch fixes that. Change-Id: I8a5af2853172a8399dd1779d90e0fcb8fc265032 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/1731 Reviewed-by: Philippe Proulx Tested-by: jenkins --- .../bt2/bt2/native_bt_message_iterator.i | 23 +++++++++++-------- .../python/bt2/test_message_iterator.py | 23 +++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/bindings/python/bt2/bt2/native_bt_message_iterator.i b/src/bindings/python/bt2/bt2/native_bt_message_iterator.i index a8b55e55..71090069 100644 --- a/src/bindings/python/bt2/bt2/native_bt_message_iterator.i +++ b/src/bindings/python/bt2/bt2/native_bt_message_iterator.i @@ -68,20 +68,25 @@ PyObject *get_msg_range_common(bt_message_iterator_next_status status, { PyObject *py_status; PyObject *py_return_tuple; - PyObject *py_msg_list = Py_None; + PyObject *py_msg_list; - py_status = SWIG_From_long_SS_long(status); - if (status != __BT_FUNC_STATUS_OK) { - goto end; - } - - py_msg_list = create_pylist_from_messages(messages, message_count); - -end: py_return_tuple = PyTuple_New(2); BT_ASSERT(py_return_tuple); + + /* Set tuple[0], status. */ + py_status = SWIG_From_long_SS_long(status); PyTuple_SET_ITEM(py_return_tuple, 0, py_status); + + /* Set tuple[1], message list on success, None otherwise. */ + if (status == __BT_FUNC_STATUS_OK) { + py_msg_list = create_pylist_from_messages(messages, message_count); + } else { + py_msg_list = Py_None; + Py_INCREF(py_msg_list); + } + PyTuple_SET_ITEM(py_return_tuple, 1, py_msg_list); + return py_return_tuple; } diff --git a/tests/bindings/python/bt2/test_message_iterator.py b/tests/bindings/python/bt2/test_message_iterator.py index 869c072c..72003da7 100644 --- a/tests/bindings/python/bt2/test_message_iterator.py +++ b/tests/bindings/python/bt2/test_message_iterator.py @@ -300,6 +300,29 @@ class UserMessageIteratorTestCase(unittest.TestCase): with self.assertRaises(bt2.Error): it.seek_beginning() + # Try consuming many times from an iterator that always returns TryAgain. + # This verifies that we are not missing an incref of Py_None, making the + # refcount of Py_None reach 0. + def test_try_again_many_times(self): + class MyIter(bt2._UserMessageIterator): + def __next__(self): + raise bt2.TryAgain + + class MySource(bt2._UserSourceComponent, message_iterator_class=MyIter): + def __init__(self, params): + self._add_output_port('out') + + graph = bt2.Graph() + src = graph.add_component(MySource, 'src') + it = graph.create_output_port_message_iterator(src.output_ports['out']) + + # The initial refcount of Py_None was in the 7000, so 100000 iterations + # should be enough to catch the bug even if there are small differences + # between configurations. + for i in range(100000): + with self.assertRaises(bt2.TryAgain): + next(it) + class OutputPortMessageIteratorTestCase(unittest.TestCase): def test_component(self): -- 2.34.1