This is an automated email from the ASF dual-hosted git repository.
mtaha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/age.git
The following commit(s) were added to refs/heads/master by this push:
new 0ea94644 Fix possible memory and file descriptors leaks (#2258)
0ea94644 is described below
commit 0ea94644f25bdc08c803ff6521bfd412a6690401
Author: Aleksey Konovkin <[email protected]>
AuthorDate: Tue Dec 9 22:49:05 2025 +0300
Fix possible memory and file descriptors leaks (#2258)
- Used postgres memory allocation functions instead of standard ones.
- Wrapped main loop of csv loader in PG_TRY block for better error handling.
---
src/backend/utils/adt/age_global_graph.c | 6 +-
src/backend/utils/adt/agtype.c | 39 ++++++-----
src/backend/utils/load/ag_load_edges.c | 91 +++++++++++++-----------
src/backend/utils/load/ag_load_labels.c | 115 +++++++++++++++++--------------
src/include/utils/agtype.h | 1 +
5 files changed, 135 insertions(+), 117 deletions(-)
diff --git a/src/backend/utils/adt/age_global_graph.c
b/src/backend/utils/adt/age_global_graph.c
index 6f30060a..c34e51ee 100644
--- a/src/backend/utils/adt/age_global_graph.c
+++ b/src/backend/utils/adt/age_global_graph.c
@@ -1237,12 +1237,10 @@ Datum age_delete_global_graphs(PG_FUNCTION_ARGS)
{
char *graph_name = NULL;
- graph_name = strndup(agtv_temp->val.string.val,
- agtv_temp->val.string.len);
+ graph_name = pnstrdup(agtv_temp->val.string.val,
+ agtv_temp->val.string.len);
success = delete_specific_GRAPH_global_contexts(graph_name);
-
- free(graph_name);
}
else
{
diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c
index be838cbd..02fc3221 100644
--- a/src/backend/utils/adt/agtype.c
+++ b/src/backend/utils/adt/agtype.c
@@ -181,6 +181,17 @@ static agtype_value
*agtype_build_map_as_agtype_value(FunctionCallInfo fcinfo);
agtype_value *agtype_composite_to_agtype_value_binary(agtype *a);
static agtype_value *tostring_helper(Datum arg, Oid type, char *msghdr);
+
+void *repalloc_check(void *ptr, size_t len)
+{
+ if (ptr != NULL)
+ {
+ return repalloc(ptr, len);
+ }
+
+ return palloc(len);
+}
+
/*
* Due to how pfree can be implemented, it may not check for a passed NULL.
This
* wrapper does just that, it will only call pfree is the pointer passed is not
@@ -5580,7 +5591,7 @@ static char *get_label_name(const char *graph_name,
graphid element_graphid)
result = NameStr(*DatumGetName(heap_getattr(tuple, Anum_ag_label_name,
tupdesc, &column_is_null)));
/* duplicate it */
- result = strdup(result);
+ result = pstrdup(result);
/* end the scan and close the relation */
systable_endscan(scan_desc);
@@ -5673,8 +5684,8 @@ Datum age_startnode(PG_FUNCTION_ARGS)
Assert(AGT_ROOT_IS_SCALAR(agt_arg));
agtv_object = get_ith_agtype_value_from_container(&agt_arg->root, 0);
Assert(agtv_object->type == AGTV_STRING);
- graph_name = strndup(agtv_object->val.string.val,
- agtv_object->val.string.len);
+ graph_name = pnstrdup(agtv_object->val.string.val,
+ agtv_object->val.string.len);
/* get the edge */
agt_arg = AG_GET_ARG_AGTYPE_P(1);
@@ -5708,8 +5719,6 @@ Datum age_startnode(PG_FUNCTION_ARGS)
result = get_vertex(graph_name, label_name, start_id);
- free(label_name);
-
return result;
}
@@ -5738,8 +5747,8 @@ Datum age_endnode(PG_FUNCTION_ARGS)
Assert(AGT_ROOT_IS_SCALAR(agt_arg));
agtv_object = get_ith_agtype_value_from_container(&agt_arg->root, 0);
Assert(agtv_object->type == AGTV_STRING);
- graph_name = strndup(agtv_object->val.string.val,
- agtv_object->val.string.len);
+ graph_name = pnstrdup(agtv_object->val.string.val,
+ agtv_object->val.string.len);
/* get the edge */
agt_arg = AG_GET_ARG_AGTYPE_P(1);
@@ -5773,8 +5782,6 @@ Datum age_endnode(PG_FUNCTION_ARGS)
result = get_vertex(graph_name, label_name, end_id);
- free(label_name);
-
return result;
}
@@ -6401,11 +6408,10 @@ Datum age_tofloat(PG_FUNCTION_ARGS)
NumericGetDatum(agtv_value->val.numeric)));
else if (agtv_value->type == AGTV_STRING)
{
- string = strndup(agtv_value->val.string.val,
- agtv_value->val.string.len);
+ string = pnstrdup(agtv_value->val.string.val,
+ agtv_value->val.string.len);
result = float8in_internal_null(string, NULL, "double precision",
string, &is_valid);
- free(string);
if (!is_valid)
PG_RETURN_NULL();
}
@@ -6703,8 +6709,8 @@ Datum age_tointeger(PG_FUNCTION_ARGS)
{
char *endptr;
/* we need a null terminated cstring */
- string = strndup(agtv_value->val.string.val,
- agtv_value->val.string.len);
+ string = pnstrdup(agtv_value->val.string.val,
+ agtv_value->val.string.len);
/* convert it if it is a regular integer string */
result = strtoi64(string, &endptr, 10);
@@ -6718,7 +6724,6 @@ Datum age_tointeger(PG_FUNCTION_ARGS)
f = float8in_internal_null(string, NULL, "double precision",
string, &is_valid);
- free(string);
/*
* If the conversions failed or it's a special float value,
* return null.
@@ -6731,10 +6736,6 @@ Datum age_tointeger(PG_FUNCTION_ARGS)
result = (int64) f;
}
- else
- {
- free(string);
- }
}
else
{
diff --git a/src/backend/utils/load/ag_load_edges.c
b/src/backend/utils/load/ag_load_edges.c
index 67049431..931c6e0d 100644
--- a/src/backend/utils/load/ag_load_edges.c
+++ b/src/backend/utils/load/ag_load_edges.c
@@ -36,8 +36,8 @@ void edge_field_cb(void *field, size_t field_len, void *data)
if (cr->cur_field == cr->alloc)
{
cr->alloc *= 2;
- cr->fields = realloc(cr->fields, sizeof(char *) * cr->alloc);
- cr->fields_len = realloc(cr->header, sizeof(size_t *) * cr->alloc);
+ cr->fields = repalloc_check(cr->fields, sizeof(char *) * cr->alloc);
+ cr->fields_len = repalloc_check(cr->header, sizeof(size_t *) *
cr->alloc);
if (cr->fields == NULL)
{
cr->error = 1;
@@ -48,7 +48,7 @@ void edge_field_cb(void *field, size_t field_len, void *data)
}
cr->fields_len[cr->cur_field] = field_len;
cr->curr_row_length += field_len;
- cr->fields[cr->cur_field] = strndup((char*)field, field_len);
+ cr->fields[cr->cur_field] = pnstrdup((char*)field, field_len);
cr->cur_field += 1;
}
@@ -78,13 +78,13 @@ void edge_row_cb(int delim __attribute__((unused)), void
*data)
{
cr->header_num = cr->cur_field;
cr->header_row_length = cr->curr_row_length;
- cr->header_len = (size_t* )malloc(sizeof(size_t *) * cr->cur_field);
- cr->header = malloc((sizeof (char*) * cr->cur_field));
+ cr->header_len = (size_t* )palloc(sizeof(size_t *) * cr->cur_field);
+ cr->header = palloc((sizeof (char*) * cr->cur_field));
for (i = 0; i<cr->cur_field; i++)
{
cr->header_len[i] = cr->fields_len[i];
- cr->header[i] = strndup(cr->fields[i], cr->header_len[i]);
+ cr->header[i] = pnstrdup(cr->fields[i], cr->header_len[i]);
}
}
else
@@ -133,7 +133,7 @@ void edge_row_cb(int delim __attribute__((unused)), void
*data)
for (i = 0; i < n_fields; ++i)
{
- free(cr->fields[i]);
+ pfree_if_not_null(cr->fields[i]);
}
if (cr->error)
@@ -192,6 +192,10 @@ int create_edges_from_csv_file(char *file_path,
(errmsg("Failed to initialize csv parser\n")));
}
+ p.malloc_func = palloc;
+ p.realloc_func = repalloc_check;
+ p.free_func = pfree_if_not_null;
+
csv_set_space_func(&p, is_space);
csv_set_term_func(&p, is_term);
@@ -202,47 +206,52 @@ int create_edges_from_csv_file(char *file_path,
(errmsg("Failed to open %s\n", file_path)));
}
- label_seq_name = get_label_seq_relation_name(label_name);
-
- memset((void*)&cr, 0, sizeof(csv_edge_reader));
- cr.alloc = 128;
- cr.fields = malloc(sizeof(char *) * cr.alloc);
- cr.fields_len = malloc(sizeof(size_t *) * cr.alloc);
- cr.header_row_length = 0;
- cr.curr_row_length = 0;
- cr.graph_name = graph_name;
- cr.graph_oid = graph_oid;
- cr.label_name = label_name;
- cr.label_id = label_id;
- cr.label_seq_relid = get_relname_relid(label_seq_name, graph_oid);
- cr.load_as_agtype = load_as_agtype;
-
- /* Initialize the batch insert state */
- init_batch_insert(&cr.batch_state, label_name, graph_oid);
-
- while ((bytes_read=fread(buf, 1, 1024, fp)) > 0)
+ PG_TRY();
{
- if (csv_parse(&p, buf, bytes_read, edge_field_cb,
- edge_row_cb, &cr) != bytes_read)
+ label_seq_name = get_label_seq_relation_name(label_name);
+
+ memset((void*)&cr, 0, sizeof(csv_edge_reader));
+ cr.alloc = 128;
+ cr.fields = palloc(sizeof(char *) * cr.alloc);
+ cr.fields_len = palloc(sizeof(size_t *) * cr.alloc);
+ cr.header_row_length = 0;
+ cr.curr_row_length = 0;
+ cr.graph_name = graph_name;
+ cr.graph_oid = graph_oid;
+ cr.label_name = label_name;
+ cr.label_id = label_id;
+ cr.label_seq_relid = get_relname_relid(label_seq_name, graph_oid);
+ cr.load_as_agtype = load_as_agtype;
+
+ /* Initialize the batch insert state */
+ init_batch_insert(&cr.batch_state, label_name, graph_oid);
+
+ while ((bytes_read=fread(buf, 1, 1024, fp)) > 0)
{
- ereport(ERROR, (errmsg("Error while parsing file: %s\n",
- csv_strerror(csv_error(&p)))));
+ if (csv_parse(&p, buf, bytes_read, edge_field_cb,
+ edge_row_cb, &cr) != bytes_read)
+ {
+ ereport(ERROR, (errmsg("Error while parsing file: %s\n",
+ csv_strerror(csv_error(&p)))));
+ }
}
- }
- csv_fini(&p, edge_field_cb, edge_row_cb, &cr);
+ csv_fini(&p, edge_field_cb, edge_row_cb, &cr);
- /* Finish any remaining batch inserts */
- finish_batch_insert(&cr.batch_state);
+ /* Finish any remaining batch inserts */
+ finish_batch_insert(&cr.batch_state);
- if (ferror(fp))
+ if (ferror(fp))
+ {
+ ereport(ERROR, (errmsg("Error while reading file %s\n",
file_path)));
+ }
+ }
+ PG_FINALLY();
{
- ereport(ERROR, (errmsg("Error while reading file %s\n", file_path)));
+ fclose(fp);
+ csv_free(&p);
}
+ PG_END_TRY();
- fclose(fp);
-
- free(cr.fields);
- csv_free(&p);
return EXIT_SUCCESS;
-}
\ No newline at end of file
+}
diff --git a/src/backend/utils/load/ag_load_labels.c
b/src/backend/utils/load/ag_load_labels.c
index 4a04f3cd..1e86bbda 100644
--- a/src/backend/utils/load/ag_load_labels.c
+++ b/src/backend/utils/load/ag_load_labels.c
@@ -39,8 +39,8 @@ void vertex_field_cb(void *field, size_t field_len, void
*data)
if (cr->cur_field == cr->alloc)
{
cr->alloc *= 2;
- cr->fields = realloc(cr->fields, sizeof(char *) * cr->alloc);
- cr->fields_len = realloc(cr->header, sizeof(size_t *) * cr->alloc);
+ cr->fields = repalloc_check(cr->fields, sizeof(char *) * cr->alloc);
+ cr->fields_len = repalloc_check(cr->header, sizeof(size_t *) *
cr->alloc);
if (cr->fields == NULL)
{
cr->error = 1;
@@ -51,7 +51,7 @@ void vertex_field_cb(void *field, size_t field_len, void
*data)
}
cr->fields_len[cr->cur_field] = field_len;
cr->curr_row_length += field_len;
- cr->fields[cr->cur_field] = strndup((char *) field, field_len);
+ cr->fields[cr->cur_field] = pnstrdup((char *) field, field_len);
cr->cur_field += 1;
}
@@ -70,13 +70,13 @@ void vertex_row_cb(int delim __attribute__((unused)), void
*data)
{
cr->header_num = cr->cur_field;
cr->header_row_length = cr->curr_row_length;
- cr->header_len = (size_t* )malloc(sizeof(size_t *) * cr->cur_field);
- cr->header = malloc((sizeof (char*) * cr->cur_field));
+ cr->header_len = (size_t* )palloc(sizeof(size_t *) * cr->cur_field);
+ cr->header = palloc((sizeof (char*) * cr->cur_field));
for (i = 0; i<cr->cur_field; i++)
{
cr->header_len[i] = cr->fields_len[i];
- cr->header[i] = strndup(cr->fields[i], cr->header_len[i]);
+ cr->header[i] = pnstrdup(cr->fields[i], cr->header_len[i]);
}
}
else
@@ -129,7 +129,7 @@ void vertex_row_cb(int delim __attribute__((unused)), void
*data)
for (i = 0; i < n_fields; ++i)
{
- free(cr->fields[i]);
+ pfree_if_not_null(cr->fields[i]);
}
if (cr->error)
@@ -189,6 +189,10 @@ int create_labels_from_csv_file(char *file_path,
(errmsg("Failed to initialize csv parser\n")));
}
+ p.malloc_func = palloc;
+ p.realloc_func = repalloc_check;
+ p.free_func = pfree_if_not_null;
+
csv_set_space_func(&p, is_space);
csv_set_term_func(&p, is_term);
@@ -199,62 +203,67 @@ int create_labels_from_csv_file(char *file_path,
(errmsg("Failed to open %s\n", file_path)));
}
- label_seq_name = get_label_seq_relation_name(label_name);
-
- memset((void*)&cr, 0, sizeof(csv_vertex_reader));
-
- cr.alloc = 2048;
- cr.fields = malloc(sizeof(char *) * cr.alloc);
- cr.fields_len = malloc(sizeof(size_t *) * cr.alloc);
- cr.header_row_length = 0;
- cr.curr_row_length = 0;
- cr.graph_name = graph_name;
- cr.graph_oid = graph_oid;
- cr.label_name = label_name;
- cr.label_id = label_id;
- cr.id_field_exists = id_field_exists;
- cr.label_seq_relid = get_relname_relid(label_seq_name, graph_oid);
- cr.load_as_agtype = load_as_agtype;
-
- if (cr.id_field_exists)
+ PG_TRY();
{
- /*
- * Set the curr_seq_num since we will need it to compare with
- * incoming entry_id.
- *
- * We cant use currval because it will error out if nextval was
- * not called before in the session.
- */
- cr.curr_seq_num = nextval_internal(cr.label_seq_relid, true);
- }
+ label_seq_name = get_label_seq_relation_name(label_name);
+
+ memset((void*)&cr, 0, sizeof(csv_vertex_reader));
+
+ cr.alloc = 2048;
+ cr.fields = palloc(sizeof(char *) * cr.alloc);
+ cr.fields_len = palloc(sizeof(size_t *) * cr.alloc);
+ cr.header_row_length = 0;
+ cr.curr_row_length = 0;
+ cr.graph_name = graph_name;
+ cr.graph_oid = graph_oid;
+ cr.label_name = label_name;
+ cr.label_id = label_id;
+ cr.id_field_exists = id_field_exists;
+ cr.label_seq_relid = get_relname_relid(label_seq_name, graph_oid);
+ cr.load_as_agtype = load_as_agtype;
+
+ if (cr.id_field_exists)
+ {
+ /*
+ * Set the curr_seq_num since we will need it to compare with
+ * incoming entry_id.
+ *
+ * We cant use currval because it will error out if nextval was
+ * not called before in the session.
+ */
+ cr.curr_seq_num = nextval_internal(cr.label_seq_relid, true);
+ }
- /* Initialize the batch insert state */
- init_batch_insert(&cr.batch_state, label_name, graph_oid);
+ /* Initialize the batch insert state */
+ init_batch_insert(&cr.batch_state, label_name, graph_oid);
- while ((bytes_read=fread(buf, 1, 1024, fp)) > 0)
- {
- if (csv_parse(&p, buf, bytes_read, vertex_field_cb,
- vertex_row_cb, &cr) != bytes_read)
+ while ((bytes_read=fread(buf, 1, 1024, fp)) > 0)
{
- ereport(ERROR, (errmsg("Error while parsing file: %s\n",
- csv_strerror(csv_error(&p)))));
+ if (csv_parse(&p, buf, bytes_read, vertex_field_cb,
+ vertex_row_cb, &cr) != bytes_read)
+ {
+ ereport(ERROR, (errmsg("Error while parsing file: %s\n",
+ csv_strerror(csv_error(&p)))));
+ }
}
- }
- csv_fini(&p, vertex_field_cb, vertex_row_cb, &cr);
+ csv_fini(&p, vertex_field_cb, vertex_row_cb, &cr);
- /* Finish any remaining batch inserts */
- finish_batch_insert(&cr.batch_state);
+ /* Finish any remaining batch inserts */
+ finish_batch_insert(&cr.batch_state);
- if (ferror(fp))
+ if (ferror(fp))
+ {
+ ereport(ERROR, (errmsg("Error while reading file %s\n",
+ file_path)));
+ }
+ }
+ PG_FINALLY();
{
- ereport(ERROR, (errmsg("Error while reading file %s\n",
- file_path)));
+ fclose(fp);
+ csv_free(&p);
}
+ PG_END_TRY();
- fclose(fp);
-
- free(cr.fields);
- csv_free(&p);
return EXIT_SUCCESS;
}
\ No newline at end of file
diff --git a/src/include/utils/agtype.h b/src/include/utils/agtype.h
index 48677532..ab2ba08c 100644
--- a/src/include/utils/agtype.h
+++ b/src/include/utils/agtype.h
@@ -556,6 +556,7 @@ void pfree_agtype_value(agtype_value* value);
void pfree_agtype_value_content(agtype_value* value);
void pfree_agtype_in_state(agtype_in_state* value);
void pfree_if_not_null(void *ptr);
+void *repalloc_check(void *ptr, size_t len);
agtype_value *agtype_value_from_cstring(char *str, int len);
/* Oid accessors for AGTYPE */
Oid get_AGTYPEOID(void);
