Fix: free all the metadata-related memory

When opening a trace, the parser allocates various data structures to
represent internally the metadata.
This patch frees all this memory when a trace is closed.

When babeltrace was originally developed, it was meant to be mainly a
trace converter, so relying on the operating system to free memory was
"acceptable", although a bad practice. Now that libbabeltrace allows
reading the CTF format from an external application, it becomes clear
that fixing those memory leaks is required.

As of this commit, valgrind invoked in the following way on babeltrace
does not report any memory leak:
G_SLICE=always-malloc G_DEBUG=gc-friendly valgrind --leak-check=full

Let's keep it that way :-) !

Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
index f1fdba178f0a773d28a2286dae9eee133c29e929..e59e3573b8709b6440e37f8560ca3dfe1e737298 100644 (file)
--- a/formats/ctf/ctf.c
+++ b/formats/ctf/ctf.c
@@ -1267,6 +1267,7 @@ int create_stream_packet_index(struct ctf_trace *td,
                packet_index.timestamp_begin = 0;
                packet_index.timestamp_end = 0;
                packet_index.events_discarded = 0;
+               packet_index.events_discarded_len = 0;
 
                /* read and check header, set stream id (and check) */
                if (file_stream->parent.trace_packet_header) {
@@ -1903,35 +1904,13 @@ void ctf_close_trace(struct trace_descriptor *tdp)
                                continue;
                        for (j = 0; j < stream->streams->len; j++) {
                                struct ctf_file_stream *file_stream;
-                               file_stream = container_of(g_ptr_array_index(stream->streams, j), struct ctf_file_stream, parent);
+                               file_stream = container_of(g_ptr_array_index(stream->streams, j),
+                                               struct ctf_file_stream, parent);
                                ctf_close_file_stream(file_stream);
                        }
-
-               }
-               g_ptr_array_free(td->streams, TRUE);
-       }
-
-       if (td->event_declarations) {
-               for (i = 0; i < td->event_declarations->len; i++) {
-                       struct bt_ctf_event_decl *event;
-
-                       event = g_ptr_array_index(td->event_declarations, i);
-                       if (event->context_decl)
-                               g_ptr_array_free(event->context_decl, TRUE);
-                       if (event->fields_decl)
-                               g_ptr_array_free(event->fields_decl, TRUE);
-                       if (event->packet_header_decl)
-                               g_ptr_array_free(event->packet_header_decl, TRUE);
-                       if (event->event_context_decl)
-                               g_ptr_array_free(event->event_context_decl, TRUE);
-                       if (event->event_header_decl)
-                               g_ptr_array_free(event->event_header_decl, TRUE);
-                       if (event->packet_context_decl)
-                               g_ptr_array_free(event->packet_context_decl, TRUE);
-                       g_free(event);
                }
-               g_ptr_array_free(td->event_declarations, TRUE);
        }
+       ctf_destroy_metadata(td);
        closedir(td->dir);
        g_free(td);
 }

diff --git a/formats/ctf/metadata/ctf-ast.h b/formats/ctf/metadata/ctf-ast.h
index f3107fd29f02e313ceddf62d2384d2f92adea3c9..d5a0544a9d6d05f98f35dbe3bd7f68c194d9c09d 100644 (file)
--- a/formats/ctf/metadata/ctf-ast.h
+++ b/formats/ctf/metadata/ctf-ast.h
@@ -307,5 +307,6 @@ int ctf_visitor_semantic_check(FILE *fd, int depth, struct ctf_node *node);
 int ctf_visitor_parent_links(FILE *fd, int depth, struct ctf_node *node);
 int ctf_visitor_construct_metadata(FILE *fd, int depth, struct ctf_node *node,
                        struct ctf_trace *trace, int byte_order);
+int ctf_destroy_metadata(struct ctf_trace *trace);
 
 #endif /* _CTF_AST_H */

