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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1731
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Tested-by: jenkins <jenkins@lttng.org>

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 a8b55e55f5bec45241621429ee5386b7cd5ea1f0..7109006991316e87d5db62306b02829481807b3c 100644 (file)
--- 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 869c072c6f508028d33e71a94ba47685305c56cf..72003da701710ce91e19e015149fc5a933825a68 100644 (file)
--- 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):