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.

Sign up for GitHub

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

Jump to bottom

Fix issue 2046: Memory leak during btree(agtype) (#2060) #2072

Merged
merged 1 commit into from
Aug 23, 2024
Merged

Fix issue 2046: Memory leak during btree(agtype) (#2060) #2072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 17 additions & 2 deletions src/backend/utils/adt/age_vle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1879,8 +1879,19 @@ Datum age_vle(PG_FUNCTION_ARGS)
*/
agtype *agt_materialize_vle_path(agtype *agt_arg_vpc)
{
/* convert the agtype_value to agtype and return it */
return agtype_value_to_agtype(agtv_materialize_vle_path(agt_arg_vpc));
agtype *agt_path = NULL;
agtype_value *agtv_path = NULL;

/* get the path */
agtv_path = agtv_materialize_vle_path(agt_arg_vpc);

/* convert agtype_value to agtype */
agt_path = agtype_value_to_agtype(agtv_path);

/* free in memory path */
pfree_agtype_value(agtv_path);

return agt_path;
}

/*
Expand Down Expand Up @@ -1940,6 +1951,8 @@ Datum age_match_two_vle_edges(PG_FUNCTION_ARGS)
left_array_size = left_path->graphid_array_size;
left_array = GET_GRAPHID_ARRAY_FROM_CONTAINER(left_path);

PG_FREE_IF_COPY(agt_arg_vpc, 0);

agt_arg_vpc = AG_GET_ARG_AGTYPE_P(1);

if (!AGT_ROOT_IS_BINARY(agt_arg_vpc) ||
Expand All @@ -1954,6 +1967,8 @@ Datum age_match_two_vle_edges(PG_FUNCTION_ARGS)
right_path = (VLE_path_container *)agt_arg_vpc;
right_array = GET_GRAPHID_ARRAY_FROM_CONTAINER(right_path);

PG_FREE_IF_COPY(agt_arg_vpc, 1);

if (left_array[left_array_size - 1] != right_array[0])
{
PG_RETURN_BOOL(false);
Expand Down
50 changes: 39 additions & 11 deletions src/backend/utils/adt/agtype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,10 @@ agtype_value *agtype_value_from_cstring(char *str, int len)
static inline Datum agtype_from_cstring(char *str, int len)
{
agtype_value *agtv = agtype_value_from_cstring(str, len);
agtype *agt = agtype_value_to_agtype(agtv);

PG_RETURN_POINTER(agtype_value_to_agtype(agtv));
pfree_agtype_value(agtv);
PG_RETURN_POINTER(agt);
}

size_t check_string_length(size_t len)
Expand Down Expand Up @@ -1928,7 +1930,7 @@ agtype_value *string_to_agtype_value(char *s)

agtv->type = AGTV_STRING;
agtv->val.string.len = check_string_length(strlen(s));
agtv->val.string.val = s;
agtv->val.string.val = pnstrdup(s, agtv->val.string.len);

return agtv;
}
Expand All @@ -1952,6 +1954,7 @@ PG_FUNCTION_INFO_V1(_agtype_build_path);
Datum _agtype_build_path(PG_FUNCTION_ARGS)
{
agtype_in_state result;
agtype *agt_result;
Datum *args = NULL;
bool *nulls = NULL;
Oid *types = NULL;
Expand Down Expand Up @@ -1994,8 +1997,10 @@ Datum _agtype_build_path(PG_FUNCTION_ARGS)
AGT_ROOT_BINARY_FLAGS(agt) == AGT_FBINARY_TYPE_VLE_PATH)
{
agtype *path = agt_materialize_vle_path(agt);
PG_FREE_IF_COPY(agt, i);
PG_RETURN_POINTER(path);
}
PG_FREE_IF_COPY(agt, i);
}
}

Expand Down Expand Up @@ -2045,6 +2050,8 @@ Datum _agtype_build_path(PG_FUNCTION_ARGS)
/* get the VLE path from the container as an agtype_value */
agtv_path = agtv_materialize_vle_path(agt);

PG_FREE_IF_COPY(agt, i);

/* it better be an AGTV_PATH */
Assert(agtv_path->type == AGTV_PATH);

Expand Down Expand Up @@ -2098,6 +2105,7 @@ Datum _agtype_build_path(PG_FUNCTION_ARGS)
{
add_agtype(AGTYPE_P_GET_DATUM(agt), false, &result, types[i],
false);
PG_FREE_IF_COPY(agt, i);
}
/* If we got here, we had a zero boundary case. So, clear it */
else
Expand All @@ -2112,7 +2120,11 @@ Datum _agtype_build_path(PG_FUNCTION_ARGS)
/* set it to a path type */
result.res->type = AGTV_PATH;

PG_RETURN_POINTER(agtype_value_to_agtype(result.res));
agt_result = agtype_value_to_agtype(result.res);

pfree_agtype_in_state(&result);

PG_RETURN_POINTER(agt_result);
}

Datum make_path(List *path)
Expand Down Expand Up @@ -2436,14 +2448,18 @@ PG_FUNCTION_INFO_V1(agtype_build_map);
Datum agtype_build_map(PG_FUNCTION_ARGS)
{
agtype_value *result = NULL;
agtype *agt_result = NULL;

result = agtype_build_map_as_agtype_value(fcinfo);
if (result == NULL)
{
PG_RETURN_NULL();
}

PG_RETURN_POINTER(agtype_value_to_agtype(result));
agt_result = agtype_value_to_agtype(result);
pfree_agtype_value(result);

PG_RETURN_POINTER(agt_result);
}

PG_FUNCTION_INFO_V1(agtype_build_map_noargs);
Expand Down Expand Up @@ -4256,7 +4272,7 @@ Datum agtype_in_operator(PG_FUNCTION_ARGS)
result = (compare_agtype_scalar_values(&agtv_item, &agtv_elem) ==
0);
}
}
}
}

