From: Simon Marchi Date: Fri, 18 Oct 2019 16:45:28 +0000 (-0400) Subject: Fix: bt2: clear Python error indicator in trace and trace class destruction listeners X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=64961f8b6306c567fd2683d19a1bdee1331a8d5b 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/2224 Reviewed-by: Francis Deslauriers --- 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 26404afb..c525fa11 100644 --- 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 f51d619f..329fda65 100644 --- 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 8b5537f2..4a1b1b61 100644 --- 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 4c2941da..3345b18a 100644 --- 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()