From c47a6bece6786574f9241c836f98571026dacedf Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 8 May 2019 17:07:22 -0400 Subject: [PATCH] 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/1282 Tested-by: jenkins Reviewed-by: Philippe Proulx --- lib/trace-ir/trace-class.c | 19 +++++++++++++++++++ lib/trace-ir/trace.c | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/lib/trace-ir/trace-class.c b/lib/trace-ir/trace-class.c index 64a057b5..591df481 100644 --- 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 792d89f7..9c99a576 100644 --- 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; -- 2.34.1