[lttng-dev] [PATCH babeltrace] Fix: Close traces on context destruction

[Jérémie Galarneau]

bt_trace_handle_destroy is called on destruction of the trace_handle
hash table's elements. This function only frees the trace handle,
leaving the input traces open.

This fix sets remove_trace_handle as the value_destroy_func ensuring
that the format's close_trace function is called before the trace_handle
is destroyed.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau at efficios.com>
---
 lib/context.c | 58 ++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/lib/context.c b/lib/context.c
index dc77366..5dc4119 100644
--- a/lib/context.c
+++ b/lib/context.c
@@ -34,6 +34,7 @@
 #include <babeltrace/trace-handle-internal.h>
 #include <babeltrace/trace-collection.h>
 #include <babeltrace/format.h>
+#include <babeltrace/format-internal.h>
 #include <babeltrace/babeltrace-internal.h>
 #include <stdlib.h>
 #include <string.h>
@@ -44,6 +45,8 @@
 
 #include <glib.h>
 
+static void remove_trace_handle(struct bt_trace_handle *handle);
+
 struct bt_context *bt_context_create(void)
 {
 	struct bt_context *ctx;
@@ -56,7 +59,7 @@ struct bt_context *bt_context_create(void)
 	/* Instanciate the trace handle container */
 	ctx->trace_handles = g_hash_table_new_full(g_direct_hash,
 				g_direct_equal, NULL,
-				(GDestroyNotify) bt_trace_handle_destroy);
+				(GDestroyNotify) remove_trace_handle);
 
 	ctx->current_iterator = NULL;
 	ctx->tc = g_new0(struct trace_collection, 1);
@@ -155,43 +158,39 @@ end:
 
 int bt_context_remove_trace(struct bt_context *ctx, int handle_id)
 {
-	struct bt_trace_handle *handle;
 	int ret;
 
-	if (!ctx)
-		return -EINVAL;
-
-	handle = g_hash_table_lookup(ctx->trace_handles,
-		(gpointer) (unsigned long) handle_id);
-	if (!handle)
-		return -ENOENT;
+	if (!ctx) {
+		ret = -EINVAL;
+		goto end;
+	}
 
-	/* Remove from containers */
-	bt_trace_collection_remove(ctx->tc, handle->td);
-	/* Close the trace */
-	ret = handle->format->close_trace(handle->td);
-	if (ret) {
-		fprintf(stderr, "Error in close_trace callback\n");
-		return ret;
+	/*
+	 * Remove the handle. remove_trace_handle will be called
+	 * automatically.
+	 */
+	if (!g_hash_table_remove(ctx->trace_handles,
+		(gpointer) (unsigned long) handle_id)) {
+		ret = -ENOENT;
+		goto end;
 	}
-	/* Remove and free the handle */
-	g_hash_table_remove(ctx->trace_handles,
-			(gpointer) (unsigned long) handle_id);
-	return 0;
+end:
+	return ret;
 }
 
 static
 void bt_context_destroy(struct bt_context *ctx)
 {
 	assert(ctx);
-	bt_finalize_trace_collection(ctx->tc);
 
 	/*
 	 * Remove all traces. The g_hash_table_destroy will call
-	 * bt_trace_handle_destroy on each elements.
+	 * remove_trace_handle on each element.
 	 */
 	g_hash_table_destroy(ctx->trace_handles);
 
+	bt_finalize_trace_collection(ctx->tc);
+
 	/* ctx->tc should always be valid */
 	assert(ctx->tc != NULL);
 	g_free(ctx->tc);
@@ -211,3 +210,18 @@ void bt_context_put(struct bt_context *ctx)
 	if (ctx->refcount == 0)
 		bt_context_destroy(ctx);
 }
+
+void remove_trace_handle(struct bt_trace_handle *handle)
+{
+	int ret;
+
+	/* Remove from containers */
+	bt_trace_collection_remove(handle->td->ctx->tc, handle->td);
+	/* Close the trace */
+	ret = handle->format->close_trace(handle->td);
+	if (ret) {
+		fprintf(stderr, "Error in close_trace callback\n");
+	}
+
+	bt_trace_handle_destroy(handle);
+}
-- 
1.8.4