diff --git a/formats/ctf/metadata/ctf-parser.y b/formats/ctf/metadata/ctf-parser.y
index 7f91365d329f4eea2bdb54cb7262f5561e752257..c59f2e751f086f3bc04b8be007dff4d9c84ab9c4 100644 (file)
--- a/formats/ctf/metadata/ctf-parser.y
+++ b/formats/ctf/metadata/ctf-parser.y
@@ -860,6 +860,7 @@ static void ctf_ast_free(struct ctf_ast *ast)
 
        bt_list_for_each_entry_safe(node, tmp, &ast->allocated_nodes, gc)
                free(node);
+       free(ast);
 }
 
 int ctf_scanner_append_ast(struct ctf_scanner *scanner)

diff --git a/formats/ctf/metadata/ctf-visitor-generate-io-struct.c b/formats/ctf/metadata/ctf-visitor-generate-io-struct.c
index de9986113de508f36bfe66ac5950c6786da728ef..0b93b7234cb443631b261c38cf43a7736ff41d03 100644 (file)
--- a/formats/ctf/metadata/ctf-visitor-generate-io-struct.c
+++ b/formats/ctf/metadata/ctf-visitor-generate-io-struct.c
@@ -508,6 +508,7 @@ struct declaration *ctf_type_declarator_visit(FILE *fd, int depth,
                                fprintf(fd, "[error] %s: cannot create array declaration.\n", __func__);
                                return NULL;
                        }
+                       declaration_unref(nested_declaration);
                        declaration = &array_declaration->p;
                        break;
                }
@@ -520,9 +521,12 @@ struct declaration *ctf_type_declarator_visit(FILE *fd, int depth,
                        sequence_declaration = sequence_declaration_new(length_name, nested_declaration, declaration_scope);
                        if (!sequence_declaration) {
                                fprintf(fd, "[error] %s: cannot create sequence declaration.\n", __func__);
+                               g_free(length_name);
                                return NULL;
                        }
+                       declaration_unref(nested_declaration);
                        declaration = &sequence_declaration->p;
+                       g_free(length_name);
                        break;
                }
                default:
@@ -571,6 +575,7 @@ int ctf_struct_type_declarators_visit(FILE *fd, int depth,
                struct_declaration_add_field(struct_declaration,
                                             g_quark_to_string(field_name),
                                             field_declaration);
+               declaration_unref(field_declaration);
        }
        return 0;
 }
@@ -604,10 +609,10 @@ int ctf_variant_type_declarators_visit(FILE *fd, int depth,
                        return -EINVAL;
                }
 
-
                untagged_variant_declaration_add_field(untagged_variant_declaration,
                                              g_quark_to_string(field_name),
                                              field_declaration);
+               declaration_unref(field_declaration);
        }
        return 0;
 }
@@ -647,6 +652,7 @@ int ctf_typedef_visit(FILE *fd, int depth, struct declaration_scope *scope,
                        type_declaration->declaration_free(type_declaration);
                        return ret;
                }
+               declaration_unref(type_declaration);
        }
        return 0;
 }