return boolean_to_agtype(result);
Expand Down Expand Up @@ -4417,19 +4433,31 @@ Datum agtype_btree_cmp(PG_FUNCTION_ARGS)
{
agtype *agtype_lhs;
agtype *agtype_rhs;
int32 result;

if (PG_ARGISNULL(0) && PG_ARGISNULL(1))
PG_RETURN_INT16(0);
{
PG_RETURN_INT32(0);
}
else if (PG_ARGISNULL(0))
PG_RETURN_INT16(1);
{
PG_RETURN_INT32(1);
}
else if (PG_ARGISNULL(1))
PG_RETURN_INT16(-1);
{
PG_RETURN_INT32(-1);
}

agtype_lhs = AG_GET_ARG_AGTYPE_P(0);
agtype_rhs = AG_GET_ARG_AGTYPE_P(1);

PG_RETURN_INT16(compare_agtype_containers_orderability(&agtype_lhs->root,
&agtype_rhs->root));
result = compare_agtype_containers_orderability(&agtype_lhs->root,
&agtype_rhs->root);

PG_FREE_IF_COPY(agtype_lhs, 0);
PG_FREE_IF_COPY(agtype_rhs, 1);

PG_RETURN_INT32(result);
}

PG_FUNCTION_INFO_V1(agtype_typecast_numeric);
Expand Down Expand Up @@ -5581,7 +5609,7 @@ Datum age_tail(PG_FUNCTION_ARGS)
WAGT_END_ARRAY, NULL);

agt_result = agtype_value_to_agtype(agis_result.res);
pfree_agtype_value(agis_result.res);
pfree_agtype_in_state(&agis_result);

PG_RETURN_POINTER(agt_result);
}
Expand Down
38 changes: 35 additions & 3 deletions src/backend/utils/adt/agtype_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ static agtype_value *push_agtype_value_scalar(agtype_parse_state **pstate,
agtype_value *scalar_val);
static int compare_two_floats_orderability(float8 lhs, float8 rhs);
static int get_type_sort_priority(enum agtype_value_type type);
static void pfree_agtype_value_content(agtype_value* value);
static void pfree_iterator_agtype_value_token(agtype_iterator_token token,
agtype_value *agtv);

/*
* Turn an in-memory agtype_value into an agtype for on-disk storage.
Expand Down Expand Up @@ -234,6 +237,17 @@ static int get_type_sort_priority(enum agtype_value_type type)
return -1;
}

static void pfree_iterator_agtype_value_token(agtype_iterator_token token,
agtype_value *agtv)
{
if (token == WAGT_KEY ||
token == WAGT_VALUE ||
token == WAGT_ELEM)
{
pfree_agtype_value_content(agtv);
}
}

/*
* BT comparator worker function. Returns an integer less than, equal to, or
* greater than zero, indicating whether a is less than, equal to, or greater
Expand Down Expand Up @@ -269,6 +283,10 @@ int compare_agtype_containers_orderability(agtype_container *a,
if (ra == WAGT_DONE)
{
/* Decisively equal */

/* free the agtype_values associated with the tokens */
pfree_iterator_agtype_value_token(ra, &va);
pfree_iterator_agtype_value_token(rb, &vb);
break;
}

