Fix: bt2: clear Python error indicator in trace and trace class destruction listeners

When an exception is raised in a trace or trace class destruction
listener, we hit this assert:

    LIB/TRACE-CLASS destroy_trace_class@trace-class.c:110 Babeltrace 2 library postcondition not satisfied; error is:
    LIB/TRACE-CLASS destroy_trace_class@trace-class.c:110 Destruction listener kept a reference to the trace class being destroyed: tc-addr=0x6080000027a0, tc-is-frozen=0, tc-stream-class-count=0, tc-assigns-auto-sc-id=1
    LIB/TRACE-CLASS destroy_trace_class@trace-class.c:110 Aborting...

The problem is that we don't clear the error indicator on the error code
path after running the user code.  The error indicator (the exception)
keeps a reference to the Python stack, which has a reference to the
trace or trace class Python object.  This, in turn has kept a reference
to the Babeltrace trace or trace class object.

The solution is to clear the error indicator, which releases all of
this.

Another problem is that calling loge_exception_append_cause appends an
error cause, even though it's not valid to leave an error cause when
exiting those functions, as they don't return an error status.  That
error cause is left dangling and might wrongfully appear on an eventual
future error stack.

To fix it, use logw_exception instead of loge_exception_append_cause.
This matches what we do in component_class_finalize and
component_class_message_iterator_finalize, which are very similar
situations.

Change-Id: Ib756461610366badb6384577ad7cdf6468944be9
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2224
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

diff --git a/src/bindings/python/bt2/bt2/native_bt_trace.i.h b/src/bindings/python/bt2/bt2/native_bt_trace.i.h
index 26404afb9c6714e4b808d8e10f378d2e415e6c2b..c525fa11e1e16cfc232f75517cfdde993d21dd61 100644 (file)
--- a/src/bindings/python/bt2/bt2/native_bt_trace.i.h
+++ b/src/bindings/python/bt2/bt2/native_bt_trace.i.h
@@ -37,9 +37,8 @@ trace_destroyed_listener(const bt_trace *trace, void *py_callable)
 
        py_res = PyObject_CallFunction(py_callable, "(O)", py_trace_ptr);
        if (!py_res) {
-               loge_exception_append_cause(
-                       "Trace's destruction listener (Python)",
-                       BT_LOG_OUTPUT_LEVEL);
+               logw_exception(BT_LOG_OUTPUT_LEVEL);
+               PyErr_Clear();
                goto end;
        }
 

diff --git a/src/bindings/python/bt2/bt2/native_bt_trace_class.i.h b/src/bindings/python/bt2/bt2/native_bt_trace_class.i.h
index f51d619fdfd70f75f5864c3a20bfe230cc03a2c1..329fda6555c04df0eae3e3b01226036bf37010c3 100644 (file)
--- a/src/bindings/python/bt2/bt2/native_bt_trace_class.i.h
+++ b/src/bindings/python/bt2/bt2/native_bt_trace_class.i.h
@@ -37,9 +37,8 @@ trace_class_destroyed_listener(const bt_trace_class *trace_class, void *py_calla
 
        py_res = PyObject_CallFunction(py_callable, "(O)", py_trace_class_ptr);
        if (!py_res) {
-               loge_exception_append_cause(
-                       "Trace class's destruction listener (Python)",
-                       BT_LOG_OUTPUT_LEVEL);
+               logw_exception(BT_LOG_OUTPUT_LEVEL);
+               PyErr_Clear();
                goto end;
        }
 

diff --git a/tests/bindings/python/bt2/test_trace.py b/tests/bindings/python/bt2/test_trace.py
index 8b5537f2c198f163f08476e5b24aafdc35fd4447..4a1b1b61669d64e829465317b9276678e4828bb9 100644 (file)
--- a/tests/bindings/python/bt2/test_trace.py
+++ b/tests/bindings/python/bt2/test_trace.py
@@ -235,6 +235,16 @@ class TraceTestCase(unittest.TestCase):
         ):
             trace.remove_destruction_listener(handle)
 
+    def test_raise_in_destruction_listener(self):
+        def on_trace_destruction(trace):
+            raise ValueError('it hurts')
+
+        trace_class = get_default_trace_class()
+        trace = trace_class()
+        trace.add_destruction_listener(on_trace_destruction)
+
+        del trace
+
 
 if __name__ == '__main__':
     unittest.main()

diff --git a/tests/bindings/python/bt2/test_trace_class.py b/tests/bindings/python/bt2/test_trace_class.py
index 4c2941da8f5aa1eddc038c5d1b1ce8fd787dcd74..3345b18a913edf0b0174ea99bea860e8d09ee4eb 100644 (file)
--- a/tests/bindings/python/bt2/test_trace_class.py
+++ b/tests/bindings/python/bt2/test_trace_class.py
@@ -236,6 +236,15 @@ class TraceClassTestCase(unittest.TestCase):
         ):
             trace_class.remove_destruction_listener(handle)
 
+    def test_raise_in_destruction_listener(self):
+        def on_trace_class_destruction(trace_class):
+            raise ValueError('it hurts')
+
+        trace_class = get_default_trace_class()
+        trace_class.add_destruction_listener(on_trace_class_destruction)
+
+        del trace_class
+
 
 if __name__ == '__main__':
     unittest.main()