From 2bebdd7fc2b96d158c6e13663319e2700e80293d Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 13 Jun 2022 09:42:44 -0400 Subject: [PATCH] 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/8328 Tested-by: jenkins Reviewed-by: Philippe Proulx --- src/bindings/python/bt2/bt2/field_class.py | 14 ++++++++++++-- src/bindings/python/bt2/bt2/trace_class.py | 12 ++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/bindings/python/bt2/bt2/field_class.py b/src/bindings/python/bt2/bt2/field_class.py index 13c5e462..0d0934a8 100644 --- 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 25a5cb62..ebfdbe4e 100644 --- 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 -- 2.34.1