From 42a6316598eda35c8f23f8afbf579e385dc6de40 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 13 Nov 2019 15:36:42 -0500 Subject: [PATCH] lib: save and restore current thread error when calling destruction listeners and finalize methods Consider the following chain of events: - User creates and runs a graph - An error happens when running the graph, _ERROR is returned and the current thread has an error - The user does put_ref on the graph to clean up, deliberately leaving the error on the current thread so it's processed above on the stack. - When calling put_ref, a trace destruction listener is called. - After calling the trace destruction listener, the lib asserts that no error is set (BT_ASSERT_POST_NO_ERROR), which is not true, so the assert fails. In this case, the user of the graph would have to take the error, do the put_ref and restore the error, if it wants to leave it for a function higher in the stack. However, it is such a common scenario to hit an error, do some put_ref to clean up and return the error, that it would be very heavy (and error-prone) to have to do this all the time. This patch makes it safe to call put_ref with an error set by wrapping all the user code that can be called as a consequence of a put_ref (destruction listeners and finalize methods) with an error take/move. Before calling the user destruction listener or finalize method, the lib saves the error on the stack. The user callback is therefore called with no error set and the lib can still validate with BT_ASSERT_POST_NO_ERROR that it leaves no error on the current thread after returning. The error is then moved back. We could say that put_ref now become "error-neutral", in that they can be called with an error set on the current thread, and they won't modify it. Change-Id: I8c7a5429d53073483b9e03f0ec654c826466ee4e Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/2385 Reviewed-by: Francis Deslauriers Reviewed-by: Philippe Proulx --- src/lib/graph/component.c | 9 +++++++++ src/lib/graph/iterator.c | 8 ++++++++ src/lib/trace-ir/trace-class.c | 8 ++++++++ src/lib/trace-ir/trace.c | 8 ++++++++ 4 files changed, 33 insertions(+) diff --git a/src/lib/graph/component.c b/src/lib/graph/component.c index b5591c9e..c771e278 100644 --- a/src/lib/graph/component.c +++ b/src/lib/graph/component.c @@ -104,9 +104,18 @@ void finalize_component(struct bt_component *comp) } if (method) { + const struct bt_error *saved_error; + + saved_error = bt_current_thread_take_error(); + BT_LIB_LOGI("Calling user's component finalization method: " "%![comp-]+c", comp); method(comp); + BT_ASSERT_POST_NO_ERROR(); + + if (saved_error) { + BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET(saved_error); + } } } diff --git a/src/lib/graph/iterator.c b/src/lib/graph/iterator.c index 49381a3c..a0a88199 100644 --- a/src/lib/graph/iterator.c +++ b/src/lib/graph/iterator.c @@ -226,9 +226,17 @@ void bt_self_component_port_input_message_iterator_try_finalize( } if (method) { + const bt_error *saved_error; + + saved_error = bt_current_thread_take_error(); + BT_LIB_LOGD("Calling user's finalization method: %!+i", iterator); method(iterator); + + if (saved_error) { + BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET(saved_error); + } } /* Detach upstream message iterators */ diff --git a/src/lib/trace-ir/trace-class.c b/src/lib/trace-ir/trace-class.c index f9aedea0..15330099 100644 --- a/src/lib/trace-ir/trace-class.c +++ b/src/lib/trace-ir/trace-class.c @@ -80,6 +80,8 @@ void destroy_trace_class(struct bt_object *obj) */ if (tc->destruction_listeners) { uint64_t i; + const struct bt_error *saved_error; + BT_LIB_LOGD("Calling trace class destruction listener(s): %!+T", tc); /* @@ -94,6 +96,8 @@ void destroy_trace_class(struct bt_object *obj) */ tc->base.ref_count++; + saved_error = bt_current_thread_take_error(); + /* Call all the trace class destruction listeners */ for (i = 0; i < tc->destruction_listeners->len; i++) { struct bt_trace_class_destruction_listener_elem elem = @@ -113,6 +117,10 @@ void destroy_trace_class(struct bt_object *obj) } g_array_free(tc->destruction_listeners, TRUE); tc->destruction_listeners = NULL; + + if (saved_error) { + BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET(saved_error); + } } if (tc->stream_classes) { diff --git a/src/lib/trace-ir/trace.c b/src/lib/trace-ir/trace.c index d155cae2..57ebf67b 100644 --- a/src/lib/trace-ir/trace.c +++ b/src/lib/trace-ir/trace.c @@ -82,6 +82,8 @@ void destroy_trace(struct bt_object *obj) */ if (trace->destruction_listeners) { uint64_t i; + const struct bt_error *saved_error; + BT_LIB_LOGD("Calling trace destruction listener(s): %!+t", trace); /* @@ -96,6 +98,8 @@ void destroy_trace(struct bt_object *obj) */ trace->base.ref_count++; + saved_error = bt_current_thread_take_error(); + /* Call all the trace destruction listeners */ for (i = 0; i < trace->destruction_listeners->len; i++) { struct bt_trace_destruction_listener_elem elem = @@ -115,6 +119,10 @@ void destroy_trace(struct bt_object *obj) } g_array_free(trace->destruction_listeners, TRUE); trace->destruction_listeners = NULL; + + if (saved_error) { + BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET(saved_error); + } } if (trace->name.str) { -- 2.34.1