lib: save and restore current thread error when calling destruction listeners and...

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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2385
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/src/lib/graph/component.c b/src/lib/graph/component.c
index b5591c9eb4f48152eed3bb76e6ed80022d109a98..c771e27870cd15ca40ca27736ca794a55a836374 100644 (file)
--- 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 49381a3ca9f69dcb4d21c7cd4520462053380726..a0a88199a0627c8a1eb6cf335f242c4700b9bf4d 100644 (file)
--- 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 f9aedea0947c2b93092d5917c5575cb69d463352..153300998afd06f1e41bebe9818316adc4cf6199 100644 (file)
--- 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 d155cae2e0ea205c0a04ffc5ab0d3019d6d637ed..57ebf67bf82257c4a3b57fe891da2e836506b9b1 100644 (file)
--- 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) {