Expand All @@ -280,6 +298,10 @@ int compare_agtype_containers_orderability(agtype_container *a,
* initially, at the WAGT_BEGIN_ARRAY and WAGT_BEGIN_OBJECT
* tokens.
*/

/* free the agtype_values associated with the tokens */
pfree_iterator_agtype_value_token(ra, &va);
pfree_iterator_agtype_value_token(rb, &vb);
continue;
}

Expand Down Expand Up @@ -367,12 +389,18 @@ int compare_agtype_containers_orderability(agtype_container *a,
if (ra == WAGT_END_ARRAY || ra == WAGT_END_OBJECT)
{
res = -1;
/* free the agtype_values associated with the tokens */
pfree_iterator_agtype_value_token(ra, &va);
pfree_iterator_agtype_value_token(rb, &vb);
break;
}
/* If right side is shorter, greater than */
if (rb == WAGT_END_ARRAY || rb == WAGT_END_OBJECT)
{
res = 1;
/* free the agtype_values associated with the tokens */
pfree_iterator_agtype_value_token(ra, &va);
pfree_iterator_agtype_value_token(rb, &vb);
break;
}

Expand All @@ -387,7 +415,7 @@ int compare_agtype_containers_orderability(agtype_container *a,
{
rb = agtype_iterator_next(&itb, &vb, false);
}

Assert(va.type != vb.type);
Assert(va.type != AGTV_BINARY);
Assert(vb.type != AGTV_BINARY);
Expand All @@ -397,6 +425,9 @@ int compare_agtype_containers_orderability(agtype_container *a,
-1 :
1;
}
/* free the agtype_values associated with the tokens */
pfree_iterator_agtype_value_token(ra, &va);
pfree_iterator_agtype_value_token(rb, &vb);
} while (res == 0);

while (ita != NULL)
Expand Down Expand Up @@ -2330,11 +2361,11 @@ void pfree_agtype_value(agtype_value* value)
}

/*
* Helper function that recursively deallocates the contents
* Helper function that recursively deallocates the contents
* of the passed agtype_value only. It does not deallocate
* `value` itself.
*/
void pfree_agtype_value_content(agtype_value* value)
static void pfree_agtype_value_content(agtype_value* value)
{
int i;

Expand All @@ -2352,6 +2383,7 @@ void pfree_agtype_value_content(agtype_value* value)
* The char pointer (val.string.val) is not free'd because
* it is not allocated by an agtype helper function.
*/
pfree(value->val.string.val);
break;

case AGTV_ARRAY:
Expand Down
17 changes: 7 additions & 10 deletions src/backend/utils/load/age_load.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,6 @@ agtype *create_agtype_from_list(char **header, char **fields, size_t fields_len,
WAGT_VALUE,
value_agtype);

pfree_agtype_value(key_agtype);
pfree_agtype_value(value_agtype);

for (i = 0; i<fields_len; i++)
{
key_agtype = string_to_agtype_value(header[i]);
Expand All @@ -156,15 +153,15 @@ agtype *create_agtype_from_list(char **header, char **fields, size_t fields_len,
result.res = push_agtype_value(&result.parse_state,
WAGT_VALUE,
value_agtype);

pfree_agtype_value(key_agtype);
pfree_agtype_value(value_agtype);
}

result.res = push_agtype_value(&result.parse_state,
WAGT_END_OBJECT, NULL);

/* serialize it */
out = agtype_value_to_agtype(result.res);

/* now that it is serialized we can free the in memory structure */
pfree_agtype_in_state(&result);

return out;
Expand Down Expand Up @@ -209,15 +206,15 @@ agtype* create_agtype_from_list_i(char **header, char **fields,
result.res = push_agtype_value(&result.parse_state,
WAGT_VALUE,
value_agtype);

pfree_agtype_value(key_agtype);
pfree_agtype_value(value_agtype);
}

result.res = push_agtype_value(&result.parse_state,
WAGT_END_OBJECT, NULL);

/* serialize it */
out = agtype_value_to_agtype(result.res);

/* now that it is serialized we can free the in memory structure */
pfree_agtype_in_state(&result);

return out;
Expand Down Expand Up @@ -497,4 +494,4 @@ static int32 get_or_create_label(Oid graph_oid, char *graph_name,
}

return label_id;
}
}
1 change: 0 additions & 1 deletion src/include/utils/agtype.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,6 @@ agtype_iterator *get_next_list_element(agtype_iterator *it,
agtype_container *agtc,
agtype_value *elem);
void pfree_agtype_value(agtype_value* value);
void pfree_agtype_value_content(agtype_value* value);
void pfree_agtype_in_state(agtype_in_state* value);
agtype_value *agtype_value_from_cstring(char *str, int len);

Expand Down
Loading