bt2: make _ListenerHandle not hold a strong reference on the target object

The bt2._ListenerHandle object currently holds a strong reference to the
Python object on which the listener was added.  This allows validating
that a handle passed to the remove_destruction_listener method of an
object mathces that object.

However, keeping that strong reference also prevents the destruction of
that target object.  So, adding a destruction listener and keeping the
handle around actually prevents the destruction from happening, making
that destruction listener useless.

Change it so the _ListenerHandle only keeps the address of the target
object.  This is enough to do the check described above.  We must
however invalidate the _ListenerHandle when the target object is
destroyed, because another object could be later created at the same
address.  To achieve this, the handle object is bound to the destruction
callback, so that we can invalidate it in
_trace_destruction_listener_from_native /
_trace_class_destruction_listener_from_native.

The "del" statements in the tests were necessary before, otherwise the
handles would keep the trace class / trace alive, and the destruction
listeners would not get called.  I removed them, so it now tests that
keeping a listener handle doesn't keep the target object alive.

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

diff --git a/src/bindings/python/bt2/bt2/trace.py b/src/bindings/python/bt2/bt2/trace.py
index 5dcaa15c422d58f48df913aebf1a4de8717913de..618899061bd30f77722d03be00dbebaebe70c665 100644 (file)
--- a/src/bindings/python/bt2/bt2/trace.py
+++ b/src/bindings/python/bt2/bt2/trace.py
@@ -166,9 +166,11 @@ class _TraceConst(object._SharedObject, collections.abc.Mapping):
         if not callable(listener):
             raise TypeError("'listener' parameter is not callable")
 
+        handle = utils._ListenerHandle(self.addr)
+
         fn = native_bt.bt2_trace_add_destruction_listener
         listener_from_native = functools.partial(
-            _trace_destruction_listener_from_native, listener
+            _trace_destruction_listener_from_native, listener, handle
         )
 
         status, listener_id = fn(self._ptr, listener_from_native)
@@ -176,12 +178,14 @@ class _TraceConst(object._SharedObject, collections.abc.Mapping):
             status, 'cannot add destruction listener to trace object'
         )
 
-        return utils._ListenerHandle(listener_id, self)
+        handle._set_listener_id(listener_id)
+
+        return handle
 
     def remove_destruction_listener(self, listener_handle):
         utils._check_type(listener_handle, utils._ListenerHandle)
 
-        if listener_handle._obj.addr != self.addr:
+        if listener_handle._addr != self.addr:
             raise ValueError(
                 'This trace destruction listener does not match the trace object.'
             )
@@ -193,7 +197,7 @@ class _TraceConst(object._SharedObject, collections.abc.Mapping):
             self._ptr, listener_handle._listener_id
         )
         utils._handle_func_status(status)
-        listener_handle._listener_id = None
+        listener_handle._invalidate()
 
 
 class _Trace(_TraceConst):
@@ -263,6 +267,7 @@ class _Trace(_TraceConst):
         return stream
 
 
-def _trace_destruction_listener_from_native(user_listener, trace_ptr):
+def _trace_destruction_listener_from_native(user_listener, handle, trace_ptr):
     trace = _TraceConst._create_from_ptr_and_get_ref(trace_ptr)
     user_listener(trace)
+    handle._invalidate()

diff --git a/src/bindings/python/bt2/bt2/trace_class.py b/src/bindings/python/bt2/bt2/trace_class.py
index dc9439eb6aa424215d3ac883249699b115796852..94010d47aa04aac8e2dc48795bf758a7dddf59e6 100644 (file)
--- a/src/bindings/python/bt2/bt2/trace_class.py
+++ b/src/bindings/python/bt2/bt2/trace_class.py
@@ -33,9 +33,12 @@ import functools
 import bt2
 
 
-def _trace_class_destruction_listener_from_native(user_listener, trace_class_ptr):
+def _trace_class_destruction_listener_from_native(
+    user_listener, handle, trace_class_ptr
+):
     trace_class = _TraceClass._create_from_ptr_and_get_ref(trace_class_ptr)
     user_listener(trace_class)
