Skip to content

Commit

Permalink
Fix: bt2: clear Python error indicator in trace and trace class destr…
Browse files Browse the repository at this point in the history
…uction 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>
  • Loading branch information
simark committed Oct 21, 2019
1 parent 6dbc193 commit 64961f8
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 6 deletions.
5 changes: 2 additions & 3 deletions src/bindings/python/bt2/bt2/native_bt_trace.i.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 2 additions & 3 deletions src/bindings/python/bt2/bt2/native_bt_trace_class.i.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
10 changes: 10 additions & 0 deletions tests/bindings/python/bt2/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,16 @@ def on_trace_destruction(trace_class):
):
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()
9 changes: 9 additions & 0 deletions tests/bindings/python/bt2/test_trace_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,15 @@ def on_trace_class_destruction(trace_class):
):
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()

0 comments on commit 64961f8

Please sign in to comment.