@@ -711,6 +717,7 @@ int ctf_typealias_visit(FILE *fd, int depth, struct declaration_scope *scope,
        err = register_declaration(alias_q, type_declaration, scope);
        if (err)
                goto error;
+       declaration_unref(type_declaration);
        return 0;
 
 error:
@@ -828,6 +835,7 @@ struct declaration *ctf_declaration_struct_visit(FILE *fd,
                struct_declaration =
                        lookup_struct_declaration(g_quark_from_string(name),
                                                  declaration_scope);
+               declaration_ref(&struct_declaration->p);
                return &struct_declaration->p;
        } else {
                uint64_t min_align_value = 0;
@@ -894,6 +902,7 @@ struct declaration *ctf_declaration_variant_visit(FILE *fd,
                untagged_variant_declaration =
                        lookup_variant_declaration(g_quark_from_string(name),
                                                   declaration_scope);
+               declaration_ref(&untagged_variant_declaration->p);
        } else {
                /* For unnamed variant, create type */
                /* For named variant (with body), create type and add to declaration scope */
@@ -1054,6 +1063,7 @@ struct declaration *ctf_declaration_enum_visit(FILE *fd, int depth,
                enum_declaration =
                        lookup_enum_declaration(g_quark_from_string(name),
                                                declaration_scope);
+               declaration_ref(&enum_declaration->p);
                return &enum_declaration->p;
        } else {
                /* For unnamed enum, create type */
@@ -1107,6 +1117,7 @@ struct declaration *ctf_declaration_enum_visit(FILE *fd, int depth,
                                        enum_declaration,
                                        declaration_scope);
                        assert(!ret);
+                       declaration_unref(&enum_declaration->p);
                }
                return &enum_declaration->p;
        }
@@ -1134,6 +1145,7 @@ struct declaration *ctf_declaration_type_specifier_visit(FILE *fd, int depth,
        id_q = g_quark_from_string(str_c);
        g_free(str_c);
        declaration = lookup_declaration(id_q, declaration_scope);
+       declaration_ref(declaration);
        return declaration;
 }
 
@@ -2644,6 +2656,7 @@ int ctf_env_declaration_visit(FILE *fd, int depth, struct ctf_node *node,
                        strncpy(env->procname, right, TRACER_ENV_LEN);
                        env->procname[TRACER_ENV_LEN - 1] = '\0';
                        printf_verbose("env.procname = \"%s\"\n", env->procname);
+                       g_free(right);
                } else if (!strcmp(left, "hostname")) {
                        char *right;
 
@@ -2659,6 +2672,7 @@ int ctf_env_declaration_visit(FILE *fd, int depth, struct ctf_node *node,
                        strncpy(env->hostname, right, TRACER_ENV_LEN);
                        env->hostname[TRACER_ENV_LEN - 1] = '\0';
                        printf_verbose("env.hostname = \"%s\"\n", env->hostname);
+                       g_free(right);
                } else if (!strcmp(left, "domain")) {
                        char *right;
 
@@ -2674,6 +2688,7 @@ int ctf_env_declaration_visit(FILE *fd, int depth, struct ctf_node *node,
                        strncpy(env->domain, right, TRACER_ENV_LEN);
                        env->domain[TRACER_ENV_LEN - 1] = '\0';
                        printf_verbose("env.domain = \"%s\"\n", env->domain);
+                       g_free(right);
                } else if (!strcmp(left, "sysname")) {
                        char *right;
 
@@ -2689,6 +2704,7 @@ int ctf_env_declaration_visit(FILE *fd, int depth, struct ctf_node *node,
                        strncpy(env->sysname, right, TRACER_ENV_LEN);
                        env->sysname[TRACER_ENV_LEN - 1] = '\0';
                        printf_verbose("env.sysname = \"%s\"\n", env->sysname);
+                       g_free(right);
                } else if (!strcmp(left, "kernel_release")) {
                        char *right;
 
@@ -2704,6 +2720,7 @@ int ctf_env_declaration_visit(FILE *fd, int depth, struct ctf_node *node,
                        strncpy(env->release, right, TRACER_ENV_LEN);
                        env->release[TRACER_ENV_LEN - 1] = '\0';
                        printf_verbose("env.release = \"%s\"\n", env->release);
+                       g_free(right);
                } else if (!strcmp(left, "kernel_version")) {
                        char *right;
 
@@ -2719,12 +2736,14 @@ int ctf_env_declaration_visit(FILE *fd, int depth, struct ctf_node *node,
                        strncpy(env->version, right, TRACER_ENV_LEN);
                        env->version[TRACER_ENV_LEN - 1] = '\0';
                        printf_verbose("env.version = \"%s\"\n", env->version);
+                       g_free(right);
                } else {
                        if (is_unary_string(&node->u.ctf_expression.right)) {
                                char *right;
 
                                right = concatenate_unary_strings(&node->u.ctf_expression.right);
                                printf_verbose("env.%s = \"%s\"\n", left, right);
+                               g_free(right);
                        } else if (is_unary_unsigned(&node->u.ctf_expression.right)) {
                                uint64_t v;
                                int ret;
@@ -2822,7 +2841,6 @@ int ctf_root_declaration_visit(FILE *fd, int depth, struct ctf_node *node, struc
        return 0;
 }
 
-/* TODO: missing metadata "destroy" (memory leak) */
 int ctf_visitor_construct_metadata(FILE *fd, int depth, struct ctf_node *node,
                struct ctf_trace *trace, int byte_order)
 {
@@ -2938,3 +2956,104 @@ error:
        g_hash_table_destroy(trace->clocks);
        return ret;
 }
+
+int ctf_destroy_metadata(struct ctf_trace *trace)
+{
+       int i, j, k;
+       struct ctf_file_stream *metadata_stream;
+
+       if (trace->streams) {
+               for (i = 0; i < trace->streams->len; i++) {
+                       struct ctf_stream_declaration *stream;
+
+                       stream = g_ptr_array_index(trace->streams, i);
+                       if (!stream)
+                               continue;
+                       for (j = 0; j < stream->streams->len; j++) {
+                               struct ctf_stream_definition *stream_def;
+
+                               stream_def = g_ptr_array_index(stream->streams, j);
+                               if (!stream_def)
+                                       continue;
+                               for (k = 0; k < stream_def->events_by_id->len; k++) {
+                                       struct ctf_event_definition *event;
+
+                                       event = g_ptr_array_index(stream_def->events_by_id, k);
+                                       if (!event)
+                                               continue;
+                                       if (&event->event_fields->p)
+                                               definition_unref(&event->event_fields->p);
+                                       if (&event->event_context->p)
+                                               definition_unref(&event->event_context->p);
+                                       g_free(event);
+                               }
+                               if (&stream_def->trace_packet_header->p)
+                                       definition_unref(&stream_def->trace_packet_header->p);
+                               if (&stream_def->stream_event_header->p)
+                                       definition_unref(&stream_def->stream_event_header->p);
+                               if (&stream_def->stream_packet_context->p)
+                                       definition_unref(&stream_def->stream_packet_context->p);
+                               if (&stream_def->stream_event_context->p)
+                                       definition_unref(&stream_def->stream_event_context->p);
+                               g_ptr_array_free(stream_def->events_by_id, TRUE);
+                               g_free(stream_def);
+                       }
+                       if (stream->event_header_decl)
+                               declaration_unref(&stream->event_header_decl->p);
+                       if (stream->event_context_decl)
+                               declaration_unref(&stream->event_context_decl->p);
+                       if (stream->packet_context_decl)
+                               declaration_unref(&stream->packet_context_decl->p);
+                       g_ptr_array_free(stream->streams, TRUE);
+                       g_ptr_array_free(stream->events_by_id, TRUE);
+                       g_hash_table_destroy(stream->event_quark_to_id);
+                       free_declaration_scope(stream->declaration_scope);
+                       g_free(stream);
+               }
+               g_ptr_array_free(trace->streams, TRUE);
+       }
+
+       if (trace->event_declarations) {
+               for (i = 0; i < trace->event_declarations->len; i++) {
+                       struct bt_ctf_event_decl *event_decl;
+                       struct ctf_event_declaration *event;
+
+                       event_decl = g_ptr_array_index(trace->event_declarations, i);
+                       if (event_decl->context_decl)
+                               g_ptr_array_free(event_decl->context_decl, TRUE);
+                       if (event_decl->fields_decl)
+                               g_ptr_array_free(event_decl->fields_decl, TRUE);
+                       if (event_decl->packet_header_decl)
+                               g_ptr_array_free(event_decl->packet_header_decl, TRUE);
+                       if (event_decl->event_context_decl)
+                               g_ptr_array_free(event_decl->event_context_decl, TRUE);
+                       if (event_decl->event_header_decl)
+                               g_ptr_array_free(event_decl->event_header_decl, TRUE);
+                       if (event_decl->packet_context_decl)
+                               g_ptr_array_free(event_decl->packet_context_decl, TRUE);
+
+                       event = &event_decl->parent;
+                       if (event->fields_decl)
+                               declaration_unref(&event->fields_decl->p);
+                       if (event->context_decl)
+                               declaration_unref(&event->context_decl->p);
+                       free_declaration_scope(event->declaration_scope);
+
+                       g_free(event);
+               }
+               g_ptr_array_free(trace->event_declarations, TRUE);
+       }
+       if (trace->packet_header_decl)
+               declaration_unref(&trace->packet_header_decl->p);
+
+       free_declaration_scope(trace->root_declaration_scope);
+       free_declaration_scope(trace->declaration_scope);
+
+       g_hash_table_destroy(trace->callsites);
+       g_hash_table_destroy(trace->clocks);
+
+       metadata_stream = container_of(trace->metadata, struct ctf_file_stream, parent);
+       g_free(metadata_stream);
+
+       return 0;
+}

diff --git a/types/sequence.c b/types/sequence.c
index bd49d2a6507f116a5f916a423785761e06fd1cb5..81270facd1096bc26fa25829fdbea505c42f0f19 100644 (file)
--- a/types/sequence.c
+++ b/types/sequence.c
@@ -206,8 +206,8 @@ void _sequence_definition_free(struct definition *definition)
                        field = g_ptr_array_index(sequence->elems, i);
                        field->declaration->definition_free(field);
                }
+               (void) g_ptr_array_free(sequence->elems, TRUE);
        }
-       (void) g_ptr_array_free(sequence->elems, TRUE);
        definition_unref(len_definition);
        free_definition_scope(sequence->p.scope);
        declaration_unref(sequence->p.declaration);

diff --git a/types/struct.c b/types/struct.c
index f360407c46f3ed956285689ae3847c9b5d01625a..22689c1cc4130f46053ccd701024c3b1d19eeb1c 100644 (file)
--- a/types/struct.c
+++ b/types/struct.c
@@ -169,6 +169,7 @@ void _struct_definition_free(struct definition *definition)
        }
        free_definition_scope(_struct->p.scope);
        declaration_unref(_struct->p.declaration);
+       g_ptr_array_free(_struct->fields, TRUE);
        g_free(_struct);
 }
 

diff --git a/types/types.c b/types/types.c
index 450c93ba6b5206c9edcdd8fb726e92c0b029d73a..f74e730f3df71e2531b27b54bff4dc1e45a2db7a 100644 (file)
--- a/types/types.c
+++ b/types/types.c
@@ -32,10 +32,13 @@ GQuark prefix_quark(const char *prefix, GQuark quark)
 {
        GQuark nq;
        GString *str;
+       char *quark_str;
 
        str = g_string_new(prefix);
        g_string_append(str, g_quark_to_string(quark));
-       nq = g_quark_from_string(g_string_free(str, FALSE));
+       quark_str = g_string_free(str, FALSE);
+       nq = g_quark_from_string(quark_str);
+       g_free(quark_str);
        return nq;
 }
 

diff --git a/types/variant.c b/types/variant.c
index faf70dde200e6bbff757ecc889e1132e5dca5cde..660fb5817d93cfb244894e85c48a6e810b9bd964 100644 (file)
--- a/types/variant.c
+++ b/types/variant.c
@@ -90,8 +90,9 @@ void _variant_declaration_free(struct declaration *declaration)
        struct declaration_variant *variant_declaration =
                container_of(declaration, struct declaration_variant, p);
 
-       _untagged_variant_declaration_free(&variant_declaration->untagged_variant->p);
+       declaration_unref(&variant_declaration->untagged_variant->p);
        g_array_free(variant_declaration->tag_name, TRUE);
+       g_free(variant_declaration);
 }
 
 struct declaration_variant *
@@ -244,6 +245,7 @@ void _variant_definition_free(struct definition *definition)
        definition_unref(variant->enum_tag);
        free_definition_scope(variant->p.scope);
        declaration_unref(variant->p.declaration);
+       g_ptr_array_free(variant->fields, TRUE);
        g_free(variant);
 }