Fix: python: don't get extra references when creating field classes

Running:

    $ G_SLICE=always-malloc G_DEBUG=gc-friendly PYTHONMALLOC=malloc tests/utils/run_python_bt2 /home/smarchi/build/babeltrace/tests/../src/cli/babeltrace2 --plugin-path=/home/smarchi/src/babeltrace/tests/data/plugins/flt.lttng-utils.debug-info -c src.test_debug_info.CompleteSrc -c sink.text.details --params with-trace-name=false,with-stream-name=false,with-uuid=false -c flt.lttng-utils.debug-info

... ASan gives me many field class-related leaks such as:

    Indirect leak of 32 byte(s) in 1 object(s) allocated from:
        #0 0x7fe783780e17 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x7fe783417ef0 in g_malloc0 ../../../glib/gmem.c:132
        #2 0x7fe7835e6305 in bt_field_class_variant_without_selector_append_option /home/smarchi/src/babeltrace/src/lib/trace-ir/field-class.c:1576
        #3 0x7fe77e92ff4c in _wrap_field_class_variant_without_selector_append_option bt2/native_bt.c:10957
        #4 0x7fe77f752686 in cfunction_call_varargs ../Objects/call.c:758

I only see this on Ubuntu 20.04, not on my other development machine that is on
Arch Linux.

I think that we erroneously make some get_refs when creating some field
classes in trace_class.py.  For instance, when creating a static array:

    def create_static_array_field_class(self, elem_fc, length, user_attributes=None):
        utils._check_type(elem_fc, bt2_field_class._FieldClass)
        utils._check_uint64(length)
        ptr = native_bt.field_class_array_static_create(self._ptr, elem_fc._ptr, length)
        self._check_field_class_create_status(ptr, 'static array')
        fc = bt2_field_class._StaticArrayFieldClass._create_from_ptr_and_get_ref(ptr)
        self._set_field_class_user_attrs(fc, user_attributes)
        return fc

The `native_bt.field_class_array_static_create` line returns a pointer
to the created object, with a refcount of 1, where the reference belongs
to the caller.  We then call `_create_from_ptr_and_get_ref` to create a
Python wrapper around the raw pointer, which brings the refcount to 2.
We then return that wrapper.  The create_static_array_field_class
function should return an object with a refcount of 1, since the only
reference at this point will belong to its caller.  So we must not take
an additional reference here, the only returned by the native code is
enough.

 - Add a field_class._obj_type_from_field_class_ptr_template function,
   to get an object wrapper type from a pointer
 - Make the existing
   field_class._create_field_class_from_ptr_and_get_ref_template
   function use it, to avoid duplicating code
 - Add a new field_class._obj_type_from_field_class_ptr, for use in the
   field class-creation functions, in trace_class.py.

With this, the command line shown above does not produce any leaks
reported by ASan.

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

diff --git a/src/bindings/python/bt2/bt2/field_class.py b/src/bindings/python/bt2/bt2/field_class.py
index 13c5e46213a26cd4d9f6ec011a173e95ceaf6ebc..0d0934a8407050ff97d8f0a45e506462a82f0951 100644 (file)
--- a/src/bindings/python/bt2/bt2/field_class.py
+++ b/src/bindings/python/bt2/bt2/field_class.py
@@ -10,9 +10,19 @@ from bt2 import value as bt2_value
 import bt2
 
 
-def _create_field_class_from_ptr_and_get_ref_template(type_map, ptr):
+def _obj_type_from_field_class_ptr_template(type_map, ptr):
     typeid = native_bt.field_class_get_type(ptr)
-    return type_map[typeid]._create_from_ptr_and_get_ref(ptr)
+    return type_map[typeid]
+
+
+def _obj_type_from_field_class_ptr(ptr):
+    return _obj_type_from_field_class_ptr_template(_FIELD_CLASS_TYPE_TO_OBJ, ptr)
+
+
+def _create_field_class_from_ptr_and_get_ref_template(type_map, ptr):
+    return _obj_type_from_field_class_ptr_template(
+        type_map, ptr
+    )._create_from_ptr_and_get_ref(ptr)
 
 
 def _create_field_class_from_ptr_and_get_ref(ptr):

diff --git a/src/bindings/python/bt2/bt2/trace_class.py b/src/bindings/python/bt2/bt2/trace_class.py
index 25a5cb6284f221a9cd0f8bf151e624096738ac22..ebfdbe4e54d3de8a5316d509731d261de99e9c72 100644 (file)
--- a/src/bindings/python/bt2/bt2/trace_class.py
+++ b/src/bindings/python/bt2/bt2/trace_class.py
@@ -418,7 +418,7 @@ class _TraceClass(_TraceClassConst):
         utils._check_uint64(length)
         ptr = native_bt.field_class_array_static_create(self._ptr, elem_fc._ptr, length)
         self._check_field_class_create_status(ptr, 'static array')
-        fc = bt2_field_class._StaticArrayFieldClass._create_from_ptr_and_get_ref(ptr)
+        fc = bt2_field_class._StaticArrayFieldClass._create_from_ptr(ptr)
         self._set_field_class_user_attrs(fc, user_attributes)
         return fc
 
@@ -436,7 +436,7 @@ class _TraceClass(_TraceClassConst):
             self._ptr, elem_fc._ptr, length_fc_ptr
         )
         self._check_field_class_create_status(ptr, 'dynamic array')
-        fc = bt2_field_class._create_field_class_from_ptr_and_get_ref(ptr)
+        fc = bt2_field_class._obj_type_from_field_class_ptr(ptr)._create_from_ptr(ptr)
         self._set_field_class_user_attrs(fc, user_attributes)
         return fc
 
@@ -448,7 +448,7 @@ class _TraceClass(_TraceClassConst):
             self._ptr, content_fc._ptr
         )
         self._check_field_class_create_status(ptr, 'option')
-        fc = bt2_field_class._create_field_class_from_ptr_and_get_ref(ptr)
+        fc = bt2_field_class._obj_type_from_field_class_ptr(ptr)._create_from_ptr(ptr)
         self._set_field_class_user_attrs(fc, user_attributes)
         return fc
 
@@ -462,7 +462,7 @@ class _TraceClass(_TraceClassConst):
             self._ptr, content_fc._ptr, selector_fc._ptr
         )
         self._check_field_class_create_status(ptr, 'option')
-        fc = bt2_field_class._create_field_class_from_ptr_and_get_ref(ptr)
+        fc = bt2_field_class._obj_type_from_field_class_ptr(ptr)._create_from_ptr(ptr)
         self._set_field_class_user_attrs(fc, user_attributes)
         fc._selector_is_reversed = selector_is_reversed
         return fc
@@ -490,7 +490,7 @@ class _TraceClass(_TraceClassConst):
             )
 
         self._check_field_class_create_status(ptr, 'option')
-        fc = bt2_field_class._create_field_class_from_ptr_and_get_ref(ptr)
+        fc = bt2_field_class._obj_type_from_field_class_ptr(ptr)._create_from_ptr(ptr)
         self._set_field_class_user_attrs(fc, user_attributes)
         return fc
 
@@ -503,6 +503,6 @@ class _TraceClass(_TraceClassConst):
 
         ptr = native_bt.field_class_variant_create(self._ptr, selector_fc_ptr)
         self._check_field_class_create_status(ptr, 'variant')
-        fc = bt2_field_class._create_field_class_from_ptr_and_get_ref(ptr)
+        fc = bt2_field_class._obj_type_from_field_class_ptr(ptr)._create_from_ptr(ptr)
         self._set_field_class_user_attrs(fc, user_attributes)
         return fc