This patch fixes some reference counting bugs when handling bt_values in
Python.
The first one is when a component class implemented in Python returns
None to a query. The _UserComponentType._bt_query_from_native method
must return a bt_value address, and it must be a new reference that it
transfers to its caller. Currently, it returns a pointer to
bt_value_null, without ever acquiring a new reference to it.
The second bug, in the same area, is when the component class returns
some value. We currently call bt2.object._release, effectively
transferring the Python object's reference to the value to the caller.
However, the user could have kept a reference to that value, wishing to
keep using it. We should instead return a new reference to this value.
_UserComponentType._bt_query_from_native is therefore modified to
acquire a new reference to return to its caller, regardless of if the
value is bt_value_null or another value.
The third bug is in bt2.value._create_from_ptr. This function receives
a pointer to a bt_value, and steals it from its caller. It creates a
Python object that, when destroy, will put the reference. However, when
the passed pointer is bt_value_null, it just returns None. The
reference to bt_value_null that has been passed therefore never gets
put.
Bugs 1 and 3 would cancel each other: when doing a query that returned
bt_value_null from Python to a Python component class, we would miss
getting a ref to bt_value_null (bug 1) but we would also miss putting it
(bug 3).
The patch adds a test where Python code queries a Python component
class, which returns None. While it might not have caught all the bugs
fixed in this patch, it at least exercises the code paths touched in
this patch, which were not tested before, AFAICT.
Change-Id: I0f7bf1004d9d766db4cc0c83475ed37ce2e3f1ed
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1666
CI-Build: Philippe Proulx <eeppeliteloop@gmail.com>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Tested-by: jenkins <jenkins@lttng.org>
results = bt2.create_value(results)
if results is None:
- results_addr = int(native_bt.value_null)
+ results_ptr = native_bt.value_null
else:
# return new reference
- results_addr = int(results._release())
+ results_ptr = results._ptr
- return results_addr
+ # We return a new reference.
+ bt2.value._Value._get_ref(results_ptr)
+
+ return int(results_ptr)
def _query(cls, query_executor, obj, params, log_level):
raise NotImplementedError
cls._get_ref(obj._ptr)
return obj
- def _release(self):
- """Return the wrapped pointer, transfer its ownership to the
- caller."""
- ptr = self._ptr
- self._ptr = None
- return ptr
-
def __del__(self):
self._put_ref(self._ptr)
def _create_from_ptr(ptr):
- if ptr is None or ptr == native_bt.value_null:
+ if ptr is None:
+ return
+
+ # bt_value_null is translated to None. However, we are given a reference
+ # to it that we are not going to manage anymore, since we don't create a
+ # Python wrapper for it. Therefore put that reference immediately.
+ if ptr == native_bt.value_null:
+ bt2.value._Value._put_ref(ptr)
return
typeid = native_bt.value_get_type(ptr)
self.assertEqual(query_log_level, bt2.LoggingLevel.WARNING)
del query_log_level
+ def test_query_returns_none(self):
+ class MySink(bt2._UserSinkComponent):
+ def _consume(self):
+ pass
+
+ @staticmethod
+ def _query(query_exec, obj, params, log_level):
+ return
+
+ res = bt2.QueryExecutor().query(MySink, 'obj', None)
+ self.assertIsNone(res)
+
def test_query_simple(self):
class MySink(bt2._UserSinkComponent):
def _consume(self):
projects / babeltrace.git / commitdiff
