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

[Mathieu Desnoyers]

Merged, thanks!

Mathieu

----- Original Message -----
> From: "Jérémie Galarneau" <jeremie.galarneau at efficios.com>
> To: lttng-dev at lists.lttng.org
> Sent: Thursday, October 10, 2013 12:08:30 PM
> Subject: [lttng-dev] [PATCH babeltrace] Fix: Close traces on context	destruction
> 
> 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
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com