This is an automated email from the ASF dual-hosted git repository.
mtaha pushed a commit to branch PG16
in repository https://gitbox.apache.org/repos/asf/age.git
The following commit(s) were added to refs/heads/PG16 by this push:
new db98357c Fix issue 2020: Memory leak (#2029)
db98357c is described below
commit db98357cec11bae53f5cb026e60ee0fe1ca1a336
Author: John Gemignani <[email protected]>
AuthorDate: Fri Aug 9 08:45:45 2024 -0700
Fix issue 2020: Memory leak (#2029)
Fixed issue 2020: Memory leak in the VLE cache cleanup routines. Or,
at least I fixed a good part of it.
NOTE: We should look further into this, resources permitting.
What was causing the leaks were the property datums in the hash tables.
The property datums were copied via datumCopy into the hash tables for
easier access when rebuilding an edge. However, they needed to be freed
when no longer needed.
No regression tests were impacted.
No regression tests were needed.
---
src/backend/utils/adt/age_global_graph.c | 54 ++++++++++++++++++++++++++++++--
src/backend/utils/adt/age_graphid_ds.c | 2 ++
2 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/age_global_graph.c
b/src/backend/utils/adt/age_global_graph.c
index aeae5e32..30ec7479 100644
--- a/src/backend/utils/adt/age_global_graph.c
+++ b/src/backend/utils/adt/age_global_graph.c
@@ -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;
@@ -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++;
@@ -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)
@@ -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)
{
@@ -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);
@@ -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);
@@ -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
@@ -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);
@@ -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 */
diff --git a/src/backend/utils/adt/age_graphid_ds.c
b/src/backend/utils/adt/age_graphid_ds.c
index 8c632b6d..73be8dc2 100644
--- a/src/backend/utils/adt/age_graphid_ds.c
+++ b/src/backend/utils/adt/age_graphid_ds.c
@@ -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);
}
