* Jérémie Galarneau ([email protected]) wrote:
> ctf_close_trace was being called twice when calling bt_context_remove_trace 
> thus
> causing free() to be called on an invalid pointer.
> 
> Calling bt_context_remove_trace() would call ctf_close_trace() once via the
> close_handle callback registered on the ctf format struct and a second call 
> would
> take place from bt_trace_handle_destroy() which is registered as the
> value_destroy_func on the trace_handles hash table of the current context.
> 
> The first explicit call to handle->format->close_trace is unnecessary.
> 
> The crash is reproducible by invoking the tests-python.py script.

I think we should leave the close_trace calls in both locations, but set
handle->td to -1 when we close it, and test for negative handle ID
before doing the close.

Thanks,

Mathieu

> 
> Signed-off-by: Jérémie Galarneau <[email protected]>
> ---
>  lib/context.c      | 8 +-------
>  lib/trace-handle.c | 5 ++++-
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/context.c b/lib/context.c
> index 5516e49..05fb994 100644
> --- a/lib/context.c
> +++ b/lib/context.c
> @@ -156,7 +156,6 @@ end:
>  int bt_context_remove_trace(struct bt_context *ctx, int handle_id)
>  {
>       struct bt_trace_handle *handle;
> -     int ret;
>  
>       if (!ctx)
>               return -EINVAL;
> @@ -168,12 +167,7 @@ int bt_context_remove_trace(struct bt_context *ctx, int 
> handle_id)
>  
>       /* Remove from containers */
>       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 and free the handle */
>       g_hash_table_remove(ctx->trace_handles,
>                       (gpointer) (unsigned long) handle_id);
> diff --git a/lib/trace-handle.c b/lib/trace-handle.c
> index 0da565b..17a6e0d 100644
> --- a/lib/trace-handle.c
> +++ b/lib/trace-handle.c
> @@ -49,7 +49,10 @@ struct bt_trace_handle *bt_trace_handle_create(struct 
> bt_context *ctx)
>  
>  void bt_trace_handle_destroy(struct bt_trace_handle *th)
>  {
> -     th->format->close_trace(th->td);
> +     if (th->format->close_trace(th->td)) {
> +             fprintf(stderr, "Error in close_trace callback\n");
> +     }
> +
>       g_free(th);
>  }
>  
> -- 
> 1.8.1.1
> 
> 
> _______________________________________________
> lttng-dev mailing list
> [email protected]
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

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

_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to