Fix: lib: prevent infinite recursion when destroying trace classes and traces

When implementing support for trace and trace class destruction
listeners in Python, we hit a problem where we enter infinite recursion
(until the process crashes).

When the refcount of a trace class reaches 0, the destruction listeners
for that trace class are called.  The Python listener creates a
TraceClass object, which acquires a new reference to the trace class
being destroyed.  When the listener is done, it drops that reference.
This causes the refcount to reach 0 again, and the lib to call all the
destruction listeners again.

The same happens for traces.

To prevent it, this patch makes the lib increment the refcount by one
before calling all destruction listeners.

Change-Id: Ib7e7bd0428c2a505fb25ad0aa80150b518da4c4e
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1282
Tested-by: jenkins
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/lib/trace-ir/trace-class.c b/lib/trace-ir/trace-class.c
index 64a057b56a42be7dc640ec934f069e4cc0ca9104..591df481cc7b53cfd8ca29dbdaec6c821b28ed27 100644 (file)
--- a/lib/trace-ir/trace-class.c
+++ b/lib/trace-ir/trace-class.c
@@ -75,6 +75,19 @@ void destroy_trace_class(struct bt_object *obj)
        if (tc->destruction_listeners) {
                uint64_t i;
                BT_LIB_LOGV("Calling trace class destruction listener(s): %!+T", tc);
+
+               /*
+               * The trace class' reference count is 0 if we're here. Increment
+               * it to avoid a double-destroy (possibly infinitely recursive).
+               * This could happen for example if a destruction listener did
+               * bt_object_get_ref() (or anything that causes
+               * bt_object_get_ref() to be called) on the trace class (ref.
+               * count goes from 0 to 1), and then bt_object_put_ref(): the
+               * reference count would go from 1 to 0 again and this function
+               * would be called again.
+               */
+               tc->base.ref_count++;
+
                /* Call all the trace class destruction listeners */
                for (i = 0; i < tc->destruction_listeners->len; i++) {
                        struct bt_trace_class_destruction_listener_elem elem =
@@ -84,6 +97,12 @@ void destroy_trace_class(struct bt_object *obj)
                        if (elem.func) {
                                elem.func(tc, elem.data);
                        }
+
+                       /*
+                        * The destruction listener should not have kept a
+                        * reference to the trace class.
+                        */
+                       BT_ASSERT_PRE(tc->base.ref_count == 1, "Destruction listener kept a reference to the trace class being destroyed: %![tc-]+T", tc);
                }
                g_array_free(tc->destruction_listeners, TRUE);
                tc->destruction_listeners = NULL;

diff --git a/lib/trace-ir/trace.c b/lib/trace-ir/trace.c
index 792d89f73389d427aed273ec92c8b3ebcca5636e..9c99a576329d18a3608eaad98901141bdd124eec 100644 (file)
--- a/lib/trace-ir/trace.c
+++ b/lib/trace-ir/trace.c
@@ -77,6 +77,19 @@ void destroy_trace(struct bt_object *obj)
        if (trace->destruction_listeners) {
                uint64_t i;
                BT_LIB_LOGV("Calling trace destruction listener(s): %!+t", trace);
+
+               /*
+               * The trace's reference count is 0 if we're here. Increment
+               * it to avoid a double-destroy (possibly infinitely recursive).
+               * This could happen for example if a destruction listener did
+               * bt_object_get_ref() (or anything that causes
+               * bt_object_get_ref() to be called) on the trace (ref.
+               * count goes from 0 to 1), and then bt_object_put_ref(): the
+               * reference count would go from 1 to 0 again and this function
+               * would be called again.
+               */
+               trace->base.ref_count++;
+
                /* Call all the trace destruction listeners */
                for (i = 0; i < trace->destruction_listeners->len; i++) {
                        struct bt_trace_destruction_listener_elem elem =
@@ -86,6 +99,12 @@ void destroy_trace(struct bt_object *obj)
                        if (elem.func) {
                                elem.func(trace, elem.data);
                        }
+
+                       /*
+                        * The destruction listener should not have kept a
+                        * reference to the trace.
+                        */
+                       BT_ASSERT_PRE(trace->base.ref_count == 1, "Destruction listener kept a reference to the trace being destroyed: %![trace-]+t", trace);
                }
                g_array_free(trace->destruction_listeners, TRUE);
                trace->destruction_listeners = NULL;