* Julien Desfossez ([email protected]) wrote: > On 21/01/13 02:32 PM, Mathieu Desnoyers wrote: > > * 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. > > I agree, we need the two calls to close, one is called when we remove a > trace manually and the other one is called when we destroy the context. > Setting and testing the td to -1 will solve the problem and will avoid > calling close_trace at all if not needed.
Can we ever call bt_trace_handle_destroy() manually on a handle that needs the trace to be closed ? After careful review, I don't think so... (see patch v2 from Jérémie). Thanks, Mathieu > > Thanks, > > Julien > > > > > 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
