Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 2020: Memory leak (#2028) #2030

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 51 additions & 3 deletions src/backend/utils/adt/age_global_graph.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ typedef struct GRAPH_global_context
int64 num_loaded_vertices; /* number of loaded vertices in this graph */
int64 num_loaded_edges; /* number of loaded edges in this graph */
ListGraphId *vertices; /* vertices for vertex hashtable cleanup */
ListGraphId *edges; /* edges for edge hashtable cleanup */
struct GRAPH_global_context *next; /* next graph */
} GRAPH_global_context;

Expand Down Expand Up @@ -301,6 +302,9 @@ static bool insert_edge_entry(GRAPH_global_context *ggctx, graphid edge_id,
ee->end_vertex_id = end_vertex_id;
ee->edge_label_table_oid = edge_label_table_oid;

/* we also need to store the edge id for clean up of edge property datums */
ggctx->edges = append_graphid(ggctx->edges, edge_id);

/* increment the number of loaded edges */
ggctx->num_loaded_edges++;

Expand Down Expand Up @@ -696,6 +700,7 @@ static void freeze_GRAPH_global_hashtables(GRAPH_global_context *ggctx)
static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
{
GraphIdNode *curr_vertex = NULL;
GraphIdNode *curr_edge = NULL;

/* don't do anything if NULL */
if (ggctx == NULL)
Expand All @@ -710,7 +715,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
ggctx->graph_oid = InvalidOid;
ggctx->next = NULL;

/* free the vertex edge lists, starting with the head */
/* free the vertex edge lists and properties, starting with the head */
curr_vertex = peek_stack_head(ggctx->vertices);
while (curr_vertex != NULL)
{
Expand All @@ -735,6 +740,10 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
return false;
}

/* free the vertex's datumCopy properties */
pfree(DatumGetPointer(value->vertex_properties));
value->vertex_properties = 0;

/* free the edge list associated with this vertex */
free_ListGraphId(value->edges_in);
free_ListGraphId(value->edges_out);
Expand All @@ -748,10 +757,47 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
curr_vertex = next_vertex;
}

/* free the edge properties, starting with the head */
curr_edge = peek_stack_head(ggctx->edges);
while (curr_edge != NULL)
{
GraphIdNode *next_edge = NULL;
edge_entry *value = NULL;
bool found = false;
graphid edge_id;

/* get the next edge in the list, if any */
next_edge = next_GraphIdNode(curr_edge);

/* get the current edge id */
edge_id = get_graphid(curr_edge);

/* retrieve the edge entry */
value = (edge_entry *)hash_search(ggctx->edge_hashtable,
(void *)&edge_id, HASH_FIND,
&found);
/* this is bad if it isn't found, but leave that to the caller */
if (found == false)
{
return false;
}

/* free the edge's datumCopy properties */
pfree(DatumGetPointer(value->edge_properties));
value->edge_properties = 0;

/* move to the next edge */
curr_edge = next_edge;
}

/* free the vertices list */
free_ListGraphId(ggctx->vertices);
ggctx->vertices = NULL;

/* free the edges list */
free_ListGraphId(ggctx->edges);
ggctx->edges = NULL;

/* free the hashtables */
hash_destroy(ggctx->vertex_hashtable);
hash_destroy(ggctx->edge_hashtable);
Expand Down Expand Up @@ -836,7 +882,7 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);

ereport(ERROR, (errcode(ERRCODE_DATA_EXCEPTION),
errmsg("missing vertex_entry during free")));
errmsg("missing vertex or edge entry during free")));
}
}
else
Expand Down Expand Up @@ -891,6 +937,8 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,

/* initialize our vertices list */
new_ggctx->vertices = NULL;
/* initialize our edges list */
new_ggctx->edges = NULL;

/* build the hashtables for this graph */
create_GRAPH_global_hashtables(new_ggctx);
Expand Down Expand Up @@ -939,7 +987,7 @@ static bool delete_GRAPH_global_contexts(void)
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);

ereport(ERROR, (errcode(ERRCODE_DATA_EXCEPTION),
errmsg("missing vertex_entry during free")));
errmsg("missing vertex or edge entry during free")));
}

/* advance to the next context */
Expand Down
2 changes: 2 additions & 0 deletions src/backend/utils/adt/age_graphid_ds.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,11 @@ void free_ListGraphId(ListGraphId *container)
next_node = curr_node->next;
/* we can do this because this is just a list of ints */
pfree(curr_node);
container->size--;
curr_node = next_node;
}

Assert(container->size == 0);
/* free the container */
pfree(container);
}
Expand Down
Loading