+    handle._invalidate()
 
 
 class _TraceClassConst(object._SharedObject, collections.abc.Mapping):
@@ -100,22 +103,26 @@ class _TraceClassConst(object._SharedObject, collections.abc.Mapping):
         if not callable(listener):
             raise TypeError("'listener' parameter is not callable")
 
-        fn = native_bt.bt2_trace_class_add_destruction_listener
+        handle = utils._ListenerHandle(self.addr)
+
         listener_from_native = functools.partial(
-            _trace_class_destruction_listener_from_native, listener
+            _trace_class_destruction_listener_from_native, listener, handle
         )
 
+        fn = native_bt.bt2_trace_class_add_destruction_listener
         status, listener_id = fn(self._ptr, listener_from_native)
         utils._handle_func_status(
             status, 'cannot add destruction listener to trace class object'
         )
 
-        return utils._ListenerHandle(listener_id, self)
+        handle._set_listener_id(listener_id)
+
+        return handle
 
     def remove_destruction_listener(self, listener_handle):
         utils._check_type(listener_handle, utils._ListenerHandle)
 
-        if listener_handle._obj.addr != self.addr:
+        if listener_handle._addr != self.addr:
             raise ValueError(
                 'This trace class destruction listener does not match the trace class object.'
             )
@@ -129,7 +136,7 @@ class _TraceClassConst(object._SharedObject, collections.abc.Mapping):
             self._ptr, listener_handle._listener_id
         )
         utils._handle_func_status(status)
-        listener_handle._listener_id = None
+        listener_handle._invalidate()
 
 
 class _TraceClass(_TraceClassConst):

diff --git a/src/bindings/python/bt2/bt2/utils.py b/src/bindings/python/bt2/bt2/utils.py
index 77ebe606057aeec6d203df13c1f9af491bf2e8d5..67017cd021aa637d2df664919220471b5ab70962 100644 (file)
--- a/src/bindings/python/bt2/bt2/utils.py
+++ b/src/bindings/python/bt2/bt2/utils.py
@@ -166,6 +166,12 @@ def _handle_func_status(status, msg=None):
 
 
 class _ListenerHandle:
-    def __init__(self, listener_id, obj):
+    def __init__(self, addr):
+        self._addr = addr
+        self._listener_id = None
+
+    def _set_listener_id(self, listener_id):
         self._listener_id = listener_id
-        self._obj = obj
+
+    def _invalidate(self):
+        self._listener_id = None

diff --git a/tests/bindings/python/bt2/test_trace.py b/tests/bindings/python/bt2/test_trace.py
index 4a1b1b61669d64e829465317b9276678e4828bb9..70e70e7548003bb52bc3e7f3cf7b93dd4f7c3d2d 100644 (file)
--- a/tests/bindings/python/bt2/test_trace.py
+++ b/tests/bindings/python/bt2/test_trace.py
@@ -168,9 +168,6 @@ class TraceTestCase(unittest.TestCase):
 
         trace.remove_destruction_listener(td_handle2)
 
-        del td_handle1
-        del td_handle2
-
         self.assertEqual(num_trace_class_destroyed_calls, 0)
         self.assertEqual(num_trace_destroyed_calls, 0)
 

diff --git a/tests/bindings/python/bt2/test_trace_class.py b/tests/bindings/python/bt2/test_trace_class.py
index bd80586cfbc03393215fc98b2e7b4ff9f3884ddc..eca5274e03f13682eeb808d7049aee3083943d92 100644 (file)
--- a/tests/bindings/python/bt2/test_trace_class.py
+++ b/tests/bindings/python/bt2/test_trace_class.py
@@ -198,9 +198,6 @@ class TraceClassTestCase(unittest.TestCase):
 
         trace_class.remove_destruction_listener(handle2)
 
-        del handle1
-        del handle2
-
         self.assertEqual(num_destruct_calls, 0)
 
         del trace_class